Date: Sat, 30 Mar 2024 07:13:14 +0000
Subject: 8143408: Crash during InstanceKlass unloading when
 clearing dependency context

---
 .../src/share/vm/code/dependencyContext.cpp   | 40 ++++++++++---------
 .../src/share/vm/code/dependencyContext.hpp   |  4 ++
 hotspot/src/share/vm/oops/instanceKlass.cpp   | 16 +++++---
 3 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/hotspot/src/share/vm/code/dependencyContext.cpp b/hotspot/src/share/vm/code/dependencyContext.cpp
index 5c0af1e3..6cb0c330 100644
--- a/hotspot/src/share/vm/code/dependencyContext.cpp
+++ b/hotspot/src/share/vm/code/dependencyContext.cpp
@@ -218,6 +218,18 @@ int DependencyContext::remove_all_dependents() {
   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;
@@ -271,28 +283,31 @@ class TestDependencyContext {
 
   intptr_t _dependency_context;
 
+  DependencyContext dependencies() {
+    DependencyContext depContext(&_dependency_context);
+    return depContext;
+  }
+
   TestDependencyContext() : _dependency_context(DependencyContext::EMPTY) {
     CodeCache_lock->lock_without_safepoint_check();
 
-    DependencyContext depContext(&_dependency_context);
-
     _nmethods[0] = reinterpret_cast<nmethod*>(0x8 * 0);
     _nmethods[1] = reinterpret_cast<nmethod*>(0x8 * 1);
     _nmethods[2] = reinterpret_cast<nmethod*>(0x8 * 2);
 
-    depContext.add_dependent_nmethod(_nmethods[2]);
-    depContext.add_dependent_nmethod(_nmethods[1]);
-    depContext.add_dependent_nmethod(_nmethods[0]);
+    dependencies().add_dependent_nmethod(_nmethods[2]);
+    dependencies().add_dependent_nmethod(_nmethods[1]);
+    dependencies().add_dependent_nmethod(_nmethods[0]);
   }
 
   ~TestDependencyContext() {
-    wipe();
+    dependencies().wipe();
     CodeCache_lock->unlock();
   }
 
   static void testRemoveDependentNmethod(int id, bool delete_immediately) {
     TestDependencyContext c;
-    DependencyContext depContext(&c._dependency_context);
+    DependencyContext depContext = c.dependencies();
     assert(!has_stale_entries(depContext), "check");
 
     nmethod* nm = c._nmethods[id];
@@ -327,17 +342,6 @@ class TestDependencyContext {
     return ctx.has_stale_entries();
   }
 
-  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;
-    }
-  }
 };
 
 void TestDependencyContext_test() {
diff --git a/hotspot/src/share/vm/code/dependencyContext.hpp b/hotspot/src/share/vm/code/dependencyContext.hpp
index 533112b8..414ce0c0 100644
--- a/hotspot/src/share/vm/code/dependencyContext.hpp
+++ b/hotspot/src/share/vm/code/dependencyContext.hpp
@@ -143,6 +143,10 @@ class DependencyContext : public StackObj {
 
   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);
diff --git a/hotspot/src/share/vm/oops/instanceKlass.cpp b/hotspot/src/share/vm/oops/instanceKlass.cpp
index 1bff1309..df44e531 100644
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp
@@ -2628,12 +2628,16 @@ void InstanceKlass::release_C_heap_structures() {
     }
   }
 
-  // release dependencies
-  {
-    DependencyContext ctx(&_dep_context);
-    int marked = ctx.remove_all_dependents();
-    assert(marked == 0, "all dependencies should be already invalidated");
-  }
+   // 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();
 
   // Deallocate breakpoint records
   if (breakpoints() != 0x0) {
-- 
2.17.1