8213150: Add verification for locking by VMThread
authorcoleenp
Tue, 24 Sep 2019 10:12:56 -0400
changeset 58291 a013100f7a35
parent 58289 3a79d4cccbcb
child 58292 a8f06f2b84b0
8213150: Add verification for locking by VMThread Summary: extend verification for all locking not just VMOperations, and fix CLDG lock to not be taken by VM thread. Reviewed-by: rehn, dholmes
src/hotspot/share/classfile/classLoaderDataGraph.cpp
src/hotspot/share/classfile/classLoaderDataGraph.hpp
src/hotspot/share/gc/shared/memAllocator.cpp
src/hotspot/share/oops/constantPool.cpp
src/hotspot/share/runtime/mutex.cpp
src/hotspot/share/runtime/mutex.hpp
src/hotspot/share/runtime/mutexLocker.cpp
src/hotspot/share/runtime/mutexLocker.hpp
src/hotspot/share/runtime/synchronizer.cpp
src/hotspot/share/runtime/thread.cpp
src/hotspot/share/runtime/thread.hpp
src/hotspot/share/runtime/vmThread.cpp
src/hotspot/share/utilities/concurrentHashTable.inline.hpp
src/hotspot/share/utilities/events.hpp
--- a/src/hotspot/share/classfile/classLoaderDataGraph.cpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/classfile/classLoaderDataGraph.cpp	Tue Sep 24 10:12:56 2019 -0400
@@ -285,17 +285,27 @@
   }
 }
 
-// Closure for locking and iterating through classes.
-LockedClassesDo::LockedClassesDo(classes_do_func_t f) : _function(f) {
-  ClassLoaderDataGraph_lock->lock();
+// Closure for locking and iterating through classes. Only lock outside of safepoint.
+LockedClassesDo::LockedClassesDo(classes_do_func_t f) : _function(f),
+  _do_lock(!SafepointSynchronize::is_at_safepoint()) {
+  if (_do_lock) {
+    ClassLoaderDataGraph_lock->lock();
+  }
 }
 
-LockedClassesDo::LockedClassesDo() : _function(NULL) {
+LockedClassesDo::LockedClassesDo() : _function(NULL),
+  _do_lock(!SafepointSynchronize::is_at_safepoint()) {
   // callers provide their own do_klass
-  ClassLoaderDataGraph_lock->lock();
+  if (_do_lock) {
+    ClassLoaderDataGraph_lock->lock();
+  }
 }
 
-LockedClassesDo::~LockedClassesDo() { ClassLoaderDataGraph_lock->unlock(); }
+LockedClassesDo::~LockedClassesDo() {
+  if (_do_lock) {
+    ClassLoaderDataGraph_lock->unlock();
+  }
+}
 
 
 // Iterating over the CLDG needs to be locked because
--- a/src/hotspot/share/classfile/classLoaderDataGraph.hpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/classfile/classLoaderDataGraph.hpp	Tue Sep 24 10:12:56 2019 -0400
@@ -156,6 +156,7 @@
 class LockedClassesDo : public KlassClosure {
   typedef void (*classes_do_func_t)(Klass*);
   classes_do_func_t _function;
+  bool _do_lock;
 public:
   LockedClassesDo();  // For callers who provide their own do_klass
   LockedClassesDo(classes_do_func_t function);
--- a/src/hotspot/share/gc/shared/memAllocator.cpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/gc/shared/memAllocator.cpp	Tue Sep 24 10:12:56 2019 -0400
@@ -171,9 +171,8 @@
   // This is a VM policy failure, so how do we exhaustively test it?
   assert(!_thread->has_pending_exception(),
          "shouldn't be allocating with pending exception");
-  // Allocation of an oop can always invoke a safepoint,
-  // hence, the true argument.
-  _thread->check_for_valid_safepoint_state(true);
+  // Allocation of an oop can always invoke a safepoint.
+  _thread->check_for_valid_safepoint_state();
 }
 #endif
 
--- a/src/hotspot/share/oops/constantPool.cpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/oops/constantPool.cpp	Tue Sep 24 10:12:56 2019 -0400
@@ -556,7 +556,8 @@
     Handle h_loader (thread, loader);
     Klass* k = SystemDictionary::find(name, h_loader, h_prot, thread);
 
-    if (k != NULL) {
+    // Avoid constant pool verification at a safepoint, which takes the Module_lock.
+    if (k != NULL && !SafepointSynchronize::is_at_safepoint()) {
       // Make sure that resolving is legal
       EXCEPTION_MARK;
       // return NULL if verification fails
--- a/src/hotspot/share/runtime/mutex.cpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/runtime/mutex.cpp	Tue Sep 24 10:12:56 2019 -0400
@@ -33,24 +33,46 @@
 #include "utilities/macros.hpp"
 
 #ifdef ASSERT
-void Mutex::check_safepoint_state(Thread* thread, bool do_safepoint_check) {
+void Mutex::check_block_state(Thread* thread) {
+  if (!_allow_vm_block && thread->is_VM_thread()) {
+    // JavaThreads are checked to make sure that they do not hold _allow_vm_block locks during operations
+    // that could safepoint.  Make sure the vm thread never uses locks with _allow_vm_block == false.
+    fatal("VM thread could block on lock that may be held by a JavaThread during safepoint: %s", name());
+  }
+
+  assert(!os::ThreadCrashProtection::is_crash_protected(thread),
+         "locking not allowed when crash protection is set");
+}
+
+void Mutex::check_safepoint_state(Thread* thread) {
+  check_block_state(thread);
+
   // If the JavaThread checks for safepoint, verify that the lock wasn't created with safepoint_check_never.
-  SafepointCheckRequired not_allowed = do_safepoint_check ?  Mutex::_safepoint_check_never :
-                                                             Mutex::_safepoint_check_always;
-  assert(!thread->is_active_Java_thread() || _safepoint_check_required != not_allowed,
+  if (thread->is_active_Java_thread()) {
+    assert(_safepoint_check_required != _safepoint_check_never,
+           "This lock should %s have a safepoint check for Java threads: %s",
+           _safepoint_check_required ? "always" : "never", name());
+
+    // Also check NoSafepointVerifier, and thread state is _thread_in_vm
+    thread->check_for_valid_safepoint_state();
+  } else {
+    // If initialized with safepoint_check_never, a NonJavaThread should never ask to safepoint check either.
+    assert(_safepoint_check_required != _safepoint_check_never,
+           "NonJavaThread should not check for safepoint");
+  }
+}
+
+void Mutex::check_no_safepoint_state(Thread* thread) {
+  check_block_state(thread);
+  assert(!thread->is_active_Java_thread() || _safepoint_check_required != _safepoint_check_always,
          "This lock should %s have a safepoint check for Java threads: %s",
          _safepoint_check_required ? "always" : "never", name());
-
-  // If defined with safepoint_check_never, a NonJavaThread should never ask to safepoint check either.
-  assert(thread->is_Java_thread() || !do_safepoint_check || _safepoint_check_required != Mutex::_safepoint_check_never,
-         "NonJavaThread should not check for safepoint");
 }
 #endif // ASSERT
 
-void Mutex::lock(Thread * self) {
-  check_safepoint_state(self, true);
+void Mutex::lock(Thread* self) {
+  check_safepoint_state(self);
 
-  DEBUG_ONLY(check_prelock_state(self, true));
   assert(_owner != self, "invariant");
 
   Mutex* in_flight_mutex = NULL;
@@ -60,7 +82,6 @@
     // The lock is contended
 
   #ifdef ASSERT
-    check_block_state(self);
     if (retry_cnt++ > 3) {
       log_trace(vmmutex)("JavaThread " INTPTR_FORMAT " on %d attempt trying to acquire vmmutex %s", p2i(self), retry_cnt, _name);
     }
@@ -98,7 +119,7 @@
 // in the wrong way this can lead to a deadlock with the safepoint code.
 
 void Mutex::lock_without_safepoint_check(Thread * self) {
-  check_safepoint_state(self, false);
+  check_no_safepoint_state(self);
   assert(_owner != self, "invariant");
   _lock.lock();
   assert_owner(NULL);
@@ -114,8 +135,9 @@
 
 bool Mutex::try_lock() {
   Thread * const self = Thread::current();
-  DEBUG_ONLY(check_prelock_state(self, false);)
-
+  // Some safepoint_check_always locks use try_lock, so cannot check
+  // safepoint state, but can check blocking state.
+  check_block_state(self);
   if (_lock.try_lock()) {
     assert_owner(NULL);
     set_owner(self);
@@ -160,7 +182,6 @@
 
 bool Monitor::wait_without_safepoint_check(long timeout) {
   Thread* const self = Thread::current();
-  check_safepoint_state(self, false);
 
   // timeout is in milliseconds - with zero meaning never timeout
   assert(timeout >= 0, "negative timeout");
@@ -171,6 +192,9 @@
   // conceptually set the owner to NULL in anticipation of
   // abdicating the lock in wait
   set_owner(NULL);
+  // Check safepoint state after resetting owner and possible NSV.
+  check_no_safepoint_state(self);
+
   int wait_status = _lock.wait(timeout);
   set_owner(self);
   return wait_status != 0;          // return true IFF timeout
@@ -178,7 +202,6 @@
 
 bool Monitor::wait(long timeout, bool as_suspend_equivalent) {
   Thread* const self = Thread::current();
-  check_safepoint_state(self, true);
 
   // timeout is in milliseconds - with zero meaning never timeout
   assert(timeout >= 0, "negative timeout");
@@ -193,6 +216,8 @@
   // conceptually set the owner to NULL in anticipation of
   // abdicating the lock in wait
   set_owner(NULL);
+  // Check safepoint state after resetting owner and possible NSV.
+  check_safepoint_state(self);
   JavaThread *jt = (JavaThread *)self;
   Mutex* in_flight_mutex = NULL;
 
@@ -255,7 +280,7 @@
   _rank            = Rank;
   _safepoint_check_required = safepoint_check_required;
 
-  assert(_safepoint_check_required != Mutex::_safepoint_check_sometimes || is_sometimes_ok(name),
+  assert(_safepoint_check_required != _safepoint_check_sometimes || is_sometimes_ok(name),
          "Lock has _safepoint_check_sometimes %s", name);
 #endif
 }
@@ -278,15 +303,27 @@
 // Non-product code
 
 #ifndef PRODUCT
+const char* print_safepoint_check(Mutex::SafepointCheckRequired safepoint_check) {
+  switch (safepoint_check) {
+  case Mutex::_safepoint_check_never:     return "safepoint_check_never";
+  case Mutex::_safepoint_check_sometimes: return "safepoint_check_sometimes";
+  case Mutex::_safepoint_check_always:    return "safepoint_check_always";
+  default: return "";
+  }
+}
+
 void Mutex::print_on(outputStream* st) const {
-  st->print_cr("Mutex: [" PTR_FORMAT "] %s - owner: " PTR_FORMAT,
-               p2i(this), _name, p2i(_owner));
+  st->print("Mutex: [" PTR_FORMAT "] %s - owner: " PTR_FORMAT,
+            p2i(this), _name, p2i(_owner));
+  if (_allow_vm_block) {
+    st->print("%s", " allow_vm_block");
+  }
+  st->print(" %s", print_safepoint_check(_safepoint_check_required));
+  st->cr();
 }
 #endif
 
-#ifndef PRODUCT
 #ifdef ASSERT
-
 void Mutex::assert_owner(Thread * expected) {
   const char* msg = "invalid owner";
   if (expected == NULL) {
@@ -340,7 +377,6 @@
   return res;
 }
 
-
 bool Mutex::contains(Mutex* locks, Mutex* lock) {
   for (; locks != NULL; locks = locks->next()) {
     if (locks == lock) {
@@ -349,7 +385,27 @@
   }
   return false;
 }
-#endif
+
+// NSV implied with locking allow_vm_block or !safepoint_check locks.
+void Mutex::no_safepoint_verifier(Thread* thread, bool enable) {
+  // Threads_lock is special, since the safepoint synchronization will not start before this is
+  // acquired. Hence, a JavaThread cannot be holding it at a safepoint. So is VMOperationRequest_lock,
+  // since it is used to transfer control between JavaThreads and the VMThread
+  // Do not *exclude* any locks unless you are absolutely sure it is correct. Ask someone else first!
+  if ((_allow_vm_block &&
+       this != Threads_lock &&
+       this != Compile_lock &&           // Temporary: should not be necessary when we get separate compilation
+       this != tty_lock &&               // The tty_lock is released for the safepoint.
+       this != VMOperationRequest_lock &&
+       this != VMOperationQueue_lock) ||
+       rank() == Mutex::special) {
+    if (enable) {
+      thread->_no_safepoint_count++;
+    } else {
+      thread->_no_safepoint_count--;
+    }
+  }
+}
 
 // Called immediately after lock acquisition or release as a diagnostic
 // to track the lock-set of the thread and test for rank violations that
@@ -376,7 +432,6 @@
 
     // link "this" into the owned locks list
 
-#ifdef ASSERT  // Thread::_owned_locks is under the same ifdef
     Mutex* locks = get_least_ranked_lock(new_owner->owned_locks());
     // Mutex::set_owner_implementation is a friend of Thread
 
@@ -401,20 +456,21 @@
 
     this->_next = new_owner->_owned_locks;
     new_owner->_owned_locks = this;
-#endif
+
+    // NSV implied with locking allow_vm_block flag.
+    no_safepoint_verifier(new_owner, true);
 
   } else {
     // the thread is releasing this lock
 
     Thread* old_owner = _owner;
-    DEBUG_ONLY(_last_owner = old_owner;)
+    _last_owner = old_owner;
 
     assert(old_owner != NULL, "removing the owner thread of an unowned mutex");
     assert(old_owner == Thread::current(), "removing the owner thread of an unowned mutex");
 
     _owner = NULL; // set the owner
 
-#ifdef ASSERT
     Mutex* locks = old_owner->owned_locks();
 
     // remove "this" from the owned locks list
@@ -434,33 +490,9 @@
       prev->_next = _next;
     }
     _next = NULL;
-#endif
+
+    // ~NSV implied with locking allow_vm_block flag.
+    no_safepoint_verifier(old_owner, false);
   }
 }
-
-
-// Factored out common sanity checks for locking mutex'es. Used by lock() and try_lock()
-void Mutex::check_prelock_state(Thread *thread, bool safepoint_check) {
-  if (safepoint_check) {
-    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());
-    }
-    DEBUG_ONLY(if (rank() != Mutex::special) \
-               thread->check_for_valid_safepoint_state(false);)
-  }
-  assert(!os::ThreadCrashProtection::is_crash_protected(thread),
-         "locking not allowed when crash protection is set");
-}
-
-void Mutex::check_block_state(Thread *thread) {
-  if (!_allow_vm_block && thread->is_VM_thread()) {
-    warning("VM thread blocked on lock");
-    print();
-    BREAKPOINT;
-  }
-  assert(_owner != thread, "deadlock: blocking on mutex owned by current thread");
-}
-
-#endif // PRODUCT
+#endif // ASSERT
--- a/src/hotspot/share/runtime/mutex.hpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/runtime/mutex.hpp	Tue Sep 24 10:12:56 2019 -0400
@@ -81,21 +81,22 @@
   char _name[MUTEX_NAME_LEN];            // Name of mutex/monitor
 
   // Debugging fields for naming, deadlock detection, etc. (some only used in debug mode)
-#ifndef PRODUCT
-  bool      _allow_vm_block;
-  DEBUG_ONLY(int _rank;)                 // rank (to avoid/detect potential deadlocks)
-  DEBUG_ONLY(Mutex* _next;)              // Used by a Thread to link up owned locks
-  DEBUG_ONLY(Thread* _last_owner;)       // the last thread to own the lock
-  DEBUG_ONLY(static bool contains(Mutex* locks, Mutex* lock);)
-  DEBUG_ONLY(static Mutex* get_least_ranked_lock(Mutex* locks);)
-  DEBUG_ONLY(Mutex* get_least_ranked_lock_besides_this(Mutex* locks);)
-#endif
+#ifdef ASSERT
+  bool    _allow_vm_block;
+  int     _rank;                 // rank (to avoid/detect potential deadlocks)
+  Mutex*  _next;                 // Used by a Thread to link up owned locks
+  Thread* _last_owner;           // the last thread to own the lock
+  static bool contains(Mutex* locks, Mutex* lock);
+  static Mutex* get_least_ranked_lock(Mutex* locks);
+  Mutex* get_least_ranked_lock_besides_this(Mutex* locks);
+#endif  // ASSERT
 
-  void set_owner_implementation(Thread* owner)                        PRODUCT_RETURN;
-  void check_prelock_state     (Thread* thread, bool safepoint_check) PRODUCT_RETURN;
-  void check_block_state       (Thread* thread)                       PRODUCT_RETURN;
-  void check_safepoint_state   (Thread* thread, bool safepoint_check) NOT_DEBUG_RETURN;
+  void set_owner_implementation(Thread* owner)                        NOT_DEBUG({ _owner = owner;});
+  void check_block_state       (Thread* thread)                       NOT_DEBUG_RETURN;
+  void check_safepoint_state   (Thread* thread)                       NOT_DEBUG_RETURN;
+  void check_no_safepoint_state(Thread* thread)                       NOT_DEBUG_RETURN;
   void assert_owner            (Thread* expected)                     NOT_DEBUG_RETURN;
+  void no_safepoint_verifier   (Thread* thread, bool enable)          NOT_DEBUG_RETURN;
 
  public:
   enum {
@@ -171,21 +172,16 @@
   #ifndef PRODUCT
     void print_on(outputStream* st) const;
     void print() const                      { print_on(::tty); }
-    DEBUG_ONLY(int    rank() const          { return _rank; })
-    bool   allow_vm_block()                 { return _allow_vm_block; }
-
-    DEBUG_ONLY(Mutex *next()  const         { return _next; })
-    DEBUG_ONLY(void   set_next(Mutex *next) { _next = next; })
   #endif
+  #ifdef ASSERT
+    int    rank() const          { return _rank; }
+    bool   allow_vm_block()      { return _allow_vm_block; }
 
-  void set_owner(Thread* owner) {
-  #ifndef PRODUCT
-    set_owner_implementation(owner);
-    DEBUG_ONLY(void verify_mutex(Thread* thr);)
-  #else
-    _owner = owner;
-  #endif
-  }
+    Mutex *next()  const         { return _next; }
+    void   set_next(Mutex *next) { _next = next; }
+  #endif // ASSERT
+
+  void set_owner(Thread* owner)             { set_owner_implementation(owner); }
 };
 
 class Monitor : public Mutex {
--- a/src/hotspot/share/runtime/mutexLocker.cpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/runtime/mutexLocker.cpp	Tue Sep 24 10:12:56 2019 -0400
@@ -230,7 +230,7 @@
   def(OopMapCacheAlloc_lock        , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always); // used for oop_map_cache allocation.
 
   def(MetaspaceExpand_lock         , PaddedMutex  , leaf-1,      true,  Monitor::_safepoint_check_never);
-  def(ClassLoaderDataGraph_lock    , PaddedMutex  , nonleaf,     true,  Monitor::_safepoint_check_always);
+  def(ClassLoaderDataGraph_lock    , PaddedMutex  , nonleaf,     false, Monitor::_safepoint_check_always);
 
   def(Patching_lock                , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);      // used for safepointing and code patching.
   def(CompiledMethod_lock          , PaddedMutex  , special-1,   true,  Monitor::_safepoint_check_never);
@@ -240,7 +240,7 @@
   def(SystemDictionary_lock        , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_always);
   def(ProtectionDomainSet_lock     , PaddedMutex  , leaf-1,      true,  Monitor::_safepoint_check_never);
   def(SharedDictionary_lock        , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);
-  def(Module_lock                  , PaddedMutex  , leaf+2,      true,  Monitor::_safepoint_check_always);
+  def(Module_lock                  , PaddedMutex  , leaf+2,      false, Monitor::_safepoint_check_always);
   def(InlineCacheBuffer_lock       , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
   def(VMStatistic_lock             , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always);
   def(ExpandHeap_lock              , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always); // Used during compilation by VM thread
@@ -291,7 +291,7 @@
   def(MethodData_lock              , PaddedMutex  , nonleaf+3,   false, Monitor::_safepoint_check_always);
   def(TouchedMethodLog_lock        , PaddedMutex  , nonleaf+3,   false, Monitor::_safepoint_check_always);
 
-  def(MethodCompileQueue_lock      , PaddedMonitor, nonleaf+4,   true,  Monitor::_safepoint_check_always);
+  def(MethodCompileQueue_lock      , PaddedMonitor, nonleaf+4,   false, Monitor::_safepoint_check_always);
   def(Debug2_lock                  , PaddedMutex  , nonleaf+4,   true,  Monitor::_safepoint_check_never);
   def(Debug3_lock                  , PaddedMutex  , nonleaf+4,   true,  Monitor::_safepoint_check_never);
   def(CompileThread_lock           , PaddedMonitor, nonleaf+5,   false, Monitor::_safepoint_check_always);
@@ -316,7 +316,7 @@
 
   def(CodeHeapStateAnalytics_lock  , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
   def(NMethodSweeperStats_lock     , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);
-  def(ThreadsSMRDelete_lock        , PaddedMonitor, special,     false, Monitor::_safepoint_check_never);
+  def(ThreadsSMRDelete_lock        , PaddedMonitor, special,     true,  Monitor::_safepoint_check_never);
   def(SharedDecoder_lock           , PaddedMutex  , native,      false, Monitor::_safepoint_check_never);
   def(DCmdFactory_lock             , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
 #if INCLUDE_NMT
--- a/src/hotspot/share/runtime/mutexLocker.hpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/runtime/mutexLocker.hpp	Tue Sep 24 10:12:56 2019 -0400
@@ -149,6 +149,8 @@
 extern Monitor* JVMCI_lock;                      // Monitor to control initialization of JVMCI
 #endif
 
+extern Mutex* tty_lock;                          // lock to synchronize output.
+
 // A MutexLocker provides mutual exclusion with respect to a given mutex
 // for the scope which contains the locker.  The lock is an OS lock, not
 // an object lock, and the two do not interoperate.  Do not use Mutex-based
--- a/src/hotspot/share/runtime/synchronizer.cpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/runtime/synchronizer.cpp	Tue Sep 24 10:12:56 2019 -0400
@@ -419,7 +419,7 @@
 ObjectLocker::ObjectLocker(Handle obj, Thread* thread, bool do_lock) {
   _dolock = do_lock;
   _thread = thread;
-  _thread->check_for_valid_safepoint_state(false);
+  _thread->check_for_valid_safepoint_state();
   _obj = obj;
 
   if (_dolock) {
--- a/src/hotspot/share/runtime/thread.cpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/runtime/thread.cpp	Tue Sep 24 10:12:56 2019 -0400
@@ -973,6 +973,7 @@
   if (!is_Java_thread()) return;
 
   if (_no_safepoint_count > 0) {
+    print_owned_locks();
     fatal("Possible safepoint reached by thread that does not allow it");
   }
 #ifdef CHECK_UNHANDLED_OOPS
@@ -981,37 +982,18 @@
 #endif // CHECK_UNHANDLED_OOPS
 }
 
-// The flag: potential_vm_operation notifies if this particular safepoint state could potentially
-// invoke the vm-thread (e.g., an oop allocation). In that case, we also have to make sure that
-// no locks which allow_vm_block's are held
-void Thread::check_for_valid_safepoint_state(bool potential_vm_operation) {
+void Thread::check_for_valid_safepoint_state() {
   if (!is_Java_thread()) return;
 
+  // Check NoSafepointVerifier, which is implied by locks taken that can be
+  // shared with the VM thread.  This makes sure that no locks with allow_vm_block
+  // are held.
   check_possible_safepoint();
 
   if (((JavaThread*)this)->thread_state() != _thread_in_vm) {
     fatal("LEAF method calling lock?");
   }
 
-  if (potential_vm_operation && !Universe::is_bootstrapping()) {
-    // Make sure we do not hold any locks that the VM thread also uses.
-    // This could potentially lead to deadlocks
-    for (Mutex* cur = _owned_locks; cur; cur = cur->next()) {
-      // Threads_lock is special, since the safepoint synchronization will not start before this is
-      // acquired. Hence, a JavaThread cannot be holding it at a safepoint. So is VMOperationRequest_lock,
-      // since it is used to transfer control between JavaThreads and the VMThread
-      // Do not *exclude* any locks unless you are absolutely sure it is correct. Ask someone else first!
-      if ((cur->allow_vm_block() &&
-           cur != Threads_lock &&
-           cur != Compile_lock &&               // Temporary: should not be necessary when we get separate compilation
-           cur != VMOperationRequest_lock &&
-           cur != VMOperationQueue_lock) ||
-           cur->rank() == Mutex::special) {
-        fatal("Thread holding lock at safepoint that vm can block on: %s", cur->name());
-      }
-    }
-  }
-
   if (GCALotAtAllSafepoints) {
     // We could enter a safepoint here and thus have a gc
     InterfaceSupport::check_gc_alot();
--- a/src/hotspot/share/runtime/thread.hpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/runtime/thread.hpp	Tue Sep 24 10:12:56 2019 -0400
@@ -753,7 +753,7 @@
 
   // These functions check conditions on a JavaThread before possibly going to a safepoint,
   // including NoSafepointVerifier.
-  void check_for_valid_safepoint_state(bool potential_vm_operation) NOT_DEBUG_RETURN;
+  void check_for_valid_safepoint_state() NOT_DEBUG_RETURN;
   void check_possible_safepoint() NOT_DEBUG_RETURN;
 
  private:
--- a/src/hotspot/share/runtime/vmThread.cpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/runtime/vmThread.cpp	Tue Sep 24 10:12:56 2019 -0400
@@ -669,7 +669,7 @@
     bool concurrent = op->evaluate_concurrently();
     // only blocking VM operations need to verify the caller's safepoint state:
     if (!concurrent) {
-      t->check_for_valid_safepoint_state(true);
+      t->check_for_valid_safepoint_state();
     }
 
     // New request from Java thread, evaluate prologue
--- a/src/hotspot/share/utilities/concurrentHashTable.inline.hpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/utilities/concurrentHashTable.inline.hpp	Tue Sep 24 10:12:56 2019 -0400
@@ -999,8 +999,8 @@
 {
   _stats_rate = TableRateStatistics();
   _resize_lock =
-    new Mutex(Mutex::leaf, "ConcurrentHashTable", false,
-              Monitor::_safepoint_check_never);
+    new Mutex(Mutex::leaf, "ConcurrentHashTable", true,
+              Mutex::_safepoint_check_never);
   _table = new InternalTable(log2size);
   assert(log2size_limit >= log2size, "bad ergo");
   _size_limit_reached = _table->_log2_size == _log2_size_limit;
--- a/src/hotspot/share/utilities/events.hpp	Mon Sep 23 16:53:16 2019 +0100
+++ b/src/hotspot/share/utilities/events.hpp	Tue Sep 24 10:12:56 2019 -0400
@@ -100,7 +100,7 @@
 
  public:
   EventLogBase<T>(const char* name, const char* handle, int length = LogEventsBufferEntries):
-    _mutex(Mutex::event, name, false, Monitor::_safepoint_check_never),
+    _mutex(Mutex::event, name, true, Mutex::_safepoint_check_never),
     _name(name),
     _handle(handle),
     _length(length),