8166364: fatal error: acquiring lock DirtyCardQ_CBL_mon/16 out of order with lock Module_lock/6 -- possible deadlock
Summary: Set the mirror's module field outside of the module lock.
Reviewed-by: dsamersoff, dholmes, rehn
--- a/hotspot/src/share/vm/classfile/javaClasses.cpp Fri Oct 07 15:40:34 2016 -0700
+++ b/hotspot/src/share/vm/classfile/javaClasses.cpp Mon Oct 10 08:34:32 2016 -0400
@@ -780,19 +780,26 @@
// Put the class on the fixup_module_list to patch later when the java.lang.reflect.Module
// for java.base is known.
assert(!Universe::is_module_initialized(), "Incorrect java.lang.reflect.Module pre module system initialization");
- MutexLocker m1(Module_lock, THREAD);
- // Keep list of classes needing java.base module fixup
- if (!ModuleEntryTable::javabase_defined()) {
- if (fixup_module_field_list() == NULL) {
- GrowableArray<Klass*>* list =
- new (ResourceObj::C_HEAP, mtModule) GrowableArray<Klass*>(500, true);
- set_fixup_module_field_list(list);
+
+ bool javabase_was_defined = false;
+ {
+ MutexLocker m1(Module_lock, THREAD);
+ // Keep list of classes needing java.base module fixup
+ if (!ModuleEntryTable::javabase_defined()) {
+ if (fixup_module_field_list() == NULL) {
+ GrowableArray<Klass*>* list =
+ new (ResourceObj::C_HEAP, mtModule) GrowableArray<Klass*>(500, true);
+ set_fixup_module_field_list(list);
+ }
+ k->class_loader_data()->inc_keep_alive();
+ fixup_module_field_list()->push(k());
+ } else {
+ javabase_was_defined = true;
}
- k->class_loader_data()->inc_keep_alive();
- fixup_module_field_list()->push(k());
- } else {
- // java.base was defined at some point between calling create_mirror()
- // and obtaining the Module_lock, patch this particular class with java.base.
+ }
+
+ // If java.base was already defined then patch this particular class with java.base.
+ if (javabase_was_defined) {
ModuleEntry *javabase_entry = ModuleEntryTable::javabase_moduleEntry();
assert(javabase_entry != NULL && javabase_entry->module() != NULL,
"Setting class module field, java.base should be defined");
--- a/hotspot/src/share/vm/classfile/moduleEntry.cpp Fri Oct 07 15:40:34 2016 -0700
+++ b/hotspot/src/share/vm/classfile/moduleEntry.cpp Mon Oct 10 08:34:32 2016 -0400
@@ -368,9 +368,6 @@
// Store pointer to the ModuleEntry for java.base in the java.lang.reflect.Module object.
java_lang_reflect_Module::set_module_entry(module_handle(), jb_module);
-
- // Patch any previously loaded classes' module field with java.base's java.lang.reflect.Module.
- patch_javabase_entries(module_handle);
}
// Within java.lang.Class instances there is a java.lang.reflect.Module field
@@ -378,7 +375,6 @@
// definition, classes needing their module field set are added to the fixup_module_list.
// Their module field is set once java.base's java.lang.reflect.Module is known to the VM.
void ModuleEntryTable::patch_javabase_entries(Handle module_handle) {
- assert(Module_lock->owned_by_self(), "should have the Module_lock");
if (module_handle.is_null()) {
fatal("Unable to patch the module field of classes loaded prior to java.base's definition, invalid java.lang.reflect.Module");
}
--- a/hotspot/src/share/vm/classfile/modules.cpp Fri Oct 07 15:40:34 2016 -0700
+++ b/hotspot/src/share/vm/classfile/modules.cpp Mon Oct 10 08:34:32 2016 -0400
@@ -244,6 +244,12 @@
"Module java.base is already defined");
}
+ // Only the thread that actually defined the base module will get here,
+ // so no locking is needed.
+
+ // Patch any previously loaded class's module field with java.base's java.lang.reflect.Module.
+ ModuleEntryTable::patch_javabase_entries(module_handle);
+
log_debug(modules)("define_javabase_module(): Definition of module: java.base,"
" version: %s, location: %s, package #: %d",
module_version != NULL ? module_version : "NULL",