# HG changeset patch
# User coleenp
# Date 1513167258 18000
# Node ID 919780ab7accad1ca347cac69a025155d8519dae
# Parent  39a84de6afd693ff9b8c60bbdae0abb332b4938a
8193053: jvm crash by G1CMBitMapClosure::do_addr
Summary: We were adding an unloaded mirror to the SATB collection set in remove_handle.
Reviewed-by: hseigel, kbarrett

diff -r 39a84de6afd6 -r 919780ab7acc src/hotspot/share/classfile/classLoaderData.cpp
--- a/src/hotspot/share/classfile/classLoaderData.cpp	Wed Dec 13 10:21:21 2017 +0100
+++ b/src/hotspot/share/classfile/classLoaderData.cpp	Wed Dec 13 07:14:18 2017 -0500
@@ -574,9 +574,9 @@
     ls.cr();
   }
 
-  // In some rare cases items added to this list will not be freed elsewhere.
-  // To keep it simple, just free everything in it here.
-  free_deallocate_list();
+  // Some items on the _deallocate_list need to free their C heap structures
+  // if they are not already on the _klasses list.
+  unload_deallocate_list();
 
   // Clean up global class iterator for compiler
   static_klass_iterator.adjust_saved_class(this);
@@ -755,6 +755,7 @@
 }
 
 void ClassLoaderData::remove_handle(OopHandle h) {
+  assert(!is_unloading(), "Do not remove a handle for a CLD that is unloading");
   oop* ptr = h.ptr_raw();
   if (ptr != NULL) {
     assert(_handles.contains(ptr), "Got unexpected handle " PTR_FORMAT, p2i(ptr));
@@ -799,6 +800,7 @@
 void ClassLoaderData::free_deallocate_list() {
   // Don't need lock, at safepoint
   assert(SafepointSynchronize::is_at_safepoint(), "only called at safepoint");
+  assert(!is_unloading(), "only called for ClassLoaderData that are not unloading");
   if (_deallocate_list == NULL) {
     return;
   }
@@ -828,6 +830,29 @@
   }
 }
 
+// This is distinct from free_deallocate_list.  For class loader data that are
+// unloading, this frees the C heap memory for constant pools on the list.  If there
+// were C heap memory allocated for methods, it would free that too.  The C heap memory
+// for InstanceKlasses on this list is freed in the ClassLoaderData destructor.
+void ClassLoaderData::unload_deallocate_list() {
+  // Don't need lock, at safepoint
+  assert(SafepointSynchronize::is_at_safepoint(), "only called at safepoint");
+  assert(is_unloading(), "only called for ClassLoaderData that are unloading");
+  if (_deallocate_list == NULL) {
+    return;
+  }
+  // Go backwards because this removes entries that are freed.
+  for (int i = _deallocate_list->length() - 1; i >= 0; i--) {
+    Metadata* m = _deallocate_list->at(i);
+    assert (!m->on_stack(), "wouldn't be unloading if this were so");
+    _deallocate_list->remove_at(i);
+    // Only constant pool entries have C heap memory to free.
+    if (m->is_constantPool()) {
+      ((ConstantPool*)m)->release_C_heap_structures();
+    }
+  }
+}
+
 // These anonymous class loaders are to contain classes used for JSR292
 ClassLoaderData* ClassLoaderData::anonymous_class_loader_data(oop loader, TRAPS) {
   // Add a new class loader data to the graph.
diff -r 39a84de6afd6 -r 919780ab7acc src/hotspot/share/classfile/classLoaderData.hpp
--- a/src/hotspot/share/classfile/classLoaderData.hpp	Wed Dec 13 10:21:21 2017 +0100
+++ b/src/hotspot/share/classfile/classLoaderData.hpp	Wed Dec 13 07:14:18 2017 -0500
@@ -307,7 +307,8 @@
   void packages_do(void f(PackageEntry*));
 
   // Deallocate free list during class unloading.
-  void free_deallocate_list();
+  void free_deallocate_list();      // for the classes that are not unloaded
+  void unload_deallocate_list();    // for the classes that are unloaded
 
   // Allocate out of this class loader data
   MetaWord* allocate(size_t size);