From 73f02463d9513c5a74022420e9407787a6b25f8c Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Wed, 17 Aug 2022 17:39:58 +0000 Subject: [PATCH 1/4] 8291736: find_method_handle_intrinsic leaks Method* --- .../share/classfile/systemDictionary.cpp | 31 +++++++------------ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/hotspot/share/classfile/systemDictionary.cpp b/src/hotspot/share/classfile/systemDictionary.cpp index e51066d3ebdb0..d42763b17c638 100644 --- a/src/hotspot/share/classfile/systemDictionary.cpp +++ b/src/hotspot/share/classfile/systemDictionary.cpp @@ -2064,21 +2064,17 @@ Method* SystemDictionary::find_method_handle_intrinsic(vmIntrinsicID iid, Symbol* signature, TRAPS) { - methodHandle empty; const int iid_as_int = vmIntrinsics::as_int(iid); assert(MethodHandles::is_signature_polymorphic(iid) && MethodHandles::is_signature_polymorphic_intrinsic(iid) && iid != vmIntrinsics::_invokeGeneric, "must be a known MH intrinsic iid=%d: %s", iid_as_int, vmIntrinsics::name_at(iid)); - Method** met; + MutexLocker ml(THREAD, InvokeMethodTable_lock); InvokeMethodKey key(signature, iid_as_int); - { - MutexLocker ml(THREAD, InvokeMethodTable_lock); - met = _invoke_method_intrinsic_table.get(key); - if (met != nullptr) { - return *met; - } + Method** met = _invoke_method_intrinsic_table.get(key); + if (met != nullptr) { + return *met; } methodHandle m = Method::make_method_handle_intrinsic(iid, signature, CHECK_NULL); @@ -2092,19 +2088,14 @@ Method* SystemDictionary::find_method_handle_intrinsic(vmIntrinsicID iid, "Out of space in CodeCache for method handle intrinsic"); } } - // Now grab the lock. We might have to throw away the new method, - // if a racing thread has managed to install one at the same time. - { - MutexLocker ml(THREAD, InvokeMethodTable_lock); - signature->make_permanent(); // The signature is never unloaded. - bool created; - met = _invoke_method_intrinsic_table.put_if_absent(key, m(), &created); - Method* saved_method = *met; - assert(Arguments::is_interpreter_only() || (saved_method->has_compiled_code() && - saved_method->code()->entry_point() == saved_method->from_compiled_entry()), + + 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"); - return saved_method; - } + return m(); } // Helper for unpacking the return value from linkMethod and linkCallSite. From c12234dabc6c5d0cc2afca89bcc99cb54d100df8 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Thu, 25 Aug 2022 11:31:43 +0000 Subject: [PATCH 2/4] Fix whitespace. --- src/hotspot/share/classfile/systemDictionary.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/classfile/systemDictionary.cpp b/src/hotspot/share/classfile/systemDictionary.cpp index d42763b17c638..0bf2d2a4fa7ae 100644 --- a/src/hotspot/share/classfile/systemDictionary.cpp +++ b/src/hotspot/share/classfile/systemDictionary.cpp @@ -2074,7 +2074,7 @@ Method* SystemDictionary::find_method_handle_intrinsic(vmIntrinsicID iid, InvokeMethodKey key(signature, iid_as_int); Method** met = _invoke_method_intrinsic_table.get(key); if (met != nullptr) { - return *met; + return *met; } methodHandle m = Method::make_method_handle_intrinsic(iid, signature, CHECK_NULL); From f8e85da234cf5fc42e5f9d55837f702fad6fd88e Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Tue, 30 Aug 2022 12:32:54 +0000 Subject: [PATCH 3/4] Move one exception out of locked region. --- .../share/classfile/systemDictionary.cpp | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/src/hotspot/share/classfile/systemDictionary.cpp b/src/hotspot/share/classfile/systemDictionary.cpp index 0bf2d2a4fa7ae..30628d0a46492 100644 --- a/src/hotspot/share/classfile/systemDictionary.cpp +++ b/src/hotspot/share/classfile/systemDictionary.cpp @@ -2070,32 +2070,38 @@ 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)); - 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, 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. - if (!m->has_compiled_code()) { - THROW_MSG_NULL(vmSymbols::java_lang_VirtualMachineError(), - "Out of space in CodeCache for method handle intrinsic"); - } + { + MutexLocker ml(THREAD, InvokeMethodTable_lock); + InvokeMethodKey key(signature, iid_as_int); + Method** met = _invoke_method_intrinsic_table.get(key); + if (met != nullptr) { + return *met; + } + + bool throw_error = false; + 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()); + } + + 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"); + return m(); + } } - 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"); - return m(); + // 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. From d0ead64c08e0998d9864a6b5237d2d1c60c77376 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Thu, 1 Sep 2022 14:58:04 +0000 Subject: [PATCH 4/4] Add comment --- src/hotspot/share/classfile/systemDictionary.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hotspot/share/classfile/systemDictionary.cpp b/src/hotspot/share/classfile/systemDictionary.cpp index 30628d0a46492..8379639fbc6c5 100644 --- a/src/hotspot/share/classfile/systemDictionary.cpp +++ b/src/hotspot/share/classfile/systemDictionary.cpp @@ -2079,6 +2079,8 @@ Method* SystemDictionary::find_method_handle_intrinsic(vmIntrinsicID iid, } 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