8007725: NPG: Klass::restore_unshareable_info() triggers assert(k->java_mirror() == NULL)
Summary: Check for exception during SystemDictionary::resolve_instance_class_or_null() and clean up.
Reviewed-by: coleenp, acorn, hseigel, minqi
Contributed-by: ioi.lam@oracle.com
--- a/hotspot/src/share/vm/classfile/classLoaderData.cpp Thu Mar 14 00:33:08 2013 -0700
+++ b/hotspot/src/share/vm/classfile/classLoaderData.cpp Fri Mar 15 17:24:40 2013 -0400
@@ -105,6 +105,7 @@
void ClassLoaderData::classes_do(KlassClosure* klass_closure) {
for (Klass* k = _klasses; k != NULL; k = k->next_link()) {
klass_closure->do_klass(k);
+ assert(k != k->next_link(), "no loops!");
}
}
@@ -113,6 +114,7 @@
if (k->oop_is_instance()) {
f(InstanceKlass::cast(k));
}
+ assert(k != k->next_link(), "no loops!");
}
}
@@ -258,6 +260,7 @@
return;
}
prev = k;
+ assert(k != k->next_link(), "no loops!");
}
ShouldNotReachHere(); // should have found this class!!
}
@@ -439,6 +442,7 @@
while (k != NULL) {
out->print_cr("klass "PTR_FORMAT", %s, CT: %d, MUT: %d", k, k->name()->as_C_string(),
k->has_modified_oops(), k->has_accumulated_modified_oops());
+ assert(k != k->next_link(), "no loops!");
k = k->next_link();
}
}
@@ -465,6 +469,7 @@
for (Klass* k = _klasses; k != NULL; k = k->next_link()) {
guarantee(k->class_loader_data() == this, "Must be the same");
k->verify();
+ assert(k != k->next_link(), "no loops!");
}
}
--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp Thu Mar 14 00:33:08 2013 -0700
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp Fri Mar 15 17:24:40 2013 -0400
@@ -804,6 +804,32 @@
}
} // load_instance_class loop
+ if (HAS_PENDING_EXCEPTION) {
+ // An exception, such as OOM could have happened at various places inside
+ // load_instance_class. We might have partially initialized a shared class
+ // and need to clean it up.
+ if (class_loader.is_null()) {
+ // In some cases k may be null. Let's find the shared class again.
+ instanceKlassHandle ik(THREAD, find_shared_class(name));
+ if (ik.not_null()) {
+ if (ik->class_loader_data() == NULL) {
+ // We didn't go as far as Klass::restore_unshareable_info(),
+ // so nothing to clean up.
+ } else {
+ MutexLocker mu(SystemDictionary_lock, THREAD);
+ Klass* kk = find_class(name, ik->class_loader_data());
+ if (kk != NULL) {
+ // No clean up is needed if the shared class has been entered
+ // into system dictionary, as load_shared_class() won't be called
+ // again.
+ } else {
+ clean_up_shared_class(ik, class_loader, THREAD);
+ }
+ }
+ }
+ }
+ }
+
if (load_instance_added == true) {
// clean up placeholder entries for LOAD_INSTANCE success or error
// This brackets the SystemDictionary updates for both defining
@@ -1140,11 +1166,6 @@
return load_shared_class(ik, class_loader, THREAD);
}
-// Note well! Changes to this method may affect oop access order
-// in the shared archive. Please take care to not make changes that
-// adversely affect cold start time by changing the oop access order
-// that is specified in dump.cpp MarkAndMoveOrderedReadOnly and
-// MarkAndMoveOrderedReadWrite closures.
instanceKlassHandle SystemDictionary::load_shared_class(
instanceKlassHandle ik, Handle class_loader, TRAPS) {
assert(class_loader.is_null(), "non-null classloader for shared class?");
@@ -1205,6 +1226,19 @@
return ik;
}
+void SystemDictionary::clean_up_shared_class(instanceKlassHandle ik, Handle class_loader, TRAPS) {
+ // Updating methods must be done under a lock so multiple
+ // threads don't update these in parallel
+ // Shared classes are all currently loaded by the bootstrap
+ // classloader, so this will never cause a deadlock on
+ // a custom class loader lock.
+ {
+ Handle lockObject = compute_loader_lock_object(class_loader, THREAD);
+ check_loader_lock_contention(lockObject, THREAD);
+ ObjectLocker ol(lockObject, THREAD, true);
+ ik->remove_unshareable_info();
+ }
+}
instanceKlassHandle SystemDictionary::load_instance_class(Symbol* class_name, Handle class_loader, TRAPS) {
instanceKlassHandle nh = instanceKlassHandle(); // null Handle
--- a/hotspot/src/share/vm/classfile/systemDictionary.hpp Thu Mar 14 00:33:08 2013 -0700
+++ b/hotspot/src/share/vm/classfile/systemDictionary.hpp Fri Mar 15 17:24:40 2013 -0400
@@ -621,6 +621,7 @@
Handle class_loader, TRAPS);
static instanceKlassHandle load_shared_class(instanceKlassHandle ik,
Handle class_loader, TRAPS);
+ static void clean_up_shared_class(instanceKlassHandle ik, Handle class_loader, TRAPS);
static instanceKlassHandle load_instance_class(Symbol* class_name, Handle class_loader, TRAPS);
static Handle compute_loader_lock_object(Handle class_loader, TRAPS);
static void check_loader_lock_contention(Handle loader_lock, TRAPS);
--- a/hotspot/src/share/vm/oops/klass.cpp Thu Mar 14 00:33:08 2013 -0700
+++ b/hotspot/src/share/vm/oops/klass.cpp Fri Mar 15 17:24:40 2013 -0400
@@ -486,6 +486,12 @@
}
void Klass::remove_unshareable_info() {
+ if (!DumpSharedSpaces) {
+ // Clean up after OOM during class loading
+ if (class_loader_data() != NULL) {
+ class_loader_data()->remove_class(this);
+ }
+ }
set_subklass(NULL);
set_next_sibling(NULL);
// Clear the java mirror
--- a/hotspot/src/share/vm/oops/method.cpp Thu Mar 14 00:33:08 2013 -0700
+++ b/hotspot/src/share/vm/oops/method.cpp Fri Mar 15 17:24:40 2013 -0400
@@ -798,7 +798,15 @@
backedge_counter()->reset();
_adapter = NULL;
_from_compiled_entry = NULL;
- assert(_method_data == NULL, "unexpected method data?");
+
+ // In case of DumpSharedSpaces, _method_data should always be NULL.
+ //
+ // During runtime (!DumpSharedSpaces), when we are cleaning a
+ // shared class that failed to load, this->link_method() may
+ // have already been called (before an exception happened), so
+ // this->_method_data may not be NULL.
+ assert(!DumpSharedSpaces || _method_data == NULL, "unexpected method data?");
+
set_method_data(NULL);
set_interpreter_throwout_count(0);
set_interpreter_invocation_count(0);