8382170: Assert in AnyObj::operator delete due to race condition in in_aot_cache()

src/hotspot/share/cds/aotMetaspace.hpp

@@ -104,8 +104,13 @@ class AOTMetaspace : AllStatic {
 
   // Return true if given address is in the shared metaspace regions (i.e., excluding the
   // mapped heap region.)
+  //
+  // Unlike MetaspaceObj::is_pointer_in_aot_cache(), this function should be called in contexts
+  // where the AOT metaspace range is known to have been initialized (which happens very early
+  // in VM boostrap), so the caller doesn't need to explicity check for
+  // MetaspaceObj::aot_metaspace_range_initialized().
   static bool in_aot_cache(const void* p) {
-    return MetaspaceObj::in_aot_cache((const MetaspaceObj*)p);
+    return MetaspaceObj::is_pointer_in_aot_cache(p);
   }
 
   static void set_aot_metaspace_range(void* base, void *static_top, void* top) NOT_CDS_RETURN;

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -1277,7 +1277,7 @@ unsigned int SystemDictionaryShared::hash_for_shared_dictionary(address ptr) {
     uintx offset = ArchiveBuilder::current()->any_to_offset(ptr);
     unsigned int hash = primitive_hash<uintx>(offset);
     DEBUG_ONLY({
-        if (MetaspaceObj::in_aot_cache((const MetaspaceObj*)ptr)) {
+        if (AOTMetaspace::in_aot_cache(ptr)) {
           assert(hash == SystemDictionaryShared::hash_for_shared_dictionary_quick(ptr), "must be");
         }
       });

src/hotspot/share/classfile/systemDictionaryShared.hpp

@@ -25,6 +25,7 @@
 #ifndef SHARE_CLASSFILE_SYSTEMDICTIONARYSHARED_HPP
 #define SHARE_CLASSFILE_SYSTEMDICTIONARYSHARED_HPP
 
+#include "cds/aotMetaspace.hpp"
 #include "cds/cds_globals.hpp"
 #include "cds/dumpTimeClassInfo.hpp"
 #include "cds/filemap.hpp"
@@ -312,7 +313,7 @@ class SystemDictionaryShared: public SystemDictionary {
 
   template <typename T>
   static unsigned int hash_for_shared_dictionary_quick(T* ptr) {
-    assert(MetaspaceObj::in_aot_cache((const MetaspaceObj*)ptr), "must be");
+    assert(AOTMetaspace::in_aot_cache(ptr), "must be");
     assert(ptr > (T*)SharedBaseAddress, "must be");
     uintx offset = uintx(ptr) - uintx(SharedBaseAddress);
     return primitive_hash<uintx>(offset);

src/hotspot/share/memory/allocation.cpp

@@ -28,6 +28,7 @@
 #include "memory/metaspace.hpp"
 #include "memory/resourceArea.hpp"
 #include "nmt/memTracker.hpp"
+#include "runtime/atomicAccess.hpp"
 #include "runtime/os.hpp"
 #include "runtime/task.hpp"
 #include "utilities/ostream.hpp"
@@ -68,6 +69,19 @@ void FreeHeap(void* p) {
 
 void* MetaspaceObj::_aot_metaspace_base = nullptr;
 void* MetaspaceObj::_aot_metaspace_top  = nullptr;
+volatile bool MetaspaceObj::_aot_metaspace_range_initialized = false;
+
+void MetaspaceObj::set_aot_metaspace_range(void* base, void* top) {
+  _aot_metaspace_base = base;
+  _aot_metaspace_top = top;
+  AtomicAccess::release_store(&_aot_metaspace_range_initialized, true);
+}
+
+#if INCLUDE_CDS
+bool MetaspaceObj::aot_metaspace_range_initialized() {
+  return AtomicAccess::load_acquire(&_aot_metaspace_range_initialized);
+}
+#endif
 
 void* MetaspaceObj::operator new(size_t size, ClassLoaderData* loader_data,
                                  size_t word_size,
@@ -168,7 +182,8 @@ void AnyObj::set_in_aot_cache() {
 }
 
 bool AnyObj::in_aot_cache() const {
-  if (AOTMetaspace::in_aot_cache(this)) {
+  if (MetaspaceObj::aot_metaspace_range_initialized() && MetaspaceObj::is_pointer_in_aot_cache(this)) {
+    // "this" can be AOT space only if aot_metaspace_range_initialized()
     precond(_allocation_t[0] == 0);
     precond(_allocation_t[1] == 0);
     return true;

src/hotspot/share/memory/allocation.hpp

@@ -267,7 +267,7 @@ class MetaspaceObj {
   // When AOT/CDS is not enabled, both pointers are set to null.
   static void* _aot_metaspace_base;  // (inclusive) low address
   static void* _aot_metaspace_top;   // (exclusive) high address
-
+  static volatile bool _aot_metaspace_range_initialized;
  public:
 
   // Returns true if the pointer points to a valid MetaspaceObj. A valid
@@ -276,24 +276,33 @@ class MetaspaceObj {
   static bool is_valid(const MetaspaceObj* p);
 
 #if INCLUDE_CDS
-  static bool in_aot_cache(const MetaspaceObj* p) {
+  // Caller must check aot_metaspace_range_initialized() if it can call this
+  // function in very early VM bootstrap.
+  inline static bool is_pointer_in_aot_cache(const void* p) {
+    // Don't assert with aot_metaspace_range_initialized(), as that has side effect
+    // of making the range visible to the current thread, even if the caller
+    // forgets to call aot_metaspace_range_initialized().
+    assert(_aot_metaspace_range_initialized, "range not initialized");
+
     // If no shared metaspace regions are mapped, _aot_metaspace_{base,top} will
     // both be null and all values of p will be rejected quickly.
-    return (((void*)p) < _aot_metaspace_top &&
-            ((void*)p) >= _aot_metaspace_base);
+    return (p < _aot_metaspace_top &&
+            p >= _aot_metaspace_base);
   }
-  bool in_aot_cache() const { return MetaspaceObj::in_aot_cache(this); }
+
+  bool in_aot_cache() const {
+    return is_pointer_in_aot_cache(this);
+  }
+  static bool aot_metaspace_range_initialized();
 #else
-  static bool in_aot_cache(const MetaspaceObj* p) { return false; }
+  static bool is_pointer_in_aot_cache(const void* p) { return false; }
   bool in_aot_cache() const { return false; }
+  static bool aot_metaspace_range_initialized() { return false; }
 #endif
 
   void print_address_on(outputStream* st) const;  // nonvirtual address printing
 
-  static void set_aot_metaspace_range(void* base, void* top) {
-    _aot_metaspace_base = base;
-    _aot_metaspace_top = top;
-  }
+  static void set_aot_metaspace_range(void* base, void* top);
 
   static void* aot_metaspace_base() { return _aot_metaspace_base; }
   static void* aot_metaspace_top()  { return _aot_metaspace_top;  }

src/hotspot/share/memory/metaspace.cpp

@@ -739,6 +739,9 @@ void Metaspace::global_initialize() {
     AOTMetaspace::initialize_runtime_shared_and_meta_spaces();
     // If any of the archived space fails to map, UseSharedSpaces
     // is reset to false.
+  } else {
+    // Trivially set the range to empty to satisfy the assert in MetaspaceObj::is_pointer_in_aot_cache()
+    MetaspaceObj::set_aot_metaspace_range(nullptr, nullptr);
   }
 #endif // INCLUDE_CDS
 

src/hotspot/share/oops/trainingData.cpp

@@ -727,7 +727,7 @@ void TrainingData::metaspace_pointers_do(MetaspaceClosure* iter) {
 }
 
 bool TrainingData::Key::can_compute_cds_hash(const Key* const& k) {
-  return k->meta() == nullptr || MetaspaceObj::in_aot_cache(k->meta());
+  return k->meta() == nullptr || k->meta()->in_aot_cache();
 }
 
 uint TrainingData::Key::cds_hash(const Key* const& k) {

src/hotspot/share/utilities/growableArray.cpp

@@ -57,7 +57,7 @@ void* GrowableArrayCHeapAllocator::allocate(int max, int element_size, MemTag me
 }
 
 void GrowableArrayCHeapAllocator::deallocate(void* elements) {
-  if (!AOTMetaspace::in_aot_cache(elements)) {
+  if (!MetaspaceObj::aot_metaspace_range_initialized() || !MetaspaceObj::is_pointer_in_aot_cache(elements)) {
     FreeHeap(elements);
   }
 }