On 5/10/16 8:17 PM, 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.

Sorry about that. Had end of year choir and graduation things to
take care of... :-)

> so anyone introducing their deletion is informed by a guarantee(false)

In recent reviews, folks have been asking that we use

   fatal("my message");

instead of:

   guarantee(false, "my message");

I see the changeset is already pushed so don't worry about it for this one.

Dan



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.hol...@oracle.com
<mailto:david.hol...@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



Reply via email to