8210856: Move InstanceKlass DependencyContext cleaning to SystemDictionary::do_unloading()

Already walk classes in ClassLoaderData::unload so generalize to also clean nmethod dependencies.

Reviewed-by: eosterlund, dlong, vlivanov

src/hotspot/share/classfile/classLoaderData.cpp

@@ -605,9 +605,10 @@ void ClassLoaderData::unload() {
   // 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);

src/hotspot/share/code/dependencyContext.cpp

@@ -218,18 +218,6 @@ 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;

src/hotspot/share/code/dependencyContext.hpp

@@ -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 @@ 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);

src/hotspot/share/oops/instanceKlass.cpp

@@ -2417,7 +2417,10 @@ static void clear_all_breakpoints(Method* m) {
 }
 #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 @@ void InstanceKlass::release_C_heap_structures() {
     FreeHeap(jmeths);
   }
 
-  // Release dependencies.
+  assert(_dep_context == DependencyContext::EMPTY,
-  // It is desirable to use DC::remove_all_dependents() here, but, unfortunately,
+         "dependencies should already be cleaned");
-  // 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();
 
 #if INCLUDE_JVMTI
   // Deallocate breakpoint records

src/hotspot/share/oops/instanceKlass.hpp

@@ -1180,7 +1180,7 @@ public:
   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

test/hotspot/gtest/code/test_dependencyContext.cpp

@@ -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 @@ class TestDependencyContext {
   }
 
   ~TestDependencyContext() {
-    dependencies().wipe();
+    wipe();
     CodeCache_lock->unlock();
   }
 
@@ -63,6 +63,18 @@ class TestDependencyContext {
     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) {