After applying your patches I still saw the cleanup messages from pthread. After comparing my original patch to yours and doing some testing it looks like we missed a couple spots.
The warning went away after I applied the following patches: http://brian.timestudybuddy.com/webrev/hotspot__clear_thread_current/webrev/hotspot/src/share/vm/gc/shared/concurrentGCThread.cpp.cdiff.html <http://brian.timestudybuddy.com/webrev/hotspot__clear_thread_current/webrev/hotspot/src/share/vm/gc/shared/concurrentGCThread.cpp.cdiff.html> http://brian.timestudybuddy.com/webrev/hotspot__clear_thread_current/webrev/hotspot/src/share/vm/gc/cms/concurrentMarkSweepThread.cpp.cdiff.html <http://brian.timestudybuddy.com/webrev/hotspot__clear_thread_current/webrev/hotspot/src/share/vm/gc/cms/concurrentMarkSweepThread.cpp.cdiff.html> Brian > On May 3, 2016, at 4:39 PM, David Holmes <david.hol...@oracle.com> wrote: > > This needs attention from GC and runtime folk please. > > bug: https://bugs.openjdk.java.net/browse/JDK-8154715 > webrev: http://cr.openjdk.java.net/~dholmes/8154715/webrev/ > > tl;dr: ensure ThreadLocalStorage::set_thread(NULL) is always called before a > thread terminates. > > Background: > > Most system-related threads do not expect to explicitly terminate, except > sometimes as part of VM termination. Such threads don't have their > destructors called, but should. > > This omission came to light due to the ThreadLocalStorage changes in > JDK-8132510. As part of that change we deleted the following from the > termination path of the VMThread: > > // Thread destructor usually does this. > ThreadLocalStorage::set_thread(NULL); > > The clearing of TLS seemed irrelevant to the VMThread as it primarily is used > to aid in JNI attach/detach. However Brian Gardner reported: > > http://mail.openjdk.java.net/pipermail/bsd-port-dev/2016-February/002788.html > > a problem on FreeBSD caused by this change and the interaction with the POSIX > pthread TLS destructor use introduced by JDK-8033696. Because the VMThread > terminated without clearing TLS, when the TLS-destructor was called it got > into a loop which ran four times (as happens on Linux) and then prints a > warning to the console (which doesn't happen on Linux). > > This indicates we need to restore the: > > ThreadLocalStorage::set_thread(NULL); > > but on further consideration it seems to me that this is not confined to the > VMThread, and the most appropriate fix would be to always invoke the Thread > destructor as a thread terminates. > > Solution: > > Further investigation shows that calling the Thread destructor in the thread > as it terminates is not possible: > > - VMThread > > This is actually destroyed by the thread that terminates the VM, but that can > happen after it terminates and so we still hit the TLS problem. The VMThread > may be able to destroy itself today but in the past this was not possible > (see existing code comment), and in the future it may also not be possible - > the problem is that the Thread destructor can interact with other VM > subsystems that are concurrently being torn down by the thread that is > terminating the VM. In the past this was the CodeHeap. So rather than > introduce something that is fragile we stick with the current scheme but > restore the ThreadLocalStorage::set_thread(NULL); - note we can't access > "this" at that time because it may already have been de-allocated. > > - WatcherThread > > The WatcherThread is never destroyed today but has the same problem as the > VMThread. We can call the destructor from the VM termination thread (and have > implemented that), but not from the WatcherThread itself. So again we just > have to restore the ThreadLocalStorage::set_thread(NULL); to fix the > potential TLS problem. > > - GC Threads > > There are two cases: > > a) GC threads that never terminate > > For these we don't need to do anything: we can't delete the thread as it > never terminates and we don't hit the TLS problem because it never > terminates. So all we will do here is add some logic to check (in > NON_PRODUCT) that we do in fact never terminate. > > b) GC threads that can terminate > > Despite the fact the threads can terminate, references to those threads are > stored elsewhere (WorkGangs and other places) and are not cleared as part of > the termination process. Those references can be touched after the thread has > terminated so we can not call the destructor at all. So again all we can do > (without some major thread management reworking) is ensure that > ThreadLocalStorage::set_thread(NULL); is called before the thread actually > terminates > > Testing: JPRT > RBT - runtime nightly tests > > Thanks, > David >
