Skip to content

Commit 4b39a26

Browse files
author
coleenp
committed
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
1 parent 4c4003e commit 4b39a26

File tree

2 files changed

+30
-4
lines changed

2 files changed

+30
-4
lines changed

src/hotspot/share/classfile/classLoaderData.cpp

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -574,9 +574,9 @@ void ClassLoaderData::unload() {
574574
ls.cr();
575575
}
576576

577-
// In some rare cases items added to this list will not be freed elsewhere.
578-
// To keep it simple, just free everything in it here.
579-
free_deallocate_list();
577+
// Some items on the _deallocate_list need to free their C heap structures
578+
// if they are not already on the _klasses list.
579+
unload_deallocate_list();
580580

581581
// Clean up global class iterator for compiler
582582
static_klass_iterator.adjust_saved_class(this);
@@ -755,6 +755,7 @@ OopHandle ClassLoaderData::add_handle(Handle h) {
755755
}
756756

757757
void ClassLoaderData::remove_handle(OopHandle h) {
758+
assert(!is_unloading(), "Do not remove a handle for a CLD that is unloading");
758759
oop* ptr = h.ptr_raw();
759760
if (ptr != NULL) {
760761
assert(_handles.contains(ptr), "Got unexpected handle " PTR_FORMAT, p2i(ptr));
@@ -799,6 +800,7 @@ void ClassLoaderData::add_to_deallocate_list(Metadata* m) {
799800
void ClassLoaderData::free_deallocate_list() {
800801
// Don't need lock, at safepoint
801802
assert(SafepointSynchronize::is_at_safepoint(), "only called at safepoint");
803+
assert(!is_unloading(), "only called for ClassLoaderData that are not unloading");
802804
if (_deallocate_list == NULL) {
803805
return;
804806
}
@@ -828,6 +830,29 @@ void ClassLoaderData::free_deallocate_list() {
828830
}
829831
}
830832

833+
// This is distinct from free_deallocate_list. For class loader data that are
834+
// unloading, this frees the C heap memory for constant pools on the list. If there
835+
// were C heap memory allocated for methods, it would free that too. The C heap memory
836+
// for InstanceKlasses on this list is freed in the ClassLoaderData destructor.
837+
void ClassLoaderData::unload_deallocate_list() {
838+
// Don't need lock, at safepoint
839+
assert(SafepointSynchronize::is_at_safepoint(), "only called at safepoint");
840+
assert(is_unloading(), "only called for ClassLoaderData that are unloading");
841+
if (_deallocate_list == NULL) {
842+
return;
843+
}
844+
// Go backwards because this removes entries that are freed.
845+
for (int i = _deallocate_list->length() - 1; i >= 0; i--) {
846+
Metadata* m = _deallocate_list->at(i);
847+
assert (!m->on_stack(), "wouldn't be unloading if this were so");
848+
_deallocate_list->remove_at(i);
849+
// Only constant pool entries have C heap memory to free.
850+
if (m->is_constantPool()) {
851+
((ConstantPool*)m)->release_C_heap_structures();
852+
}
853+
}
854+
}
855+
831856
// These anonymous class loaders are to contain classes used for JSR292
832857
ClassLoaderData* ClassLoaderData::anonymous_class_loader_data(oop loader, TRAPS) {
833858
// Add a new class loader data to the graph.

src/hotspot/share/classfile/classLoaderData.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,8 @@ class ClassLoaderData : public CHeapObj<mtClass> {
307307
void packages_do(void f(PackageEntry*));
308308

309309
// Deallocate free list during class unloading.
310-
void free_deallocate_list();
310+
void free_deallocate_list(); // for the classes that are not unloaded
311+
void unload_deallocate_list(); // for the classes that are unloaded
311312

312313
// Allocate out of this class loader data
313314
MetaWord* allocate(size_t size);

0 commit comments

Comments
 (0)