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