8074355: make MutexLocker smarter about non-JavaThreads
authorcoleenp
Wed, 01 May 2019 08:56:38 -0400
changeset 54663 f03d5a093093
parent 54662 afce4a27f2c2
child 54664 5ddea9d48649
8074355: make MutexLocker smarter about non-JavaThreads Summary: Use safepoint_check_always/safepoint_check_never instead of safepoint_check_sometimes for locks that are taken by JavaThreads and non-JavaThreads Reviewed-by: dholmes, rehn, dcubed, lmesnik
src/hotspot/share/gc/cms/compactibleFreeListSpace.cpp
src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp
src/hotspot/share/gc/cms/yieldingWorkgroup.cpp
src/hotspot/share/gc/shared/oopStorage.cpp
src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp
src/hotspot/share/runtime/mutex.cpp
src/hotspot/share/runtime/mutex.hpp
src/hotspot/share/runtime/mutexLocker.cpp
src/hotspot/share/runtime/vmThread.cpp
test/hotspot/jtreg/runtime/Shutdown/ShutdownTest.java
--- a/src/hotspot/share/gc/cms/compactibleFreeListSpace.cpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/gc/cms/compactibleFreeListSpace.cpp	Wed May 01 08:56:38 2019 -0400
@@ -331,11 +331,11 @@
   // Note: this requires that CFLspace c'tors
   // are called serially in the order in which the locks are
   // are acquired in the program text. This is true today.
-  _freelistLock(_lockRank--, "CompactibleFreeListSpace._lock", true,
-                Monitor::_safepoint_check_sometimes),
+  _freelistLock(_lockRank--, "CompactibleFreeListSpace_lock", true,
+                Monitor::_safepoint_check_never),
   _preconsumptionDirtyCardClosure(NULL),
   _parDictionaryAllocLock(Mutex::leaf - 1,  // == rank(ExpandHeap_lock) - 1
-                          "CompactibleFreeListSpace._dict_par_lock", true,
+                          "CompactibleFreeListSpace_dict_par_lock", true,
                           Monitor::_safepoint_check_never)
 {
   assert(sizeof(FreeChunk) / BytesPerWord <= MinChunkSize,
@@ -366,7 +366,7 @@
   // Initialize locks for parallel case.
   for (size_t i = IndexSetStart; i < IndexSetSize; i += IndexSetStride) {
     _indexedFreeListParLocks[i] = new Mutex(Mutex::leaf - 1, // == ExpandHeap_lock - 1
-                                            "a freelist par lock", true, Mutex::_safepoint_check_sometimes);
+                                            "a freelist par lock", true, Mutex::_safepoint_check_never);
     DEBUG_ONLY(
       _indexedFreeList[i].set_protecting_lock(_indexedFreeListParLocks[i]);
     )
@@ -2042,7 +2042,7 @@
   if (rem_sz < SmallForDictionary) {
     // The freeList lock is held, but multiple GC task threads might be executing in parallel.
     bool is_par = Thread::current()->is_GC_task_thread();
-    if (is_par) _indexedFreeListParLocks[rem_sz]->lock();
+    if (is_par) _indexedFreeListParLocks[rem_sz]->lock_without_safepoint_check();
     returnChunkToFreeList(ffc);
     split(size, rem_sz);
     if (is_par) _indexedFreeListParLocks[rem_sz]->unlock();
--- a/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp	Wed May 01 08:56:38 2019 -0400
@@ -491,7 +491,7 @@
   _stats(cmsGen),
   _eden_chunk_lock(new Mutex(Mutex::leaf + 1, "CMS_eden_chunk_lock", true,
                              //verify that this lock should be acquired with safepoint check.
-                             Monitor::_safepoint_check_sometimes)),
+                             Monitor::_safepoint_check_never)),
   _eden_chunk_array(NULL),     // may be set in ctor body
   _eden_chunk_index(0),        // -- ditto --
   _eden_chunk_capacity(0),     // -- ditto --
@@ -5643,7 +5643,7 @@
   _shifter(shifter),
   _bm(),
   _lock(mutex_rank >= 0 ? new Mutex(mutex_rank, mutex_name, true,
-                                    Monitor::_safepoint_check_sometimes) : NULL)
+                                    Monitor::_safepoint_check_never) : NULL)
 {
   _bmStartWord = 0;
   _bmWordSize  = 0;
@@ -7526,7 +7526,7 @@
   }
 
   ConcurrentMarkSweepThread::synchronize(true);
-  _freelistLock->lock();
+  _freelistLock->lock_without_safepoint_check();
   _bitMap->lock()->lock_without_safepoint_check();
   _collector->startTimer();
 }
--- a/src/hotspot/share/gc/cms/yieldingWorkgroup.cpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/gc/cms/yieldingWorkgroup.cpp	Wed May 01 08:56:38 2019 -0400
@@ -43,7 +43,7 @@
   _monitor = new Monitor(/* priority */       Mutex::leaf,
                          /* name */           "WorkGroup monitor",
                          /* allow_vm_block */ are_GC_task_threads,
-                                              Monitor::_safepoint_check_sometimes);
+                                              Monitor::_safepoint_check_never);
 
   assert(monitor() != NULL, "Failed to allocate monitor");
 }
--- a/src/hotspot/share/gc/shared/oopStorage.cpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/gc/shared/oopStorage.cpp	Wed May 01 08:56:38 2019 -0400
@@ -765,10 +765,10 @@
          "%s: active_mutex must have lower rank than allocation_mutex", _name);
   assert(Service_lock->rank() < _active_mutex->rank(),
          "%s: active_mutex must have higher rank than Service_lock", _name);
-  assert(_active_mutex->_safepoint_check_required != Mutex::_safepoint_check_always,
-         "%s: active mutex requires safepoint check", _name);
-  assert(_allocation_mutex->_safepoint_check_required != Mutex::_safepoint_check_always,
-         "%s: allocation mutex requires safepoint check", _name);
+  assert(_active_mutex->_safepoint_check_required == Mutex::_safepoint_check_never,
+         "%s: active mutex requires never safepoint check", _name);
+  assert(_allocation_mutex->_safepoint_check_required == Mutex::_safepoint_check_never,
+         "%s: allocation mutex requires never safepoint check", _name);
 }
 
 void OopStorage::delete_empty_block(const Block& block) {
--- a/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp	Wed May 01 08:56:38 2019 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -136,7 +136,7 @@
  public:
   StackTraceWrite(JfrStackTraceRepository& stack_trace_repo, JfrCheckpointWriter& writer) :
     _stack_trace_repo(stack_trace_repo), _writer(writer), _count(0) {
-    JfrStacktrace_lock->lock();
+    JfrStacktrace_lock->lock_without_safepoint_check();
   }
   ~StackTraceWrite() {
     assert(JfrStacktrace_lock->owned_by_self(), "invariant");
--- a/src/hotspot/share/runtime/mutex.cpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/runtime/mutex.cpp	Wed May 01 08:56:38 2019 -0400
@@ -32,11 +32,23 @@
 #include "utilities/events.hpp"
 #include "utilities/macros.hpp"
 
+#ifdef ASSERT
+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,
+         "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 != Monitor::_safepoint_check_never,
+         "NonJavaThread should not check for safepoint");
+}
+#endif // ASSERT
 
 void Monitor::lock(Thread * self) {
-  // Ensure that the Monitor requires/allows safepoint checks.
-  assert(_safepoint_check_required != Monitor::_safepoint_check_never,
-         "This lock should never have a safepoint check: %s", name());
+  check_safepoint_state(self, true);
 
 #ifdef CHECK_UNHANDLED_OOPS
   // Clear unhandled oops in JavaThreads so we get a crash right away.
@@ -91,9 +103,7 @@
 // in the wrong way this can lead to a deadlock with the safepoint code.
 
 void Monitor::lock_without_safepoint_check(Thread * self) {
-  // Ensure that the Monitor does not require safepoint checks.
-  assert(_safepoint_check_required != Monitor::_safepoint_check_always,
-         "This lock should always have a safepoint check: %s", name());
+  check_safepoint_state(self, false);
   assert(_owner != self, "invariant");
   _lock.lock();
   assert_owner(NULL);
@@ -154,14 +164,12 @@
 #endif // ASSERT
 
 bool Monitor::wait_without_safepoint_check(long timeout) {
-  // Make sure safepoint checking is used properly.
-  assert(_safepoint_check_required != Monitor::_safepoint_check_always,
-         "This lock should always have a safepoint check: %s", name());
+  Thread* const self = Thread::current();
+  check_safepoint_state(self, false);
 
   // timeout is in milliseconds - with zero meaning never timeout
   assert(timeout >= 0, "negative timeout");
 
-  Thread * const self = Thread::current();
   assert_owner(self);
   assert_wait_lock_state(self);
 
@@ -174,14 +182,12 @@
 }
 
 bool Monitor::wait(long timeout, bool as_suspend_equivalent) {
-  // Make sure safepoint checking is used properly.
-  assert(_safepoint_check_required != Monitor::_safepoint_check_never,
-         "This lock should never have a safepoint check: %s", name());
+  Thread* const self = Thread::current();
+  check_safepoint_state(self, true);
 
   // timeout is in milliseconds - with zero meaning never timeout
   assert(timeout >= 0, "negative timeout");
 
-  Thread* const self = Thread::current();
   assert_owner(self);
 
   // Safepoint checking logically implies java_thread
@@ -273,6 +279,12 @@
   ClearMonitor(this);
 }
 
+
+// Only Threads_lock, Heap_lock and SR_lock may be safepoint_check_sometimes.
+bool is_sometimes_ok(const char* name) {
+  return (strcmp(name, "Threads_lock") == 0 || strcmp(name, "Heap_lock") == 0 || strcmp(name, "SR_lock") == 0);
+}
+
 Monitor::Monitor(int Rank, const char * name, bool allow_vm_block,
                  SafepointCheckRequired safepoint_check_required) {
   assert(os::mutex_init_done(), "Too early!");
@@ -281,6 +293,9 @@
   _allow_vm_block  = allow_vm_block;
   _rank            = Rank;
   NOT_PRODUCT(_safepoint_check_required = safepoint_check_required;)
+
+  assert(_safepoint_check_required != Monitor::_safepoint_check_sometimes || is_sometimes_ok(name),
+         "Lock has _safepoint_check_sometimes %s", name);
 #endif
 }
 
@@ -291,6 +306,9 @@
   _allow_vm_block   = allow_vm_block;
   _rank             = Rank;
   NOT_PRODUCT(_safepoint_check_required = safepoint_check_required;)
+
+  assert(_safepoint_check_required != Monitor::_safepoint_check_sometimes || is_sometimes_ok(name),
+         "Lock has _safepoint_check_sometimes %s", name);
 #endif
 }
 
--- a/src/hotspot/share/runtime/mutex.hpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/runtime/mutex.hpp	Wed May 01 08:56:38 2019 -0400
@@ -92,6 +92,7 @@
   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 assert_owner            (Thread* expected)                     NOT_DEBUG_RETURN;
   void assert_wait_lock_state  (Thread* self)                         NOT_DEBUG_RETURN;
 
@@ -101,28 +102,39 @@
     _as_suspend_equivalent_flag = true
   };
 
+  // Locks can be acquired with or without a safepoint check. NonJavaThreads do not follow
+  // the safepoint protocol when acquiring locks.
+
+  // Each lock can be acquired by only JavaThreads, only NonJavaThreads, or shared between
+  // Java and NonJavaThreads. When the lock is initialized with _safepoint_check_always,
+  // that means that whenever the lock is acquired by a JavaThread, it will verify that
+  // it is done with a safepoint check. In corollary, when the lock is initialized with
+  // _safepoint_check_never, that means that whenever the lock is acquired by a JavaThread
+  // it will verify that it is done without a safepoint check.
+
+
+  // There are a couple of existing locks that will sometimes have a safepoint check and
+  // sometimes not when acquired by a JavaThread, but these locks are set up carefully
+  // to avoid deadlocks. TODO: Fix these locks and remove _safepoint_check_sometimes.
+
+  // TODO: Locks that are shared between JavaThreads and NonJavaThreads
+  // should never encounter a safepoint check while they are held, or else a
+  // deadlock can occur. We should check this by noting which
+  // locks are shared, and walk held locks during safepoint checking.
+
   enum SafepointCheckFlag {
     _safepoint_check_flag,
     _no_safepoint_check_flag
   };
 
-  // Locks can be acquired with or without safepoint check.
-  // Monitor::lock and Monitor::lock_without_safepoint_check
-  // checks these flags when acquiring a lock to ensure
-  // consistent checking for each lock.
-  // A few existing locks will sometimes have a safepoint check and
-  // sometimes not, but these locks are set up in such a way to avoid deadlocks.
-  // Note: monitors that may be shared between JavaThreads and the VMThread
-  // should never encounter a safepoint check whilst they are held, else a
-  // deadlock with the VMThread can occur.
   enum SafepointCheckRequired {
     _safepoint_check_never,       // Monitors with this value will cause errors
-                                  // when acquired with a safepoint check.
-    _safepoint_check_sometimes,   // Certain locks are called sometimes with and
-                                  // sometimes without safepoint checks. These
+                                  // when acquired by a JavaThread with a safepoint check.
+    _safepoint_check_sometimes,   // A couple of special locks are acquired by JavaThreads sometimes
+                                  // with and sometimes without safepoint checks. These
                                   // locks will not produce errors when locked.
-    _safepoint_check_always       // Causes error if locked without a safepoint
-                                  // check.
+    _safepoint_check_always       // Monitors with this value will cause errors
+                                  // when acquired by a JavaThread without a safepoint check.
   };
 
   NOT_PRODUCT(SafepointCheckRequired _safepoint_check_required;)
@@ -159,7 +171,7 @@
   // Lock without safepoint check. Should ONLY be used by safepoint code and other code
   // that is guaranteed not to block while running inside the VM.
   void lock_without_safepoint_check();
-  void lock_without_safepoint_check (Thread * Self) ;
+  void lock_without_safepoint_check(Thread* self);
 
   // Current owner - not not MT-safe. Can only be used to guarantee that
   // the current running thread owns the lock
--- a/src/hotspot/share/runtime/mutexLocker.cpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/runtime/mutexLocker.cpp	Wed May 01 08:56:38 2019 -0400
@@ -241,53 +241,53 @@
     def(StringDedupQueue_lock      , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_never);
     def(StringDedupTable_lock      , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
   }
-  def(ParGCRareEvent_lock          , PaddedMutex  , leaf     ,   true,  Monitor::_safepoint_check_sometimes);
+  def(ParGCRareEvent_lock          , PaddedMutex  , leaf     ,   true,  Monitor::_safepoint_check_always);
   def(DerivedPointerTableGC_lock   , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
-  def(CGCPhaseManager_lock         , PaddedMonitor, leaf,        false, Monitor::_safepoint_check_sometimes);
+  def(CGCPhaseManager_lock         , PaddedMonitor, leaf,        false, Monitor::_safepoint_check_always);
   def(CodeCache_lock               , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);
   def(RawMonitor_lock              , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);
-  def(OopMapCacheAlloc_lock        , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);     // used for oop_map_cache allocation.
+  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(Patching_lock                , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);      // used for safepointing and code patching.
   def(Service_lock                 , PaddedMonitor, special,     true,  Monitor::_safepoint_check_never);      // used for service thread operations
-  def(JmethodIdCreation_lock       , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);     // used for creating jmethodIDs.
+  def(JmethodIdCreation_lock       , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always); // used for creating jmethodIDs.
 
-  def(SystemDictionary_lock        , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_always);     // lookups done by VM thread
+  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);     // lookups done by VM thread
+  def(SharedDictionary_lock        , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);
   def(Module_lock                  , PaddedMutex  , leaf+2,      true,  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
+  def(ExpandHeap_lock              , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always); // Used during compilation by VM thread
   def(JNIHandleBlockFreeList_lock  , PaddedMutex  , leaf-1,      true,  Monitor::_safepoint_check_never);      // handles are used by VM thread
   def(SignatureHandlerLibrary_lock , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always);
   def(SymbolArena_lock             , PaddedMutex  , leaf+2,      true,  Monitor::_safepoint_check_never);
-  def(ProfilePrint_lock            , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always);     // serial profile printing
-  def(ExceptionCache_lock          , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always);     // serial profile printing
+  def(ProfilePrint_lock            , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always); // serial profile printing
+  def(ExceptionCache_lock          , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always); // serial profile printing
   def(OsrList_lock                 , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
   def(Debug1_lock                  , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
 #ifndef PRODUCT
-  def(FullGCALot_lock              , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always);     // a lock to make FullGCALot MT safe
+  def(FullGCALot_lock              , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always); // a lock to make FullGCALot MT safe
 #endif
   def(BeforeExit_lock              , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_always);
-  def(PerfDataMemAlloc_lock        , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);     // used for allocating PerfData memory for performance data
-  def(PerfDataManager_lock         , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);     // used for synchronized access to PerfDataManager resources
+  def(PerfDataMemAlloc_lock        , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always); // used for allocating PerfData memory for performance data
+  def(PerfDataManager_lock         , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always); // used for synchronized access to PerfDataManager resources
 
   // CMS_modUnionTable_lock                   leaf
   // CMS_bitMap_lock                          leaf 1
   // CMS_freeList_lock                        leaf 2
 
-  def(Threads_lock                 , PaddedMonitor, barrier,     true,  Monitor::_safepoint_check_sometimes);
+  def(Threads_lock                 , PaddedMonitor, barrier,     true,  Monitor::_safepoint_check_sometimes);  // Used for safepoint protocol.
   def(NonJavaThreadsList_lock      , PaddedMutex,   leaf,        true,  Monitor::_safepoint_check_never);
   def(NonJavaThreadsListSync_lock  , PaddedMutex,   leaf,        true,  Monitor::_safepoint_check_never);
 
-  def(VMOperationQueue_lock        , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_sometimes);  // VM_thread allowed to block on these
-  def(VMOperationRequest_lock      , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_sometimes);
+  def(VMOperationQueue_lock        , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_never);  // VM_thread allowed to block on these
+  def(VMOperationRequest_lock      , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_always);
   def(RetData_lock                 , PaddedMutex  , nonleaf,     false, Monitor::_safepoint_check_always);
-  def(Terminator_lock              , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_sometimes);
+  def(Terminator_lock              , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_always);
   def(InitCompleted_lock           , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_never);
   def(VtableStubs_lock             , PaddedMutex  , nonleaf,     true,  Monitor::_safepoint_check_never);
   def(Notify_lock                  , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_always);
@@ -295,11 +295,11 @@
   def(JNIGlobalActive_lock         , PaddedMutex  , nonleaf-1,   true,  Monitor::_safepoint_check_never);
   def(JNIWeakAlloc_lock            , PaddedMutex  , nonleaf,     true,  Monitor::_safepoint_check_never);
   def(JNIWeakActive_lock           , PaddedMutex  , nonleaf-1,   true,  Monitor::_safepoint_check_never);
-  def(JNICritical_lock             , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_always);     // used for JNI critical regions
+  def(JNICritical_lock             , PaddedMonitor, nonleaf,     true,  Monitor::_safepoint_check_always); // used for JNI critical regions
   def(AdapterHandlerLibrary_lock   , PaddedMutex  , nonleaf,     true,  Monitor::_safepoint_check_always);
 
-  def(Heap_lock                    , PaddedMonitor, nonleaf+1,   false, Monitor::_safepoint_check_sometimes);
-  def(JfieldIdCreation_lock        , PaddedMutex  , nonleaf+1,   true,  Monitor::_safepoint_check_always);     // jfieldID, Used in VM_Operation
+  def(Heap_lock                    , PaddedMonitor, nonleaf+1,   false, Monitor::_safepoint_check_sometimes);  // Doesn't safepoint check during termination.
+  def(JfieldIdCreation_lock        , PaddedMutex  , nonleaf+1,   true,  Monitor::_safepoint_check_always); // jfieldID, Used in VM_Operation
 
   def(CompiledIC_lock              , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_never);      // locks VtableStubs_lock, InlineCacheBuffer_lock
   def(CompileTaskAlloc_lock        , PaddedMutex  , nonleaf+2,   true,  Monitor::_safepoint_check_always);
@@ -307,8 +307,8 @@
   def(DirectivesStack_lock         , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);
   def(MultiArray_lock              , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always);
 
-  def(JvmtiThreadState_lock        , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always);     // Used by JvmtiThreadState/JvmtiEventController
-  def(Management_lock              , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always);     // used for JVM management
+  def(JvmtiThreadState_lock        , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always); // Used by JvmtiThreadState/JvmtiEventController
+  def(Management_lock              , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always); // used for JVM management
 
   def(Compile_lock                 , PaddedMutex  , nonleaf+3,   true,  Monitor::_safepoint_check_always);
   def(MethodData_lock              , PaddedMutex  , nonleaf+3,   false, Monitor::_safepoint_check_always);
@@ -318,7 +318,7 @@
   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);
-  def(PeriodicTask_lock            , PaddedMonitor, nonleaf+5,   true,  Monitor::_safepoint_check_sometimes);
+  def(PeriodicTask_lock            , PaddedMonitor, nonleaf+5,   true,  Monitor::_safepoint_check_always);
   def(RedefineClasses_lock         , PaddedMonitor, nonleaf+5,   true,  Monitor::_safepoint_check_always);
 
   if (WhiteBoxAPI) {
@@ -329,7 +329,7 @@
   def(JfrMsg_lock                  , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_always);
   def(JfrBuffer_lock               , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
   def(JfrStream_lock               , PaddedMutex  , leaf+1,      true,  Monitor::_safepoint_check_never);      // ensure to rank lower than 'safepoint'
-  def(JfrStacktrace_lock           , PaddedMutex  , special,     true,  Monitor::_safepoint_check_sometimes);
+  def(JfrStacktrace_lock           , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);
   def(JfrThreadSampler_lock        , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_never);
 #endif
 
@@ -338,8 +338,7 @@
 #endif
 
   def(CodeHeapStateAnalytics_lock  , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
-
-  def(NMethodSweeperStats_lock     , PaddedMutex  , special,     true,  Monitor::_safepoint_check_sometimes);
+  def(NMethodSweeperStats_lock     , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);
   def(ThreadsSMRDelete_lock        , PaddedMonitor, special,     false, Monitor::_safepoint_check_never);
   def(SharedDecoder_lock           , PaddedMutex  , native,      false, Monitor::_safepoint_check_never);
   def(DCmdFactory_lock             , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
--- a/src/hotspot/share/runtime/vmThread.cpp	Wed May 01 07:12:14 2019 -0400
+++ b/src/hotspot/share/runtime/vmThread.cpp	Wed May 01 08:56:38 2019 -0400
@@ -623,10 +623,8 @@
     }
 
     //
-    //  Notify (potential) waiting Java thread(s) - lock without safepoint
-    //  check so that sneaking is not possible
-    { MutexLocker mu(VMOperationRequest_lock,
-                     Mutex::_no_safepoint_check_flag);
+    //  Notify (potential) waiting Java thread(s)
+    { MutexLocker mu(VMOperationRequest_lock, Mutex::_no_safepoint_check_flag);
       VMOperationRequest_lock->notify_all();
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/Shutdown/ShutdownTest.java	Wed May 01 08:56:38 2019 -0400
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 4656449 4945125 8074355
+ * @summary Make MutexLocker smarter about non-Java threads
+ * @library /test/lib
+ * @run driver/timeout=240 ShutdownTest
+ */
+
+// This test is adapted from an old regression test for bug 4945125, where VerifyBeforeExit
+// crashes before exit for the regression test for bug 4656449.
+// The fix is to acquire the Heap_lock before exit after the JavaThread is removed from
+// the threads list.  This fix is still valid.  This code requires Heap_lock be acquired
+// without a safepoint check at exit.
+
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class ShutdownTest {
+   Object[] obj;
+
+   ShutdownTest() {
+       // Allocate to get some GC pressure.
+       obj = new Object[100000];
+   }
+
+    static class ShutdownTestThread extends Thread {
+       public void run() {
+         while (true) {
+           ShutdownTest st = new ShutdownTest();
+         }
+       }
+
+       public static void main(String args[]) {
+         System.out.println("- ShutdownTest -");
+
+         for (int i = 0; i < 100; i++) {
+           ShutdownTestThread st = new ShutdownTestThread();
+           st.setDaemon(true);
+           st.start();
+         }
+       }
+    }
+
+    private static void startVM(List<String> options) throws Throwable {
+        // Combine VM flags given from command-line and your additional options
+        OutputAnalyzer output = ProcessTools.executeTestJvm(options.toArray(new String[options.size()]));
+        output.shouldContain("- ShutdownTest -");
+        output.shouldHaveExitValue(0);
+
+    }
+
+    public static void main(String[] args) throws Throwable {
+        List<String> options = new ArrayList<>();
+
+        Collections.addAll(options,
+                "-Xmx2500k",
+                "-XX:+UnlockDiagnosticVMOptions",
+                "-XX:+VerifyBeforeExit");
+        options.add(ShutdownTestThread.class.getName());
+
+        for (int iteration = 0; iteration < 5; ++iteration) {
+            startVM(options);
+        }
+    }
+}