From 269cd38b55391364db0f92291eb29c3b6803db94 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Tue, 17 Sep 2024 10:39:31 +0000 Subject: [PATCH] 8338566: Lazy creation of exception instances is not thread safe Reviewed-by: shade, kvn, dlong --- src/hotspot/share/cds/heapShared.cpp | 3 ++ src/hotspot/share/ci/ciEnv.cpp | 64 ++++----------------------- src/hotspot/share/ci/ciEnv.hpp | 23 +++++----- src/hotspot/share/memory/universe.cpp | 21 ++++++++- src/hotspot/share/memory/universe.hpp | 3 ++ src/hotspot/share/runtime/threads.cpp | 1 + 6 files changed, 47 insertions(+), 68 deletions(-) diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index 68be14a46fe..2bf75a5ba65 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -1322,6 +1322,9 @@ void HeapShared::check_default_subgraph_classes() { name == vmSymbols::java_lang_ArithmeticException() || name == vmSymbols::java_lang_NullPointerException() || name == vmSymbols::java_lang_InternalError() || + name == vmSymbols::java_lang_ArrayIndexOutOfBoundsException() || + name == vmSymbols::java_lang_ArrayStoreException() || + name == vmSymbols::java_lang_ClassCastException() || name == vmSymbols::object_array_signature() || name == vmSymbols::byte_array_signature() || name == vmSymbols::char_array_signature(), diff --git a/src/hotspot/share/ci/ciEnv.cpp b/src/hotspot/share/ci/ciEnv.cpp index 3079d469ebe..9caf89628cc 100644 --- a/src/hotspot/share/ci/ciEnv.cpp +++ b/src/hotspot/share/ci/ciEnv.cpp @@ -100,10 +100,6 @@ ciSymbol* ciEnv::_unloaded_cisymbol = nullptr; ciInstanceKlass* ciEnv::_unloaded_ciinstance_klass = nullptr; ciObjArrayKlass* ciEnv::_unloaded_ciobjarrayklass = nullptr; -jobject ciEnv::_ArrayIndexOutOfBoundsException_handle = nullptr; -jobject ciEnv::_ArrayStoreException_handle = nullptr; -jobject ciEnv::_ClassCastException_handle = nullptr; - #ifndef PRODUCT static bool firstEnv = true; #endif /* PRODUCT */ @@ -158,10 +154,16 @@ ciEnv::ciEnv(CompileTask* task) o = Universe::arithmetic_exception_instance(); assert(o != nullptr, "should have been initialized"); _ArithmeticException_instance = get_object(o)->as_instance(); + o = Universe::array_index_out_of_bounds_exception_instance(); + assert(o != nullptr, "should have been initialized"); + _ArrayIndexOutOfBoundsException_instance = get_object(o)->as_instance(); + o = Universe::array_store_exception_instance(); + assert(o != nullptr, "should have been initialized"); + _ArrayStoreException_instance = get_object(o)->as_instance(); + o = Universe::class_cast_exception_instance(); + assert(o != nullptr, "should have been initialized"); + _ClassCastException_instance = get_object(o)->as_instance(); - _ArrayIndexOutOfBoundsException_instance = nullptr; - _ArrayStoreException_instance = nullptr; - _ClassCastException_instance = nullptr; _the_null_string = nullptr; _the_min_jint_string = nullptr; @@ -363,29 +365,6 @@ void ciEnv::cache_dtrace_flags() { _dtrace_alloc_probes = DTraceAllocProbes; } -// ------------------------------------------------------------------ -// helper for lazy exception creation -ciInstance* ciEnv::get_or_create_exception(jobject& handle, Symbol* name) { - VM_ENTRY_MARK; - if (handle == nullptr) { - // Cf. universe.cpp, creation of Universe::_null_ptr_exception_instance. - InstanceKlass* ik = SystemDictionary::find_instance_klass(THREAD, name, Handle(), Handle()); - jobject objh = nullptr; - if (ik != nullptr) { - oop obj = ik->allocate_instance(THREAD); - if (!HAS_PENDING_EXCEPTION) - objh = JNIHandles::make_global(Handle(THREAD, obj)); - } - if (HAS_PENDING_EXCEPTION) { - CLEAR_PENDING_EXCEPTION; - } else { - handle = objh; - } - } - oop obj = JNIHandles::resolve(handle); - return obj == nullptr? nullptr: get_object(obj)->as_instance(); -} - ciInstanceKlass* ciEnv::get_box_klass_for_primitive_type(BasicType type) { switch (type) { case T_BOOLEAN: return Boolean_klass(); @@ -403,31 +382,6 @@ ciInstanceKlass* ciEnv::get_box_klass_for_primitive_type(BasicType type) { } } -ciInstance* ciEnv::ArrayIndexOutOfBoundsException_instance() { - if (_ArrayIndexOutOfBoundsException_instance == nullptr) { - _ArrayIndexOutOfBoundsException_instance - = get_or_create_exception(_ArrayIndexOutOfBoundsException_handle, - vmSymbols::java_lang_ArrayIndexOutOfBoundsException()); - } - return _ArrayIndexOutOfBoundsException_instance; -} -ciInstance* ciEnv::ArrayStoreException_instance() { - if (_ArrayStoreException_instance == nullptr) { - _ArrayStoreException_instance - = get_or_create_exception(_ArrayStoreException_handle, - vmSymbols::java_lang_ArrayStoreException()); - } - return _ArrayStoreException_instance; -} -ciInstance* ciEnv::ClassCastException_instance() { - if (_ClassCastException_instance == nullptr) { - _ClassCastException_instance - = get_or_create_exception(_ClassCastException_handle, - vmSymbols::java_lang_ClassCastException()); - } - return _ClassCastException_instance; -} - ciInstance* ciEnv::the_null_string() { if (_the_null_string == nullptr) { VM_ENTRY_MARK; diff --git a/src/hotspot/share/ci/ciEnv.hpp b/src/hotspot/share/ci/ciEnv.hpp index 5ee9b420033..6c66633ee17 100644 --- a/src/hotspot/share/ci/ciEnv.hpp +++ b/src/hotspot/share/ci/ciEnv.hpp @@ -94,10 +94,6 @@ private: static ciInstanceKlass* _unloaded_ciinstance_klass; static ciObjArrayKlass* _unloaded_ciobjarrayklass; - static jobject _ArrayIndexOutOfBoundsException_handle; - static jobject _ArrayStoreException_handle; - static jobject _ClassCastException_handle; - ciInstance* _NullPointerException_instance; ciInstance* _ArithmeticException_instance; ciInstance* _ArrayIndexOutOfBoundsException_instance; @@ -230,8 +226,6 @@ private: ciMethod* get_method_from_handle(Method* method); - ciInstance* get_or_create_exception(jobject& handle, Symbol* name); - // Get a ciMethod representing either an unfound method or // a method with an unloaded holder. Ensures uniqueness of // the result. @@ -402,11 +396,18 @@ public: assert(_ArithmeticException_instance != nullptr, "initialization problem"); return _ArithmeticException_instance; } - - // Lazy constructors: - ciInstance* ArrayIndexOutOfBoundsException_instance(); - ciInstance* ArrayStoreException_instance(); - ciInstance* ClassCastException_instance(); + ciInstance* ArrayIndexOutOfBoundsException_instance() { + assert(_ArrayIndexOutOfBoundsException_instance != nullptr, "initialization problem"); + return _ArrayIndexOutOfBoundsException_instance; + } + ciInstance* ArrayStoreException_instance() { + assert(_ArrayStoreException_instance != nullptr, "initialization problem"); + return _ArrayStoreException_instance; + } + ciInstance* ClassCastException_instance() { + assert(_ClassCastException_instance != nullptr, "initialization problem"); + return _ClassCastException_instance; + } ciInstance* the_null_string(); ciInstance* the_min_jint_string(); diff --git a/src/hotspot/share/memory/universe.cpp b/src/hotspot/share/memory/universe.cpp index 8f20f331235..4b6554173f9 100644 --- a/src/hotspot/share/memory/universe.cpp +++ b/src/hotspot/share/memory/universe.cpp @@ -229,6 +229,9 @@ public: static BuiltinException _null_ptr_exception; static BuiltinException _arithmetic_exception; static BuiltinException _internal_error; +static BuiltinException _array_index_out_of_bounds_exception; +static BuiltinException _array_store_exception; +static BuiltinException _class_cast_exception; objArrayOop Universe::the_empty_class_array () { return (objArrayOop)_the_empty_class_array.resolve(); @@ -246,6 +249,9 @@ oop Universe::the_min_jint_string() { return _the_min_jint_string. oop Universe::null_ptr_exception_instance() { return _null_ptr_exception.instance(); } oop Universe::arithmetic_exception_instance() { return _arithmetic_exception.instance(); } oop Universe::internal_error_instance() { return _internal_error.instance(); } +oop Universe::array_index_out_of_bounds_exception_instance() { return _array_index_out_of_bounds_exception.instance(); } +oop Universe::array_store_exception_instance() { return _array_store_exception.instance(); } +oop Universe::class_cast_exception_instance() { return _class_cast_exception.instance(); } oop Universe::the_null_sentinel() { return _the_null_sentinel.resolve(); } @@ -302,6 +308,9 @@ void Universe::archive_exception_instances() { _null_ptr_exception.store_in_cds(); _arithmetic_exception.store_in_cds(); _internal_error.store_in_cds(); + _array_index_out_of_bounds_exception.store_in_cds(); + _array_store_exception.store_in_cds(); + _class_cast_exception.store_in_cds(); } void Universe::load_archived_object_instances() { @@ -318,6 +327,9 @@ void Universe::load_archived_object_instances() { _null_ptr_exception.load_from_cds(); _arithmetic_exception.load_from_cds(); _internal_error.load_from_cds(); + _array_index_out_of_bounds_exception.load_from_cds(); + _array_store_exception.load_from_cds(); + _class_cast_exception.load_from_cds(); } } #endif @@ -334,6 +346,9 @@ void Universe::serialize(SerializeClosure* f) { _null_ptr_exception.serialize(f); _arithmetic_exception.serialize(f); _internal_error.serialize(f); + _array_index_out_of_bounds_exception.serialize(f); + _array_store_exception.serialize(f); + _class_cast_exception.serialize(f); #endif f->do_ptr(&_fillerArrayKlass); @@ -1083,10 +1098,12 @@ bool universe_post_init() { Universe::_delayed_stack_overflow_error_message = OopHandle(Universe::vm_global(), instance); } - // Setup preallocated NullPointerException/ArithmeticException - // (used for a cheap & dirty solution in compiler exception handling) + // Setup preallocated exceptions used for a cheap & dirty solution in compiler exception handling _null_ptr_exception.init_if_empty(vmSymbols::java_lang_NullPointerException(), CHECK_false); _arithmetic_exception.init_if_empty(vmSymbols::java_lang_ArithmeticException(), CHECK_false); + _array_index_out_of_bounds_exception.init_if_empty(vmSymbols::java_lang_ArrayIndexOutOfBoundsException(), CHECK_false); + _array_store_exception.init_if_empty(vmSymbols::java_lang_ArrayStoreException(), CHECK_false); + _class_cast_exception.init_if_empty(vmSymbols::java_lang_ClassCastException(), CHECK_false); // Virtual Machine Error for when we get into a situation we can't resolve Klass* k = vmClasses::InternalError_klass(); diff --git a/src/hotspot/share/memory/universe.hpp b/src/hotspot/share/memory/universe.hpp index c7bdb8f2339..0a3b9eaae01 100644 --- a/src/hotspot/share/memory/universe.hpp +++ b/src/hotspot/share/memory/universe.hpp @@ -230,6 +230,9 @@ class Universe: AllStatic { static oop null_ptr_exception_instance(); static oop arithmetic_exception_instance(); static oop internal_error_instance(); + static oop array_index_out_of_bounds_exception_instance(); + static oop array_store_exception_instance(); + static oop class_cast_exception_instance(); static oop vm_exception() { return internal_error_instance(); } static Array* the_array_interfaces_array() { return _the_array_interfaces_array; } diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp index af904bdcfcd..8266bd86a96 100644 --- a/src/hotspot/share/runtime/threads.cpp +++ b/src/hotspot/share/runtime/threads.cpp @@ -393,6 +393,7 @@ void Threads::initialize_java_lang_classes(JavaThread* main_thread, TRAPS) { initialize_class(vmSymbols::java_lang_ClassCastException(), CHECK); initialize_class(vmSymbols::java_lang_ArrayStoreException(), CHECK); initialize_class(vmSymbols::java_lang_ArithmeticException(), CHECK); + initialize_class(vmSymbols::java_lang_ArrayIndexOutOfBoundsException(), CHECK); initialize_class(vmSymbols::java_lang_StackOverflowError(), CHECK); initialize_class(vmSymbols::java_lang_IllegalMonitorStateException(), CHECK); initialize_class(vmSymbols::java_lang_IllegalArgumentException(), CHECK);