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

[David Holmes]

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.

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
>