From 77519e5f4fe75f953c02fb3f15b7f9a58c933fea Mon Sep 17 00:00:00 2001 From: Robbin Ehn Date: Tue, 14 Feb 2023 14:38:46 +0000 Subject: [PATCH] 8302354: InstanceKlass init state/thread should be atomic Reviewed-by: coleenp, dholmes --- src/hotspot/share/jvmci/vmStructs_jvmci.cpp | 4 ++-- src/hotspot/share/oops/cpCache.cpp | 2 +- src/hotspot/share/oops/instanceKlass.cpp | 4 ++-- src/hotspot/share/oops/instanceKlass.hpp | 15 ++++++++------- src/hotspot/share/runtime/vmStructs.cpp | 4 ++-- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/hotspot/share/jvmci/vmStructs_jvmci.cpp b/src/hotspot/share/jvmci/vmStructs_jvmci.cpp index bb21ee78fa8..f53da656e8b 100644 --- a/src/hotspot/share/jvmci/vmStructs_jvmci.cpp +++ b/src/hotspot/share/jvmci/vmStructs_jvmci.cpp @@ -158,8 +158,8 @@ \ nonstatic_field(InstanceKlass, _fields, Array*) \ nonstatic_field(InstanceKlass, _constants, ConstantPool*) \ - nonstatic_field(InstanceKlass, _init_state, InstanceKlass::ClassState) \ - nonstatic_field(InstanceKlass, _init_thread, Thread*) \ + volatile_nonstatic_field(InstanceKlass, _init_state, InstanceKlass::ClassState) \ + volatile_nonstatic_field(InstanceKlass, _init_thread, JavaThread*) \ nonstatic_field(InstanceKlass, _misc_flags._flags, u2) \ nonstatic_field(InstanceKlass, _annotations, Annotations*) \ \ diff --git a/src/hotspot/share/oops/cpCache.cpp b/src/hotspot/share/oops/cpCache.cpp index 05b5efd4ecd..8a884b508a1 100644 --- a/src/hotspot/share/oops/cpCache.cpp +++ b/src/hotspot/share/oops/cpCache.cpp @@ -251,7 +251,7 @@ void ConstantPoolCacheEntry::set_direct_or_vtable_call(Bytecodes::Code invoke_co } if (invoke_code == Bytecodes::_invokestatic) { assert(method->method_holder()->is_initialized() || - method->method_holder()->is_init_thread(Thread::current()), + method->method_holder()->is_init_thread(JavaThread::current()), "invalid class initialization state for invoke_static"); if (!VM_Version::supports_fast_class_init_checks() && method->needs_clinit_barrier()) { diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp index 5a90bb0a0c3..03ebf658553 100644 --- a/src/hotspot/share/oops/instanceKlass.cpp +++ b/src/hotspot/share/oops/instanceKlass.cpp @@ -3354,7 +3354,7 @@ static void print_vtable(vtableEntry* start, int len, outputStream* st) { } const char* InstanceKlass::init_state_name() const { - return state_names[_init_state]; + return state_names[init_state()]; } void InstanceKlass::print_on(outputStream* st) const { @@ -3898,7 +3898,7 @@ void InstanceKlass::set_init_state(ClassState state) { assert(good_state || state == allocated || link_failed, "illegal state transition"); #endif assert(_init_thread == nullptr, "should be cleared before state change"); - _init_state = state; + Atomic::store(&_init_state, state); } #if INCLUDE_JVMTI diff --git a/src/hotspot/share/oops/instanceKlass.hpp b/src/hotspot/share/oops/instanceKlass.hpp index 74917f02495..3264113e597 100644 --- a/src/hotspot/share/oops/instanceKlass.hpp +++ b/src/hotspot/share/oops/instanceKlass.hpp @@ -226,15 +226,15 @@ class InstanceKlass: public Klass { // _misc_flags right now. bool _is_marked_dependent; // used for marking during flushing and deoptimization - ClassState _init_state; // state of class + volatile ClassState _init_state; // state of class u1 _reference_type; // reference type // State is set while executing, eventually atomically to not disturb other state InstanceKlassFlags _misc_flags; - Monitor* _init_monitor; // mutual exclusion to _init_state and _init_thread. - Thread* _init_thread; // Pointer to current thread doing initialization (to handle recursive initialization) + Monitor* _init_monitor; // mutual exclusion to _init_state and _init_thread. + JavaThread* volatile _init_thread; // Pointer to current thread doing initialization (to handle recursive initialization) OopMapCache* volatile _oop_map_cache; // OopMapCache for all methods in the klass (allocated lazily) JNIid* _jni_ids; // First JNI identifier for static fields in this class @@ -511,7 +511,7 @@ public: bool is_not_initialized() const { return init_state() < being_initialized; } bool is_being_initialized() const { return init_state() == being_initialized; } bool is_in_error_state() const { return init_state() == initialization_error; } - bool is_init_thread(Thread *thread) { return thread == _init_thread; } + bool is_init_thread(JavaThread *thread) { return thread == Atomic::load(&_init_thread); } ClassState init_state() const { return Atomic::load(&_init_state); } const char* init_state_name() const; bool is_rewritten() const { return _misc_flags.rewritten(); } @@ -1077,9 +1077,10 @@ public: // initialization state void set_init_state(ClassState state); void set_rewritten() { _misc_flags.set_rewritten(true); } - void set_init_thread(Thread *thread) { - assert(thread == nullptr || _init_thread == nullptr, "Only one thread is allowed to own initialization"); - _init_thread = thread; + void set_init_thread(JavaThread *thread) { + assert((thread == JavaThread::current() && _init_thread == nullptr) || + (thread == nullptr && _init_thread == JavaThread::current()), "Only one thread is allowed to own initialization"); + Atomic::store(&_init_thread, thread); } // The RedefineClasses() API can cause new method idnums to be needed diff --git a/src/hotspot/share/runtime/vmStructs.cpp b/src/hotspot/share/runtime/vmStructs.cpp index d62342578f6..96623646ac9 100644 --- a/src/hotspot/share/runtime/vmStructs.cpp +++ b/src/hotspot/share/runtime/vmStructs.cpp @@ -235,8 +235,8 @@ nonstatic_field(InstanceKlass, _static_oop_field_count, u2) \ nonstatic_field(InstanceKlass, _nonstatic_oop_map_size, int) \ nonstatic_field(InstanceKlass, _is_marked_dependent, bool) \ - nonstatic_field(InstanceKlass, _init_state, InstanceKlass::ClassState) \ - nonstatic_field(InstanceKlass, _init_thread, Thread*) \ + volatile_nonstatic_field(InstanceKlass, _init_state, InstanceKlass::ClassState) \ + volatile_nonstatic_field(InstanceKlass, _init_thread, JavaThread*) \ nonstatic_field(InstanceKlass, _itable_len, int) \ nonstatic_field(InstanceKlass, _reference_type, u1) \ volatile_nonstatic_field(InstanceKlass, _oop_map_cache, OopMapCache*) \