8215889: assert(!_unloading) failed: This oop is not available to unloading class loader data with ZGC
Reviewed-by: coleenp, neliasso
--- a/src/hotspot/share/ci/ciEnv.hpp Wed Jan 09 15:53:56 2019 +0100
+++ b/src/hotspot/share/ci/ciEnv.hpp Thu Jan 10 18:10:15 2019 +0100
@@ -46,6 +46,7 @@
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 @@
}
}
+ 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();
--- a/src/hotspot/share/ci/ciMethodData.cpp Wed Jan 09 15:53:56 2019 +0100
+++ b/src/hotspot/share/ci/ciMethodData.cpp Thu Jan 10 18:10:15 2019 +0100
@@ -78,10 +78,81 @@
_parameters = NULL;
}
+// 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 @@
// 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 @@
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);
- }
+ data_dst.translate_from(&data_src);
break;
}
case DataLayout::bit_data_tag:
--- a/src/hotspot/share/ci/ciMethodData.hpp Wed Jan 09 15:53:56 2019 +0100
+++ b/src/hotspot/share/ci/ciMethodData.hpp Thu Jan 10 18:10:15 2019 +0100
@@ -475,6 +475,7 @@
return (address) _data;
}
+ void prepare_metadata();
void load_extra_data();
ciProfileData* bci_to_extra_data(int bci, ciMethod* m, bool& two_free_slots);
--- a/src/hotspot/share/ci/ciObjectFactory.cpp Wed Jan 09 15:53:56 2019 +0100
+++ b/src/hotspot/share/ci/ciObjectFactory.cpp Thu Jan 10 18:10:15 2019 +0100
@@ -266,6 +266,24 @@
}
// ------------------------------------------------------------------
+// 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
//
// Get the ciMetadata corresponding to some Metadata. If the ciMetadata has
--- a/src/hotspot/share/ci/ciObjectFactory.hpp Wed Jan 09 15:53:56 2019 +0100
+++ b/src/hotspot/share/ci/ciObjectFactory.hpp Thu Jan 10 18:10:15 2019 +0100
@@ -100,6 +100,7 @@
// 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.
--- a/src/hotspot/share/oops/instanceKlass.cpp Wed Jan 09 15:53:56 2019 +0100
+++ b/src/hotspot/share/oops/instanceKlass.cpp Thu Jan 10 18:10:15 2019 +0100
@@ -2186,6 +2186,7 @@
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);
}
}
--- a/src/hotspot/share/oops/methodData.cpp Wed Jan 09 15:53:56 2019 +0100
+++ b/src/hotspot/share/oops/methodData.cpp Thu Jan 10 18:10:15 2019 +0100
@@ -1653,11 +1653,6 @@
}
}
-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;
--- a/src/hotspot/share/oops/methodData.hpp Wed Jan 09 15:53:56 2019 +0100
+++ b/src/hotspot/share/oops/methodData.hpp Thu Jan 10 18:10:15 2019 +0100
@@ -1943,7 +1943,11 @@
// 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 @@
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;
}