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

[David Holmes]

Hi Stefan,

Thanks for looking at this again.

On 10/05/2016 6:24 PM, Stefan Karlsson wrote:
> Hi David,
>
> On 10/05/16 02:33, David Holmes 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)
>
> This makes the model easier to understand, IMHO. Either you delete the
> thread from the run() method, or you don't delete it at all.
>
> I's there a way to determine how much memory we leak by not deleting the
> memory owned by the VMThread instance? I'm a bit worried that the
> VMThread might use more resources than the other threads we don't delete.

There is nothing at all special about the VMThread instance, it is just 
a NamedThread with nothing being referenced by instance fields beyond 
the name. Any runtime resources are released as the thread terminates - 
and nothing special there either AFAICS.

Thanks,
David

> Thanks,
> StefanK
>
>>
>>
>> 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
>