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

[Daniel D. Daugherty]

On 5/10/16 8:17 PM, David Holmes wrote:
> On 11/05/2016 6:28 AM, Brian Gardner wrote:
>> I really like the rename of java_start to native_thread_entry, it makes
>> things make more sense.  It looks like os_bsd.cpp and os_linux.cpp are
>> both missing the clear_thread_current logic after thread->run().
>
> Doh! Thanks. That's what I get for trying to multi-task :)
>
> Webrev updated in place.
>
> Can I get another review from someone in hotspot please. Dan is 
> unfortunately (for me) away.

Sorry about that. Had end of year choir and graduation things to
take care of... :-)

 > so anyone introducing their deletion is informed by a guarantee(false)

In recent reviews, folks have been asking that we use

    fatal("my message");

instead of:

    guarantee(false, "my message");

I see the changeset is already pushed so don't worry about it for this one.

Dan


>
> Thanks,
> David
>
>> + // If a thread has not deleted itself ("delete this") as part of its
>> + // termination sequence, we have to ensure thread-local-storage is
>> + // cleared before we actually terminate. No threads should ever be
>> + // deleted asynchronously with respect to their termination.
>> + if (Thread::current_or_null_safe() != NULL) {
>> + assert(Thread::current_or_null_safe() == thread, "current thread is
>> wrong");
>> + thread->clear_thread_current();
>> + }
>> +
>>
>>
>>> On May 9, 2016, at 5:33 PM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>> Okay here is version 2:
>>>
>>> http://cr.openjdk.java.net/~dholmes/8154715/webrev.v2/
>>>
>>> Lots of cosmetic changes but only a couple of functional ones:
>>>
>>> -  After thread->run() returns we clear the TLS by calling
>>> clear_thread_current(), but only for threads where it has not already
>>> been cleared - as those threads may already have been deleted so we
>>> can't dereference 'thread'
>>>
>>> - No asynchronous thread deletion is permitted, and we avoid races
>>> with VM termination. This means the VMThread no longer gets deleted -
>>> that should not be an issue as many threads do not get deleted when
>>> the VM terminates. I added destructors for the VMThread and
>>> WatcherThread so anyone introducing their deletion is informed by a
>>> guarantee(false)
>>>
>>>
>>> Cosmetic changes:
>>>
>>> - renamed java_start to thread_native_entry (it is used by all threads
>>> not just "java" ones, so this avoids potential confusion)
>>>
>>> - updated os::free_thread to always assume it works on the current
>>> thread (and add assert to verify that)
>>>
>>> Thanks,
>>> David
>>