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

[serguei.spitsyn at oracle.com]

On 5/10/16 21:47, David Holmes wrote:
> On 11/05/2016 1:42 PM, serguei.spitsyn at oracle.com wrote:
>> David,
>>
>> It looks good and safe.
>
> Thanks Serguei!
>
>> Is there anything to do for the closed code as well?
>
> No.

Ok.

Thanks,
Serguei


>
> David
>
>> Thanks,
>> Serguei
>>
>>
>> On 5/10/16 19:17, 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.
>>>
>>> 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
>>>>
>>