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
