8210856: Move InstanceKlass DependencyContext cleaning to SystemDictionary::do_unloading()
Summary: Already walk classes in ClassLoaderData::unload so generalize to also clean nmethod dependencies.
Reviewed-by: eosterlund, dlong, vlivanov
--- a/src/hotspot/share/classfile/classLoaderData.cpp Tue Sep 25 21:33:51 2018 -0700
+++ b/src/hotspot/share/classfile/classLoaderData.cpp Wed Sep 26 14:01:48 2018 -0400
@@ -605,9 +605,10 @@
// if they are not already on the _klasses list.
free_deallocate_list_C_heap_structures();
- // Tell serviceability tools these classes are unloading
+ // Clean up class dependencies and tell serviceability tools
+ // these classes are unloading. Must be called
// after erroneous classes are released.
- classes_do(InstanceKlass::notify_unload_class);
+ classes_do(InstanceKlass::unload_class);
// Clean up global class iterator for compiler
static_klass_iterator.adjust_saved_class(this);
--- a/src/hotspot/share/code/dependencyContext.cpp Tue Sep 25 21:33:51 2018 -0700
+++ b/src/hotspot/share/code/dependencyContext.cpp Wed Sep 26 14:01:48 2018 -0400
@@ -218,18 +218,6 @@
return marked;
}
-void DependencyContext::wipe() {
- assert_locked_or_safepoint(CodeCache_lock);
- nmethodBucket* b = dependencies();
- set_dependencies(NULL);
- set_has_stale_entries(false);
- while (b != NULL) {
- nmethodBucket* next = b->next();
- delete b;
- b = next;
- }
-}
-
#ifndef PRODUCT
void DependencyContext::print_dependent_nmethods(bool verbose) {
int idx = 0;
--- a/src/hotspot/share/code/dependencyContext.hpp Tue Sep 25 21:33:51 2018 -0700
+++ b/src/hotspot/share/code/dependencyContext.hpp Wed Sep 26 14:01:48 2018 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -141,10 +141,6 @@
void expunge_stale_entries();
- // Unsafe deallocation of nmethodBuckets. Used in IK::release_C_heap_structures
- // to clean up the context possibly containing live entries pointing to unloaded nmethods.
- void wipe();
-
#ifndef PRODUCT
void print_dependent_nmethods(bool verbose);
bool is_dependent_nmethod(nmethod* nm);
--- a/src/hotspot/share/oops/instanceKlass.cpp Tue Sep 25 21:33:51 2018 -0700
+++ b/src/hotspot/share/oops/instanceKlass.cpp Wed Sep 26 14:01:48 2018 -0400
@@ -2417,7 +2417,10 @@
}
#endif
-void InstanceKlass::notify_unload_class(InstanceKlass* ik) {
+void InstanceKlass::unload_class(InstanceKlass* ik) {
+ // Release dependencies.
+ ik->dependencies().remove_all_dependents();
+
// notify the debugger
if (JvmtiExport::should_post_class_unload()) {
JvmtiExport::post_class_unload(ik);
@@ -2462,16 +2465,8 @@
FreeHeap(jmeths);
}
- // Release dependencies.
- // It is desirable to use DC::remove_all_dependents() here, but, unfortunately,
- // it is not safe (see JDK-8143408). The problem is that the klass dependency
- // context can contain live dependencies, since there's a race between nmethod &
- // klass unloading. If the klass is dead when nmethod unloading happens, relevant
- // dependencies aren't removed from the context associated with the class (see
- // nmethod::flush_dependencies). It ends up during klass unloading as seemingly
- // live dependencies pointing to unloaded nmethods and causes a crash in
- // DC::remove_all_dependents() when it touches unloaded nmethod.
- dependencies().wipe();
+ assert(_dep_context == DependencyContext::EMPTY,
+ "dependencies should already be cleaned");
#if INCLUDE_JVMTI
// Deallocate breakpoint records
--- a/src/hotspot/share/oops/instanceKlass.hpp Tue Sep 25 21:33:51 2018 -0700
+++ b/src/hotspot/share/oops/instanceKlass.hpp Wed Sep 26 14:01:48 2018 -0400
@@ -1180,7 +1180,7 @@
bool on_stack() const { return _constants->on_stack(); }
// callbacks for actions during class unloading
- static void notify_unload_class(InstanceKlass* ik);
+ static void unload_class(InstanceKlass* ik);
static void release_C_heap_structures(InstanceKlass* ik);
// Naming
--- a/test/hotspot/gtest/code/test_dependencyContext.cpp Tue Sep 25 21:33:51 2018 -0700
+++ b/test/hotspot/gtest/code/test_dependencyContext.cpp Wed Sep 26 14:01:48 2018 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -50,7 +50,7 @@
}
~TestDependencyContext() {
- dependencies().wipe();
+ wipe();
CodeCache_lock->unlock();
}
@@ -63,6 +63,18 @@
return ctx.find_stale_entries();
}
#endif
+
+ void wipe() {
+ DependencyContext ctx(&_dependency_context);
+ nmethodBucket* b = ctx.dependencies();
+ ctx.set_dependencies(NULL);
+ ctx.set_has_stale_entries(false);
+ while (b != NULL) {
+ nmethodBucket* next = b->next();
+ delete b;
+ b = next;
+ }
+ }
};
static void test_remove_dependent_nmethod(int id, bool delete_immediately) {