8226228: Make Threads_lock an always safepoint checked lock

Reviewed-by: coleenp, dcubed, dholmes

src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp

@@ -516,7 +516,7 @@ void JfrThreadSampler::task_stacktrace(JfrSampleType type, JavaThread** last_thr
     elapsedTimer sample_time;
     sample_time.start();
     {
-      MutexLocker tlock(Threads_lock, Mutex::_no_safepoint_check_flag);
+      MutexLocker tlock(Threads_lock);
       ThreadsListHandle tlh;
       // Resolve a sample session relative start position index into the thread list array.
       // In cases where the last sampled thread is NULL or not-NULL but stale, find_index() returns -1.

src/hotspot/share/prims/jni.cpp

@@ -4135,14 +4135,13 @@ static jint attach_current_thread(JavaVM *vm, void **penv, void *_args, bool dae
 
   thread->cache_global_variables();
 
-  // Crucial that we do not have a safepoint check for this thread, since it has
+  // This thread will not do a safepoint check, since it has
   // not been added to the Thread list yet.
-  { Threads_lock->lock_without_safepoint_check();
+  { MutexLocker ml(Threads_lock);
     // This must be inside this lock in order to get FullGCALot to work properly, i.e., to
     // avoid this thread trying to do a GC before it is added to the thread-list
     thread->set_active_handles(JNIHandleBlock::allocate_block());
     Threads::add(thread, daemon);
-    Threads_lock->unlock();
   }
   // Create thread group and name info from attach arguments
   oop group = NULL;

src/hotspot/share/runtime/handshake.cpp

@@ -137,7 +137,7 @@ class VM_HandshakeOneThread: public VM_Handshake {
       // There is an assumption in the code that the Threads_lock should be
       // locked during certain phases.
       {
-        MutexLocker ml(Threads_lock, Mutex::_no_safepoint_check_flag);
+        MutexLocker ml(Threads_lock);
         _target->handshake_process_by_vmthread();
       }
     } while (!poll_for_completed_thread());
@@ -186,7 +186,7 @@ class VM_HandshakeAllThreads: public VM_Handshake {
           // There is an assumption in the code that the Threads_lock should
           // be locked during certain phases.
           jtiwh.rewind();
-          MutexLocker ml(Threads_lock, Mutex::_no_safepoint_check_flag);
+          MutexLocker ml(Threads_lock);
           for (JavaThread *thr = jtiwh.next(); thr != NULL; thr = jtiwh.next()) {
             // A new thread on the ThreadsList will not have an operation,
             // hence it is skipped in handshake_process_by_vmthread.

src/hotspot/share/runtime/mutex.cpp

@@ -37,7 +37,7 @@ void Monitor::check_safepoint_state(Thread* thread, bool do_safepoint_check) {
   // If the JavaThread checks for safepoint, verify that the lock wasn't created with safepoint_check_never.
   SafepointCheckRequired not_allowed = do_safepoint_check ?  Monitor::_safepoint_check_never :
                                                              Monitor::_safepoint_check_always;
-  assert(!thread->is_Java_thread() || _safepoint_check_required != not_allowed,
+  assert(!thread->is_active_Java_thread() || _safepoint_check_required != not_allowed,
          "This lock should %s have a safepoint check for Java threads: %s",
          _safepoint_check_required ? "always" : "never", name());
 
@@ -52,7 +52,7 @@ void Monitor::lock(Thread * self) {
 
 #ifdef CHECK_UNHANDLED_OOPS
   // Clear unhandled oops in JavaThreads so we get a crash right away.
-  if (self->is_Java_thread()) {
+  if (self->is_active_Java_thread()) {
     self->clear_unhandled_oops();
   }
 #endif // CHECK_UNHANDLED_OOPS
@@ -62,6 +62,7 @@ void Monitor::lock(Thread * self) {
 
   Monitor* in_flight_monitor = NULL;
   DEBUG_ONLY(int retry_cnt = 0;)
+  bool is_active_Java_thread = self->is_active_Java_thread();
   while (!_lock.try_lock()) {
     // The lock is contended
 
@@ -72,7 +73,8 @@ void Monitor::lock(Thread * self) {
     }
   #endif // ASSERT
 
-    if (self->is_Java_thread()) {
+    // Is it a JavaThread participating in the safepoint protocol.
+    if (is_active_Java_thread) {
       assert(rank() > Mutex::special, "Potential deadlock with special or lesser rank mutex");
       { ThreadBlockInVMWithDeadlockCheck tbivmdc((JavaThread *) self, &in_flight_monitor);
         in_flight_monitor = this;  // save for ~ThreadBlockInVMWithDeadlockCheck
@@ -190,8 +192,8 @@ bool Monitor::wait(long timeout, bool as_suspend_equivalent) {
 
   assert_owner(self);
 
-  // Safepoint checking logically implies java_thread
-  guarantee(self->is_Java_thread(), "invariant");
+  // Safepoint checking logically implies an active JavaThread.
+  guarantee(self->is_active_Java_thread(), "invariant");
   assert_wait_lock_state(self);
 
 #ifdef CHECK_UNHANDLED_OOPS
@@ -470,7 +472,7 @@ void Monitor::set_owner_implementation(Thread *new_owner) {
 // Factored out common sanity checks for locking mutex'es. Used by lock() and try_lock()
 void Monitor::check_prelock_state(Thread *thread, bool safepoint_check) {
   if (safepoint_check) {
-    assert((!thread->is_Java_thread() || ((JavaThread *)thread)->thread_state() == _thread_in_vm)
+    assert((!thread->is_active_Java_thread() || ((JavaThread *)thread)->thread_state() == _thread_in_vm)
            || rank() == Mutex::special, "wrong thread state for using locks");
     if (thread->is_VM_thread() && !allow_vm_block()) {
       fatal("VM thread using lock %s (not allowed to block on)", name());

src/hotspot/share/runtime/mutexLocker.cpp

@@ -287,7 +287,7 @@ void mutex_init() {
   // CMS_bitMap_lock                          leaf 1
   // CMS_freeList_lock                        leaf 2
 
-  def(Threads_lock                 , PaddedMonitor, barrier,     true,  Monitor::_safepoint_check_sometimes);  // Used for safepoint protocol.
+  def(Threads_lock                 , PaddedMonitor, barrier,     true,  Monitor::_safepoint_check_always);  // Used for safepoint protocol.
   def(NonJavaThreadsList_lock      , PaddedMutex,   leaf,        true,  Monitor::_safepoint_check_never);
   def(NonJavaThreadsListSync_lock  , PaddedMutex,   leaf,        true,  Monitor::_safepoint_check_never);
 

src/hotspot/share/runtime/thread.cpp

@@ -1804,7 +1804,7 @@ void JavaThread::block_if_vm_exited() {
   if (_terminated == _vm_exited) {
     // _vm_exited is set at safepoint, and Threads_lock is never released
     // we will block here forever
-    Threads_lock->lock_without_safepoint_check();
+    Threads_lock->lock();
     ShouldNotReachHere();
   }
 }

src/hotspot/share/runtime/thread.hpp

@@ -491,6 +491,10 @@ class Thread: public ThreadShadow {
   // Can this thread make Java upcalls
   virtual bool can_call_java() const                 { return false; }
 
+  // Is this a JavaThread that is on the VM's current ThreadsList?
+  // If so it must participate in the safepoint protocol.
+  virtual bool is_active_Java_thread() const         { return false; }
+
   // Casts
   virtual WorkerThread* as_Worker_thread() const     { return NULL; }
 
@@ -1247,6 +1251,10 @@ class JavaThread: public Thread {
   virtual bool is_Java_thread() const            { return true;  }
   virtual bool can_call_java() const             { return true; }
 
+  virtual bool is_active_Java_thread() const {
+    return on_thread_list() && !is_terminated();
+  }
+
   // Thread oop. threadObj() can be NULL for initial JavaThread
   // (or for threads attached via JNI)
   oop threadObj() const                          { return _threadObj; }

src/hotspot/share/runtime/threadSMR.cpp

@@ -942,9 +942,9 @@ void ThreadsSMRSupport::smr_delete(JavaThread *thread) {
 
   while (true) {
     {
-      // No safepoint check because this JavaThread is not on the
-      // Threads list.
-      MutexLocker ml(Threads_lock, Mutex::_no_safepoint_check_flag);
+      // Will not make a safepoint check because this JavaThread
+      // is not on the current ThreadsList.
+      MutexLocker ml(Threads_lock);
       // Cannot use a MonitorLocker helper here because we have
       // to drop the Threads_lock first if we wait.
       ThreadsSMRSupport::delete_lock()->lock_without_safepoint_check();

src/hotspot/share/runtime/vmOperations.cpp

@@ -522,7 +522,7 @@ void VM_Exit::wait_if_vm_exited() {
       Thread::current_or_null() != _shutdown_thread) {
     // _vm_exited is set at safepoint, and the Threads_lock is never released
     // we will block here until the process dies
-    Threads_lock->lock_without_safepoint_check();
+    Threads_lock->lock();
     ShouldNotReachHere();
   }
 }

src/hotspot/share/utilities/vmError.cpp

@@ -1797,11 +1797,14 @@ void VMError::controlled_crash(int how) {
   // Case 16 is tested by test/hotspot/jtreg/runtime/ErrorHandling/ThreadsListHandleInErrorHandlingTest.java.
   // Case 17 is tested by test/hotspot/jtreg/runtime/ErrorHandling/NestedThreadsListHandleInErrorHandlingTest.java.
 
-  // We grab Threads_lock to keep ThreadsSMRSupport::print_info_on()
+  // We try to grab Threads_lock to keep ThreadsSMRSupport::print_info_on()
   // from racing with Threads::add() or Threads::remove() as we
   // generate the hs_err_pid file. This makes our ErrorHandling tests
   // more stable.
-  MutexLocker ml(Threads_lock->owned_by_self() ? NULL : Threads_lock, Mutex::_no_safepoint_check_flag);
+  if (!Threads_lock->owned_by_self()) {
+    Threads_lock->try_lock();
+    // The VM is going to die so no need to unlock Thread_lock.
+  }
 
   switch (how) {
     case  1: vmassert(str == NULL, "expected null"); break;