RFR: 8154715: Missing destructor and/or TLS clearing calls for terminating threads
stefan.karlsson at oracle.com
Tue May 10 18:59:48 UTC 2016
On 10/05/16 12:17, David Holmes wrote:
> 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:
>>> 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
>> 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
> 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.
I was more thinking about the contents of the following memory:
The amount of memory that we don't hand back is about 1500 bytes for the
VMThread. Most comes from the resource_area(), which is initialized to
(init_size = 1*K - slack) in the Thread constructor.
The amount of memory doesn't seem to be out-of-the ordinary, so leaking
the VMThread doesn't seem to be worse than leaking the other threads, as
The patch looks good to me, but you probably want to get a Review by
someone well-versed in the HotSpot threading system.
>>> 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)
More information about the hotspot-dev