Pull to refresh

Пятничная история про synchronized-методы в классе Thread

Reading time 2 min
Views 13K
Недавно я наткнулся на интересную особенность синхронизации на классе Thread. Я создал класс, наследующий Thread и использовал в нем паттерн lock object для внутренней синхронизации. В какой-то момент мне показалось что lock object раздувает код, а поскольку созданный поток я использую из очень малого числа мест, я его выкинул, заменив на synchronized методы и wait/notify на this. Если хотите знать, что из этого маленького нарушения «кодекса» вышло — добро пожаловать под кат.

В-общем ничего хорошего не вышло. Программа работала нормально почти всегда, но на CI периодически начала вести себя некорректно.
Беглый просмотр класса Thread показал, что механизм java synchronization используется в самом классе Thread, а именно в методе join:
    public final synchronized void join(long millis) 
    throws InterruptedException {
	long base = System.currentTimeMillis();
	long now = 0;

	if (millis < 0) {
            throw new IllegalArgumentException("timeout value is negative");
	}

	if (millis == 0) {
	    while (isAlive()) {
		wait(0);
	    }
	} else {
	    while (isAlive()) {
		long delay = millis - now;
		if (delay <= 0) {
		    break;
		}
		wait(delay);
		now = System.currentTimeMillis() - base;
	    }
	}
    }


Собственно сам notify() в классе Thread не вызывается, а вызывается в cpp-шном коде после завершения потока:
static void ensure_join(JavaThread* thread) {
  // We do not need to grap the Threads_lock, since we are operating on ourself.
  Handle threadObj(thread, thread->threadObj());
  assert(threadObj.not_null(), "java thread object must exist");
  ObjectLocker lock(threadObj, thread);
  // Ignore pending exception (ThreadDeath), since we are exiting anyway
  thread->clear_pending_exception();
  // It is of profound importance that we set the stillborn bit and reset the thread object,
  // before we do the notify. Since, changing these two variable will make JVM_IsAlive return
  // false. So in case another thread is doing a join on this thread , it will detect that the thread
  // is dead when it gets notified.
  java_lang_Thread::set_stillborn(threadObj());
  // Thread is exiting. So set thread_status field in  java.lang.Thread class to TERMINATED.
  java_lang_Thread::set_thread_status(threadObj(), java_lang_Thread::TERMINATED);
  java_lang_Thread::set_thread(threadObj(), NULL);
  lock.notify_all(thread); 
  // Ignore pending exception (ThreadDeath), since we are exiting anyway
  thread->clear_pending_exception();
}

Таким образом, создается две проблемы — делая notify() вы, если не повезет, пробуждаете поток join, что не страшно, но не пробуждаете свой поток, что страшно.
И второе, после завершения потока, на нем вызывается notifyAll() что разбудит все ваши wait(), хотя вы очевидно этого не хотели бы.

В-общем мораль пятничной истории банальна — используйте lock object, если есть хотя бы потенциальная возможность что на объекты вашего класса будет синхронизоваться кто-то кроме Вас. А если вы наследуетесь от чужого класса, который не Object, то такая вероятность точно есть.
Tags:
Hubs:
+9
Comments 9
Comments Comments 9

Articles