8352116: Deadlock with GCLocker and JVMTI after JDK-8192647

Reviewed-by: kbarrett, tschatzl, eosterlund

Author: albertnetymk

src/hotspot/share/gc/shared/gcLocker.cpp

@@ -65,8 +65,8 @@ volatile bool GCLocker::_is_gc_request_pending;
 DEBUG_ONLY(uint64_t GCLocker::_verify_in_cr_count;)
 
 void GCLocker::initialize() {
-  assert(Heap_lock != nullptr, "inv");
-  _lock = Heap_lock;
+  assert(JNICritical_lock != nullptr, "inv");
+  _lock = JNICritical_lock;
   _is_gc_request_pending = false;
 
   DEBUG_ONLY(_verify_in_cr_count = 0;)
@@ -82,7 +82,8 @@ bool GCLocker::is_active() {
 }
 
 void GCLocker::block() {
-  assert(_lock->is_locked(), "precondition");
+  // _lock is held from the beginning of block() to the end of of unblock().
+  _lock->lock();
   assert(Atomic::load(&_is_gc_request_pending) == false, "precondition");
 
   GCLockerTimingDebugLogger logger("Thread blocked to start GC.");
@@ -116,10 +117,10 @@ void GCLocker::block() {
 }
 
 void GCLocker::unblock() {
-  assert(_lock->is_locked(), "precondition");
   assert(Atomic::load(&_is_gc_request_pending) == true, "precondition");
 
   Atomic::store(&_is_gc_request_pending, false);
+  _lock->unlock();
 }
 
 void GCLocker::enter_slow(JavaThread* current_thread) {

src/hotspot/share/gc/shared/gcVMOperations.cpp

@@ -93,6 +93,11 @@ bool VM_GC_Operation::skip_operation() const {
   return skip;
 }
 
+static bool should_use_gclocker() {
+  // Only Serial and Parallel use GCLocker to synchronize with threads in JNI critical-sections, in order to handle pinned objects.
+  return UseSerialGC || UseParallelGC;
+}
+
 bool VM_GC_Operation::doit_prologue() {
   assert(((_gc_cause != GCCause::_no_gc) &&
           (_gc_cause != GCCause::_no_cause_specified)), "Illegal GCCause");
@@ -106,34 +111,38 @@ bool VM_GC_Operation::doit_prologue() {
               proper_unit_for_byte_size(NewSize)));
   }
 
+
+  if (should_use_gclocker()) {
+    GCLocker::block();
+  }
   VM_GC_Sync_Operation::doit_prologue();
 
   // Check invocations
   if (skip_operation()) {
     // skip collection
     Heap_lock->unlock();
+    if (should_use_gclocker()) {
+      GCLocker::unblock();
+    }
     _prologue_succeeded = false;
   } else {
-    if (UseSerialGC || UseParallelGC) {
-      GCLocker::block();
-    }
     _prologue_succeeded = true;
   }
   return _prologue_succeeded;
 }
 
 
 void VM_GC_Operation::doit_epilogue() {
-  if (UseSerialGC || UseParallelGC) {
-    GCLocker::unblock();
-  }
   // GC thread root traversal likely used OopMapCache a lot, which
   // might have created lots of old entries. Trigger the cleanup now.
   OopMapCache::try_trigger_cleanup();
   if (Universe::has_reference_pending_list()) {
     Heap_lock->notify_all();
   }
   VM_GC_Sync_Operation::doit_epilogue();
+  if (should_use_gclocker()) {
+    GCLocker::unblock();
+  }
 }
 
 bool VM_GC_HeapInspection::doit_prologue() {

src/hotspot/share/runtime/mutexLocker.cpp

@@ -46,6 +46,7 @@ Mutex*   CompiledIC_lock              = nullptr;
 Mutex*   VMStatistic_lock             = nullptr;
 Mutex*   JmethodIdCreation_lock       = nullptr;
 Mutex*   JfieldIdCreation_lock        = nullptr;
+Monitor* JNICritical_lock             = nullptr;
 Mutex*   JvmtiThreadState_lock        = nullptr;
 Monitor* EscapeBarrier_lock           = nullptr;
 Monitor* JvmtiVTMSTransition_lock     = nullptr;
@@ -318,7 +319,8 @@ void mutex_init() {
 
   MUTEX_DEFL(Threads_lock                   , PaddedMonitor, CompileThread_lock, true);
   MUTEX_DEFL(Compile_lock                   , PaddedMutex  , MethodCompileQueue_lock);
-  MUTEX_DEFL(Heap_lock                      , PaddedMonitor, AdapterHandlerLibrary_lock);
+  MUTEX_DEFL(JNICritical_lock               , PaddedMonitor, AdapterHandlerLibrary_lock); // used for JNI critical regions
+  MUTEX_DEFL(Heap_lock                      , PaddedMonitor, JNICritical_lock);
 
   MUTEX_DEFL(PerfDataMemAlloc_lock          , PaddedMutex  , Heap_lock);
   MUTEX_DEFL(PerfDataManager_lock           , PaddedMutex  , Heap_lock);

src/hotspot/share/runtime/mutexLocker.hpp

@@ -44,6 +44,7 @@ extern Mutex*   CompiledIC_lock;                 // a lock used to guard compile
 extern Mutex*   VMStatistic_lock;                // a lock used to guard statistics count increment
 extern Mutex*   JmethodIdCreation_lock;          // a lock on creating JNI method identifiers
 extern Mutex*   JfieldIdCreation_lock;           // a lock on creating JNI static field identifiers
+extern Monitor* JNICritical_lock;                // a lock used while synchronizing with threads entering/leaving JNI critical regions
 extern Mutex*   JvmtiThreadState_lock;           // a lock on modification of JVMTI thread data
 extern Monitor* EscapeBarrier_lock;              // a lock to sync reallocating and relocking objects because of JVMTI access
 extern Monitor* JvmtiVTMSTransition_lock;        // a lock for Virtual Thread Mount State transition (VTMS transition) management