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