RFR: 8154715: Missing destructor and/or TLS clearing calls for terminating threads

[David Holmes]

PING!

David

On 4/05/2016 9:39 AM, David Holmes 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