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

Недавно я наткнулся на интересную особенность синхронизации на классе 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, то такая вероятность точно есть.