changeset 12175:b1c62e595c4a

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
author hseigel
date Mon, 10 Oct 2016 08:34:32 -0400
parents bd5e566edbc4
children d3f508318705
files src/share/vm/classfile/javaClasses.cpp src/share/vm/classfile/moduleEntry.cpp src/share/vm/classfile/modules.cpp
diffstat 3 files changed, 25 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/classfile/javaClasses.cpp	Fri Oct 07 15:40:34 2016 -0700
+++ b/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/src/share/vm/classfile/moduleEntry.cpp	Fri Oct 07 15:40:34 2016 -0700
+++ b/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/src/share/vm/classfile/modules.cpp	Fri Oct 07 15:40:34 2016 -0700
+++ b/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",