8276824: refactor Thread::is_JavaThread_protected

Reviewed-by: coleenp, rehn, dholmes

src/hotspot/share/runtime/handshake.cpp

@@ -355,7 +355,7 @@ void Handshake::execute(HandshakeClosure* hs_cl, ThreadsListHandle* tlh, JavaThr
 
   guarantee(target != nullptr, "must be");
   if (tlh == nullptr) {
-    guarantee(Thread::is_JavaThread_protected(target, /* checkTLHOnly */ true),
+    guarantee(Thread::is_JavaThread_protected_by_TLH(target),
               "missing ThreadsListHandle in calling context.");
     target->handshake_state()->add_operation(&op);
   } else if (tlh->includes(target)) {
@@ -412,7 +412,7 @@ void Handshake::execute(AsyncHandshakeClosure* hs_cl, JavaThread* target) {
   if (current != target) {
     // Another thread is handling the request and it must be protecting
     // the target.
-    guarantee(Thread::is_JavaThread_protected(target, /* checkTLHOnly */ true),
+    guarantee(Thread::is_JavaThread_protected_by_TLH(target),
               "missing ThreadsListHandle in calling context.");
   }
   // Implied else:

src/hotspot/share/runtime/thread.cpp

@@ -434,60 +434,71 @@ void Thread::check_for_dangling_thread_pointer(Thread *thread) {
 #endif
 
 // Is the target JavaThread protected by the calling Thread or by some other
-// mechanism? If checkTLHOnly is true (default is false), then we only check
-// if the target JavaThread is protected by a ThreadsList (if any) associated
-// with the calling Thread.
+// mechanism?
 //
-bool Thread::is_JavaThread_protected(const JavaThread* p, bool checkTLHOnly) {
+bool Thread::is_JavaThread_protected(const JavaThread* target) {
   Thread* current_thread = Thread::current();
-  if (!checkTLHOnly) {
-    // Do the simplest check first:
-    if (SafepointSynchronize::is_at_safepoint()) {
-      // The target is protected since JavaThreads cannot exit
-      // while we're at a safepoint.
-      return true;
-    }
 
-    // If the target hasn't been started yet then it is trivially
-    // "protected". We assume the caller is the thread that will do
-    // the starting.
-    if (p->osthread() == NULL || p->osthread()->get_state() <= INITIALIZED) {
-      return true;
-    }
+  // Do the simplest check first:
+  if (SafepointSynchronize::is_at_safepoint()) {
+    // The target is protected since JavaThreads cannot exit
+    // while we're at a safepoint.
+    return true;
+  }
 
-    // Now make the simple checks based on who the caller is:
-    if (current_thread == p || Threads_lock->owner() == current_thread) {
-      // Target JavaThread is self or calling thread owns the Threads_lock.
-      // Second check is the same as Threads_lock->owner_is_self(),
-      // but we already have the current thread so check directly.
-      return true;
-    }
+  // If the target hasn't been started yet then it is trivially
+  // "protected". We assume the caller is the thread that will do
+  // the starting.
+  if (target->osthread() == NULL || target->osthread()->get_state() <= INITIALIZED) {
+    return true;
+  }
+
+  // Now make the simple checks based on who the caller is:
+  if (current_thread == target || Threads_lock->owner() == current_thread) {
+    // Target JavaThread is self or calling thread owns the Threads_lock.
+    // Second check is the same as Threads_lock->owner_is_self(),
+    // but we already have the current thread so check directly.
+    return true;
+  }
+
+  // Check the ThreadsLists associated with the calling thread (if any)
+  // to see if one of them protects the target JavaThread:
+  if (is_JavaThread_protected_by_TLH(target)) {
+    return true;
   }
 
+  // Use this debug code with -XX:+UseNewCode to diagnose locations that
+  // are missing a ThreadsListHandle or other protection mechanism:
+  // guarantee(!UseNewCode, "current_thread=" INTPTR_FORMAT " is not protecting target="
+  //           INTPTR_FORMAT, p2i(current_thread), p2i(target));
+
+  // Note: Since 'target' isn't protected by a TLH, the call to
+  // target->is_handshake_safe_for() may crash, but we have debug bits so
+  // we'll be able to figure out what protection mechanism is missing.
+  assert(target->is_handshake_safe_for(current_thread), "JavaThread=" INTPTR_FORMAT
+         " is not protected and not handshake safe.", p2i(target));
+
+  // The target JavaThread is not protected so it is not safe to query:
+  return false;
+}
+
+// Is the target JavaThread protected by a ThreadsListHandle (TLH) associated
+// with the calling Thread?
+//
+bool Thread::is_JavaThread_protected_by_TLH(const JavaThread* target) {
+  Thread* current_thread = Thread::current();
+
   // Check the ThreadsLists associated with the calling thread (if any)
   // to see if one of them protects the target JavaThread:
   for (SafeThreadsListPtr* stlp = current_thread->_threads_list_ptr;
        stlp != NULL; stlp = stlp->previous()) {
-    if (stlp->list()->includes(p)) {
+    if (stlp->list()->includes(target)) {
       // The target JavaThread is protected by this ThreadsList:
       return true;
     }
   }
 
-  if (!checkTLHOnly) {
-    // Use this debug code with -XX:+UseNewCode to diagnose locations that
-    // are missing a ThreadsListHandle or other protection mechanism:
-    // guarantee(!UseNewCode, "current_thread=" INTPTR_FORMAT " is not protecting p="
-    //           INTPTR_FORMAT, p2i(current_thread), p2i(p));
-
-    // Note: Since 'p' isn't protected by a TLH, the call to
-    // p->is_handshake_safe_for() may crash, but we have debug bits so
-    // we'll be able to figure out what protection mechanism is missing.
-    assert(p->is_handshake_safe_for(current_thread), "JavaThread=" INTPTR_FORMAT
-           " is not protected and not handshake safe.", p2i(p));
-  }
-
-  // The target JavaThread is not protected so it is not safe to query:
+  // The target JavaThread is not protected by a TLH so it is not safe to query:
   return false;
 }
 
@@ -1747,13 +1758,13 @@ void JavaThread::send_thread_stop(oop java_throwable)  {
 //   - Target thread will not enter any new monitors.
 //
 bool JavaThread::java_suspend() {
-  guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
+  guarantee(Thread::is_JavaThread_protected_by_TLH(/* target */ this),
             "missing ThreadsListHandle in calling context.");
   return this->handshake_state()->suspend();
 }
 
 bool JavaThread::java_resume() {
-  guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
+  guarantee(Thread::is_JavaThread_protected_by_TLH(/* target */ this),
             "missing ThreadsListHandle in calling context.");
   return this->handshake_state()->resume();
 }
@@ -2160,7 +2171,7 @@ void JavaThread::verify() {
 // if vm exit occurs during initialization). These cases can all be accounted
 // for such that this method never returns NULL.
 const char* JavaThread::name() const  {
-  if (Thread::is_JavaThread_protected(this)) {
+  if (Thread::is_JavaThread_protected(/* target */ this)) {
     // The target JavaThread is protected so get_thread_name_string() is safe:
     return get_thread_name_string();
   }

src/hotspot/share/runtime/thread.hpp

@@ -200,10 +200,11 @@ class Thread: public ThreadShadow {
 
  public:
   // Is the target JavaThread protected by the calling Thread or by some other
-  // mechanism? If checkTLHOnly is true (default is false), then we only check
-  // if the target JavaThread is protected by a ThreadsList (if any) associated
-  // with the calling Thread.
-  static bool is_JavaThread_protected(const JavaThread* p, bool checkTLHOnly = false);
+  // mechanism?
+  static bool is_JavaThread_protected(const JavaThread* target);
+  // Is the target JavaThread protected by a ThreadsListHandle (TLH) associated
+  // with the calling Thread?
+  static bool is_JavaThread_protected_by_TLH(const JavaThread* target);
 
   void* operator new(size_t size) throw() { return allocate(size, true); }
   void* operator new(size_t size, const std::nothrow_t& nothrow_constant) throw() {