8029178: Parallel class loading test anonymous-simple gets SIGSEGV in Metaspace::contains

Metaspace::contains cannot look at purged metaspaces while CMS concurrently deallocates them.

Reviewed-by: mgerdin, sspitsyn, jmasa

hotspot/src/share/vm/classfile/classLoaderData.cpp

@@ -648,12 +648,12 @@ GrowableArray<ClassLoaderData*>* ClassLoaderDataGraph::new_clds() {
   return array;
 }
 
-#ifndef PRODUCT
-// for debugging and hsfind(x)
-bool ClassLoaderDataGraph::contains(address x) {
-  // I think we need the _metaspace_lock taken here because the class loader
-  // data graph could be changing while we are walking it (new entries added,
-  // new entries being unloaded, etc).
+// For profiling and hsfind() only.  Otherwise, this is unsafe (and slow).  This
+// is done lock free to avoid lock inversion problems.  It is safe because
+// new ClassLoaderData are added to the end of the CLDG, and only removed at
+// safepoint.  The _unloading list can be deallocated concurrently with CMS so
+// this doesn't look in metaspace for classes that have been unloaded.
+bool ClassLoaderDataGraph::contains(const void* x) {
   if (DumpSharedSpaces) {
     // There are only two metaspaces to worry about.
     ClassLoaderData* ncld = ClassLoaderData::the_null_class_loader_data();
@@ -670,16 +670,11 @@ bool ClassLoaderDataGraph::contains(address x) {
     }
   }
 
-  // Could also be on an unloading list which is okay, ie. still allocated
-  // for a little while.
-  for (ClassLoaderData* ucld = _unloading; ucld != NULL; ucld = ucld->next()) {
-    if (ucld->metaspace_or_null() != NULL && ucld->metaspace_or_null()->contains(x)) {
-      return true;
-    }
-  }
+  // Do not check unloading list because deallocation can be concurrent.
   return false;
 }
 
+#ifndef PRODUCT
 bool ClassLoaderDataGraph::contains_loader_data(ClassLoaderData* loader_data) {
   for (ClassLoaderData* data = _head; data != NULL; data = data->next()) {
     if (loader_data == data) {

hotspot/src/share/vm/classfile/classLoaderData.hpp

@@ -90,9 +90,9 @@ class ClassLoaderDataGraph : public AllStatic {
   static void dump() { dump_on(tty); }
   static void verify();
 
-#ifndef PRODUCT
   // expensive test for pointer in metaspace for debugging
-  static bool contains(address x);
+  static bool contains(const void* x);
+#ifndef PRODUCT
   static bool contains_loader_data(ClassLoaderData* loader_data);
 #endif
 

hotspot/src/share/vm/code/dependencies.cpp

@@ -655,8 +655,6 @@ inline Metadata* Dependencies::DepStream::recorded_metadata_at(int i) {
   } else {
     o = _deps->oop_recorder()->metadata_at(i);
   }
-  assert(o == NULL || o->is_metaspace_object(),
-         err_msg("Should be metadata " PTR_FORMAT, o));
   return o;
 }
 

hotspot/src/share/vm/memory/allocation.cpp

@@ -71,9 +71,8 @@ bool MetaspaceObj::is_shared() const {
   return MetaspaceShared::is_in_shared_space(this);
 }
 
-
 bool MetaspaceObj::is_metaspace_object() const {
-  return Metaspace::contains((void*)this);
+  return ClassLoaderDataGraph::contains((void*)this);
 }
 
 void MetaspaceObj::print_address_on(outputStream* st) const {

hotspot/src/share/vm/memory/allocation.hpp

@@ -264,7 +264,7 @@ class ClassLoaderData;
 
 class MetaspaceObj {
  public:
-  bool is_metaspace_object() const;  // more specific test but slower
+  bool is_metaspace_object() const;
   bool is_shared() const;
   void print_address_on(outputStream* st) const;  // nonvirtual address printing
 

hotspot/src/share/vm/memory/metachunk.hpp

@@ -143,6 +143,8 @@ class Metachunk : public Metabase<Metachunk> {
   void set_is_tagged_free(bool v) { _is_tagged_free = v; }
 #endif
 
+  bool contains(const void* ptr) { return bottom() <= ptr && ptr < _top; }
+
   NOT_PRODUCT(void mangle();)
 
   void print_on(outputStream* st) const;

hotspot/src/share/vm/memory/metaspace.cpp

@@ -513,8 +513,6 @@ class VirtualSpaceList : public CHeapObj<mtClass> {
   // Unlink empty VirtualSpaceNodes and free it.
   void purge(ChunkManager* chunk_manager);
 
-  bool contains(const void *ptr);
-
   void print_on(outputStream* st) const;
 
   class VirtualSpaceListIterator : public StackObj {
@@ -558,7 +556,7 @@ class SpaceManager : public CHeapObj<mtClass> {
 
  private:
 
-  // protects allocations and contains.
+  // protects allocations
   Mutex* const _lock;
 
   // Type of metadata allocated.
@@ -595,7 +593,11 @@ class SpaceManager : public CHeapObj<mtClass> {
  private:
   // Accessors
   Metachunk* chunks_in_use(ChunkIndex index) const { return _chunks_in_use[index]; }
-  void set_chunks_in_use(ChunkIndex index, Metachunk* v) { _chunks_in_use[index] = v; }
+  void set_chunks_in_use(ChunkIndex index, Metachunk* v) {
+    // ensure lock-free iteration sees fully initialized node
+    OrderAccess::storestore();
+    _chunks_in_use[index] = v;
+  }
 
   BlockFreelist* block_freelists() const {
     return (BlockFreelist*) &_block_freelists;
@@ -708,6 +710,8 @@ class SpaceManager : public CHeapObj<mtClass> {
   void print_on(outputStream* st) const;
   void locked_print_chunks_in_use_on(outputStream* st) const;
 
+  bool contains(const void *ptr);
+
   void verify();
   void verify_chunk_size(Metachunk* chunk);
   NOT_PRODUCT(void mangle_freed_chunks();)
@@ -1159,8 +1163,6 @@ bool VirtualSpaceList::create_new_virtual_space(size_t vs_word_size) {
   } else {
     assert(new_entry->reserved_words() == vs_word_size,
         "Reserved memory size differs from requested memory size");
-    // ensure lock-free iteration sees fully initialized node
-    OrderAccess::storestore();
     link_vs(new_entry);
     return true;
   }
@@ -1287,19 +1289,6 @@ void VirtualSpaceList::print_on(outputStream* st) const {
   }
 }
 
-bool VirtualSpaceList::contains(const void *ptr) {
-  VirtualSpaceNode* list = virtual_space_list();
-  VirtualSpaceListIterator iter(list);
-  while (iter.repeat()) {
-    VirtualSpaceNode* node = iter.get_next();
-    if (node->reserved()->contains(ptr)) {
-      return true;
-    }
-  }
-  return false;
-}
-
-
 // MetaspaceGC methods
 
 // VM_CollectForMetadataAllocation is the vm operation used to GC.
@@ -2392,6 +2381,21 @@ MetaWord* SpaceManager::allocate_work(size_t word_size) {
   return result;
 }
 
+// This function looks at the chunks in the metaspace without locking.
+// The chunks are added with store ordering and not deleted except for at
+// unloading time.
+bool SpaceManager::contains(const void *ptr) {
+  for (ChunkIndex i = ZeroIndex; i < NumberOfInUseLists; i = next_chunk_index(i))
+  {
+    Metachunk* curr = chunks_in_use(i);
+    while (curr != NULL) {
+      if (curr->contains(ptr)) return true;
+      curr = curr->next();
+    }
+  }
+  return false;
+}
+
 void SpaceManager::verify() {
   // If there are blocks in the dictionary, then
   // verfication of chunks does not work since
@@ -3463,17 +3467,12 @@ void Metaspace::print_on(outputStream* out) const {
   }
 }
 
-bool Metaspace::contains(const void * ptr) {
-  if (MetaspaceShared::is_in_shared_space(ptr)) {
-    return true;
+bool Metaspace::contains(const void* ptr) {
+  if (vsm()->contains(ptr)) return true;
+  if (using_class_space()) {
+    return class_vsm()->contains(ptr);
   }
-  // This is checked while unlocked.  As long as the virtualspaces are added
-  // at the end, the pointer will be in one of them.  The virtual spaces
-  // aren't deleted presently.  When they are, some sort of locking might
-  // be needed.  Note, locking this can cause inversion problems with the
-  // caller in MetaspaceObj::is_metadata() function.
-  return space_list()->contains(ptr) ||
-         (using_class_space() && class_space_list()->contains(ptr));
+  return false;
 }
 
 void Metaspace::verify() {

hotspot/src/share/vm/memory/metaspace.hpp

@@ -225,7 +225,7 @@ class Metaspace : public CHeapObj<mtClass> {
   MetaWord* expand_and_allocate(size_t size,
                                 MetadataType mdtype);
 
-  static bool contains(const void *ptr);
+  bool contains(const void* ptr);
   void dump(outputStream* const out) const;
 
   // Free empty virtualspaces

hotspot/src/share/vm/oops/klass.cpp

@@ -376,8 +376,6 @@ void Klass::append_to_sibling_list() {
 }
 
 bool Klass::is_loader_alive(BoolObjectClosure* is_alive) {
-  assert(ClassLoaderDataGraph::contains((address)this), "is in the metaspace");
-
 #ifdef ASSERT
   // The class is alive iff the class loader is alive.
   oop loader = class_loader();

hotspot/src/share/vm/runtime/os.cpp

@@ -1081,7 +1081,6 @@ void os::print_location(outputStream* st, intptr_t x, bool verbose) {
 
   }
 
-#ifndef PRODUCT
   // Check if in metaspace.
   if (ClassLoaderDataGraph::contains((address)addr)) {
     // Use addr->print() from the debugger instead (not here)
@@ -1089,7 +1088,6 @@ void os::print_location(outputStream* st, intptr_t x, bool verbose) {
                  " is pointing into metadata", addr);
     return;
   }
-#endif
 
   // Try an OS specific find
   if (os::find(addr, st)) {