8306951: [BACKOUT] JDK-8305252 make_method_handle_intrinsic may call java code under a lock

Reviewed-by: dcubed

Author: coleenp

src/hotspot/share/classfile/systemDictionary.cpp

@@ -1583,13 +1583,11 @@ void SystemDictionary::methods_do(void f(Method*)) {
   }
 
   auto doit = [&] (InvokeMethodKey key, Method* method) {
-    if (method != nullptr) {
-      f(method);
-    }
+    f(method);
   };
 
   {
-    MutexLocker ml(InvokeMethodIntrinsicTable_lock);
+    MutexLocker ml(InvokeMethodTable_lock);
     _invoke_method_intrinsic_table.iterate_all(doit);
   }
 
@@ -1941,62 +1939,40 @@ Method* SystemDictionary::find_method_handle_intrinsic(vmIntrinsicID iid,
          iid != vmIntrinsics::_invokeGeneric,
          "must be a known MH intrinsic iid=%d: %s", iid_as_int, vmIntrinsics::name_at(iid));
 
-  InvokeMethodKey key(signature, iid_as_int);
-  Method** met = nullptr;
-
-  // We only want one entry in the table for this (signature/id, method) pair but the code
-  // to create the intrinsic method needs to be outside the lock.
-  // The first thread claims the entry by adding the key and the other threads wait, until the
-  // Method has been added as the value.
   {
-    MonitorLocker ml(THREAD, InvokeMethodIntrinsicTable_lock);
-    while (met == nullptr) {
-      bool created;
-      met = _invoke_method_intrinsic_table.put_if_absent(key, &created);
-      if (met != nullptr && *met != nullptr) {
-        return *met;
-      } else if (!created) {
-        // Second thread waits for first to actually create the entry and returns
-        // it after notify. Loop until method return is non-null.
-        ml.wait();
-      }
+    MutexLocker ml(THREAD, InvokeMethodTable_lock);
+    InvokeMethodKey key(signature, iid_as_int);
+    Method** met = _invoke_method_intrinsic_table.get(key);
+    if (met != nullptr) {
+      return *met;
     }
-  }
 
-  methodHandle m = Method::make_method_handle_intrinsic(iid, signature, THREAD);
-  bool throw_error = HAS_PENDING_EXCEPTION;
-  if (!throw_error && (!Arguments::is_interpreter_only() || iid == vmIntrinsics::_linkToNative)) {
-    // Generate a compiled form of the MH intrinsic
-    // linkToNative doesn't have interpreter-specific implementation, so always has to go through compiled version.
-    AdapterHandlerLibrary::create_native_wrapper(m);
-    // Check if have the compiled code.
-    throw_error = (!m->has_compiled_code());
-  }
+    bool throw_error = false;
+    // This function could get an OOM but it is safe to call inside of a lock because
+    // throwing OutOfMemoryError doesn't call Java code.
+    methodHandle m = Method::make_method_handle_intrinsic(iid, signature, CHECK_NULL);
+    if (!Arguments::is_interpreter_only() || iid == vmIntrinsics::_linkToNative) {
+        // Generate a compiled form of the MH intrinsic
+        // linkToNative doesn't have interpreter-specific implementation, so always has to go through compiled version.
+        AdapterHandlerLibrary::create_native_wrapper(m);
+        // Check if have the compiled code.
+        throw_error = (!m->has_compiled_code());
+    }
 
-  {
-    MonitorLocker ml(THREAD, InvokeMethodIntrinsicTable_lock);
-    if (throw_error) {
-      // Remove the entry and let another thread try, or get the same exception.
-      bool removed = _invoke_method_intrinsic_table.remove(key);
-      assert(removed, "must be the owner");
-      ml.notify_all();
-    } else {
+    if (!throw_error) {
       signature->make_permanent(); // The signature is never unloaded.
+      bool created = _invoke_method_intrinsic_table.put(key, m());
+      assert(created, "must be since we still hold the lock");
       assert(Arguments::is_interpreter_only() || (m->has_compiled_code() &&
              m->code()->entry_point() == m->from_compiled_entry()),
              "MH intrinsic invariant");
-      *met = m(); // insert the element
-      ml.notify_all();
       return m();
     }
   }
 
-  // Throw VirtualMachineError or the pending exception in the JavaThread
-  if (throw_error && !HAS_PENDING_EXCEPTION) {
-    THROW_MSG_NULL(vmSymbols::java_lang_VirtualMachineError(),
-                   "Out of space in CodeCache for method handle intrinsic");
-  }
-  return nullptr;
+  // Throw error outside of the lock.
+  THROW_MSG_NULL(vmSymbols::java_lang_VirtualMachineError(),
+                 "Out of space in CodeCache for method handle intrinsic");
 }
 
 // Helper for unpacking the return value from linkMethod and linkCallSite.
@@ -2139,7 +2115,7 @@ Handle SystemDictionary::find_method_handle_type(Symbol* signature,
   Handle empty;
   OopHandle* o;
   {
-    MutexLocker ml(THREAD, InvokeMethodTypeTable_lock);
+    MutexLocker ml(THREAD, InvokeMethodTable_lock);
     o = _invoke_method_type_table.get(signature);
   }
 
@@ -2208,7 +2184,7 @@ Handle SystemDictionary::find_method_handle_type(Symbol* signature,
 
   if (can_be_cached) {
     // We can cache this MethodType inside the JVM.
-    MutexLocker ml(THREAD, InvokeMethodTypeTable_lock);
+    MutexLocker ml(THREAD, InvokeMethodTable_lock);
     bool created = false;
     assert(method_type != nullptr, "unexpected null");
     OopHandle* h = _invoke_method_type_table.get(signature);

src/hotspot/share/runtime/mutexLocker.cpp

@@ -39,8 +39,7 @@
 Mutex*   Patching_lock                = nullptr;
 Mutex*   CompiledMethod_lock          = nullptr;
 Monitor* SystemDictionary_lock        = nullptr;
-Mutex*   InvokeMethodTypeTable_lock   = nullptr;
-Monitor* InvokeMethodIntrinsicTable_lock = nullptr;
+Mutex*   InvokeMethodTable_lock       = nullptr;
 Mutex*   SharedDictionary_lock        = nullptr;
 Monitor* ClassInitError_lock          = nullptr;
 Mutex*   Module_lock                  = nullptr;
@@ -255,9 +254,7 @@ void mutex_init() {
   }
 
   MUTEX_DEFN(JmethodIdCreation_lock          , PaddedMutex  , nosafepoint-2); // used for creating jmethodIDs.
-  MUTEX_DEFN(InvokeMethodTypeTable_lock      , PaddedMutex  , safepoint);
-  MUTEX_DEFN(InvokeMethodIntrinsicTable_lock , PaddedMonitor, safepoint);
-  MUTEX_DEFN(AdapterHandlerLibrary_lock      , PaddedMutex  , safepoint);
+  MUTEX_DEFN(InvokeMethodTable_lock          , PaddedMutex  , safepoint);
   MUTEX_DEFN(SharedDictionary_lock           , PaddedMutex  , safepoint);
   MUTEX_DEFN(VMStatistic_lock                , PaddedMutex  , safepoint);
   MUTEX_DEFN(SignatureHandlerLibrary_lock    , PaddedMutex  , safepoint);
@@ -347,6 +344,7 @@ void mutex_init() {
 
   MUTEX_DEFL(Threads_lock                   , PaddedMonitor, CompileThread_lock, true);
   MUTEX_DEFL(Compile_lock                   , PaddedMutex  , MethodCompileQueue_lock);
+  MUTEX_DEFL(AdapterHandlerLibrary_lock     , PaddedMutex  , InvokeMethodTable_lock);
   MUTEX_DEFL(Heap_lock                      , PaddedMonitor, AdapterHandlerLibrary_lock);
 
   MUTEX_DEFL(PerfDataMemAlloc_lock          , PaddedMutex  , Heap_lock);

src/hotspot/share/runtime/mutexLocker.hpp

@@ -34,8 +34,7 @@
 extern Mutex*   Patching_lock;                   // a lock used to guard code patching of compiled code
 extern Mutex*   CompiledMethod_lock;             // a lock used to guard a compiled method and OSR queues
 extern Monitor* SystemDictionary_lock;           // a lock on the system dictionary
-extern Mutex*   InvokeMethodTypeTable_lock;
-extern Monitor* InvokeMethodIntrinsicTable_lock;
+extern Mutex*   InvokeMethodTable_lock;
 extern Mutex*   SharedDictionary_lock;           // a lock on the CDS shared dictionary
 extern Monitor* ClassInitError_lock;             // a lock on the class initialization error table
 extern Mutex*   Module_lock;                     // a lock on module and package related data structures