8215889: assert(!_unloading) failed: This oop is not available to unloading class loader data with ZGC

Reviewed-by: coleenp, neliasso

src/hotspot/share/ci/ciEnv.hpp

@@ -46,6 +46,7 @@ class ciEnv : StackObj {
 
   friend class CompileBroker;
   friend class Dependencies;  // for get_object, during logging
+  friend class PrepareExtraDataClosure;
 
 private:
   Arena*           _arena;       // Alias for _ciEnv_arena except in init_shared_objects()
@@ -188,6 +189,10 @@ private:
     }
   }
 
+  ciMetadata* cached_metadata(Metadata* o) {
+    return _factory->cached_metadata(o);
+  }
+
   ciInstance* get_instance(oop o) {
     if (o == NULL) return NULL;
     return get_object(o)->as_instance();

src/hotspot/share/ci/ciMethodData.cpp

@@ -78,10 +78,81 @@ ciMethodData::ciMethodData() : ciMetadata(NULL) {
   _parameters = NULL;
 }
 
-void ciMethodData::load_extra_data() {
+// Check for entries that reference an unloaded method
+class PrepareExtraDataClosure : public CleanExtraDataClosure {
+  MethodData*            _mdo;
+  uint64_t               _safepoint_counter;
+  GrowableArray<Method*> _uncached_methods;
+
+public:
+  PrepareExtraDataClosure(MethodData* mdo)
+    : _mdo(mdo),
+      _safepoint_counter(SafepointSynchronize::safepoint_counter()),
+      _uncached_methods()
+  { }
+
+  bool is_live(Method* m) {
+    if (!m->method_holder()->is_loader_alive()) {
+      return false;
+    }
+    if (CURRENT_ENV->cached_metadata(m) == NULL) {
+      // Uncached entries need to be pre-populated.
+      _uncached_methods.append(m);
+    }
+    return true;
+  }
+
+  bool has_safepointed() {
+    return SafepointSynchronize::safepoint_counter() != _safepoint_counter;
+  }
+
+  bool finish() {
+    if (_uncached_methods.length() == 0) {
+      // Preparation finished iff all Methods* were already cached.
+      return true;
+    }
+    // Holding locks through safepoints is bad practice.
+    MutexUnlocker mu(_mdo->extra_data_lock());
+    for (int i = 0; i < _uncached_methods.length(); ++i) {
+      if (has_safepointed()) {
+        // The metadata in the growable array might contain stale
+        // entries after a safepoint.
+        return false;
+      }
+      Method* method = _uncached_methods.at(i);
+      // Populating ciEnv caches may cause safepoints due
+      // to taking the Compile_lock with safepoint checks.
+      (void)CURRENT_ENV->get_method(method);
+    }
+    return false;
+  }
+};
+
+void ciMethodData::prepare_metadata() {
   MethodData* mdo = get_MethodData();
 
+  for (;;) {
+    ResourceMark rm;
+    PrepareExtraDataClosure cl(mdo);
+    mdo->clean_extra_data(&cl);
+    if (cl.finish()) {
+      // When encountering uncached metadata, the Compile_lock might be
+      // acquired when creating ciMetadata handles, causing safepoints
+      // which requires a new round of preparation to clean out potentially
+      // new unloading metadata.
+      return;
+    }
+  }
+}
+
+void ciMethodData::load_extra_data() {
+  MethodData* mdo = get_MethodData();
   MutexLocker ml(mdo->extra_data_lock());
+  // Deferred metadata cleaning due to concurrent class unloading.
+  prepare_metadata();
+  // After metadata preparation, there is no stale metadata,
+  // and no safepoints can introduce more stale metadata.
+  NoSafepointVerifier no_safepoint;
 
   // speculative trap entries also hold a pointer to a Method so need to be translated
   DataLayout* dp_src  = mdo->extra_data_base();
@@ -94,7 +165,7 @@ void ciMethodData::load_extra_data() {
     // New traps in the MDO may have been added since we copied the
     // data (concurrent deoptimizations before we acquired
     // extra_data_lock above) or can be removed (a safepoint may occur
-    // in the translate_from call below) as we translate the copy:
+    // in the prepare_metadata call above) as we translate the copy:
     // update the copy as we go.
     int tag = dp_src->tag();
     if (tag != DataLayout::arg_info_data_tag) {
@@ -105,11 +176,7 @@ void ciMethodData::load_extra_data() {
     case DataLayout::speculative_trap_data_tag: {
       ciSpeculativeTrapData data_dst(dp_dst);
       SpeculativeTrapData   data_src(dp_src);
-
-      { // During translation a safepoint can happen or VM lock can be taken (e.g., Compile_lock).
-        MutexUnlocker ml(mdo->extra_data_lock());
       data_dst.translate_from(&data_src);
-      }
       break;
     }
     case DataLayout::bit_data_tag:

src/hotspot/share/ci/ciMethodData.hpp

@@ -475,6 +475,7 @@ private:
     return (address) _data;
   }
 
+  void prepare_metadata();
   void load_extra_data();
   ciProfileData* bci_to_extra_data(int bci, ciMethod* m, bool& two_free_slots);
 

src/hotspot/share/ci/ciObjectFactory.cpp

@@ -265,6 +265,24 @@ int ciObjectFactory::metadata_compare(Metadata* const& key, ciMetadata* const& e
   else                  return 0;
 }
 
+// ------------------------------------------------------------------
+// ciObjectFactory::cached_metadata
+//
+// Get the ciMetadata corresponding to some Metadata. If the ciMetadata has
+// already been created, it is returned. Otherwise, null is returned.
+ciMetadata* ciObjectFactory::cached_metadata(Metadata* key) {
+  ASSERT_IN_VM;
+
+  bool found = false;
+  int index = _ci_metadata->find_sorted<Metadata*, ciObjectFactory::metadata_compare>(key, found);
+
+  if (!found) {
+    return NULL;
+  }
+  return _ci_metadata->at(index)->as_metadata();
+}
+
+
 // ------------------------------------------------------------------
 // ciObjectFactory::get_metadata
 //

src/hotspot/share/ci/ciObjectFactory.hpp

@@ -100,6 +100,7 @@ public:
   // Get the ciObject corresponding to some oop.
   ciObject* get(oop key);
   ciMetadata* get_metadata(Metadata* key);
+  ciMetadata* cached_metadata(Metadata* key);
   ciSymbol* get_symbol(Symbol* key);
 
   // Get the ciSymbol corresponding to one of the vmSymbols.

src/hotspot/share/oops/instanceKlass.cpp

@@ -2186,6 +2186,7 @@ void InstanceKlass::clean_method_data() {
   for (int m = 0; m < methods()->length(); m++) {
     MethodData* mdo = methods()->at(m)->method_data();
     if (mdo != NULL) {
+      MutexLockerEx ml(SafepointSynchronize::is_at_safepoint() ? NULL : mdo->extra_data_lock());
       mdo->clean_method_data(/*always_clean*/false);
     }
   }

src/hotspot/share/oops/methodData.cpp

@@ -1653,11 +1653,6 @@ void MethodData::clean_extra_data_helper(DataLayout* dp, int shift, bool reset)
   }
 }
 
-class CleanExtraDataClosure : public StackObj {
-public:
-  virtual bool is_live(Method* m) = 0;
-};
-
 // Check for entries that reference an unloaded method
 class CleanExtraDataKlassClosure : public CleanExtraDataClosure {
   bool _always_clean;

src/hotspot/share/oops/methodData.hpp

@@ -1943,7 +1943,11 @@ public:
 // adjusted in the event of a change in control flow.
 //
 
-class CleanExtraDataClosure;
+class CleanExtraDataClosure : public StackObj {
+public:
+  virtual bool is_live(Method* m) = 0;
+};
+
 
 class MethodData : public Metadata {
   friend class VMStructs;
@@ -2116,11 +2120,12 @@ private:
   static bool profile_parameters_jsr292_only();
   static bool profile_all_parameters();
 
-  void clean_extra_data(CleanExtraDataClosure* cl);
   void clean_extra_data_helper(DataLayout* dp, int shift, bool reset = false);
   void verify_extra_data_clean(CleanExtraDataClosure* cl);
 
 public:
+  void clean_extra_data(CleanExtraDataClosure* cl);
+
   static int header_size() {
     return sizeof(MethodData)/wordSize;
   }