8163406: The fixup_module_list must be protected by Module_lock when inserting new entries
authorlfoltan
Mon, 19 Sep 2016 12:04:28 -0400
changeset 41183 207b92e69457
parent 41182 dbd59c1da636
child 41185 9549e775d661
8163406: The fixup_module_list must be protected by Module_lock when inserting new entries Summary: In java_lang_Class::create_mirror, restructure the check for adding a class to the fixup_module_list, guarded by Module_lock. Reviewed-by: acorn, coleenp, dholmes, zgu
hotspot/src/share/vm/classfile/classLoader.cpp
hotspot/src/share/vm/classfile/javaClasses.cpp
hotspot/src/share/vm/classfile/javaClasses.hpp
hotspot/src/share/vm/classfile/moduleEntry.cpp
hotspot/src/share/vm/classfile/moduleEntry.hpp
hotspot/src/share/vm/classfile/modules.cpp
hotspot/src/share/vm/oops/instanceKlass.cpp
hotspot/src/share/vm/oops/klass.cpp
hotspot/src/share/vm/oops/typeArrayKlass.cpp
hotspot/src/share/vm/utilities/hashtable.inline.hpp
--- a/hotspot/src/share/vm/classfile/classLoader.cpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classLoader.cpp	Mon Sep 19 12:04:28 2016 -0400
@@ -1358,7 +1358,7 @@
   if (!Universe::is_module_initialized() &&
       !ModuleEntryTable::javabase_defined() &&
       mod_entry == NULL) {
-    mod_entry = ModuleEntryTable::javabase_module();
+    mod_entry = ModuleEntryTable::javabase_moduleEntry();
   }
 
   // The module must be a named module
@@ -1708,7 +1708,7 @@
     if (jb_module == NULL) {
       vm_exit_during_initialization("Unable to create ModuleEntry for java.base");
     }
-    ModuleEntryTable::set_javabase_module(jb_module);
+    ModuleEntryTable::set_javabase_moduleEntry(jb_module);
   }
 }
 
--- a/hotspot/src/share/vm/classfile/javaClasses.cpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/classfile/javaClasses.cpp	Mon Sep 19 12:04:28 2016 -0400
@@ -773,6 +773,41 @@
   InstanceKlass::cast(k())->do_local_static_fields(&initialize_static_field, mirror, CHECK);
 }
 
+// Set the java.lang.reflect.Module module field in the java_lang_Class mirror
+void java_lang_Class::set_mirror_module_field(KlassHandle k, Handle mirror, Handle module, TRAPS) {
+  if (module.is_null()) {
+    // During startup, the module may be NULL only if java.base has not been defined yet.
+    // 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);
+      }
+      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.
+      ModuleEntry *javabase_entry = ModuleEntryTable::javabase_moduleEntry();
+      assert(javabase_entry != NULL && javabase_entry->module() != NULL,
+             "Setting class module field, java.base should be defined");
+      Handle javabase_handle(THREAD, JNIHandles::resolve(javabase_entry->module()));
+      set_module(mirror(), javabase_handle());
+    }
+  } else {
+    assert(Universe::is_module_initialized() ||
+           (ModuleEntryTable::javabase_defined() &&
+            (module() == JNIHandles::resolve(ModuleEntryTable::javabase_moduleEntry()->module()))),
+           "Incorrect java.lang.reflect.Module specification while creating mirror");
+    set_module(mirror(), module());
+  }
+}
+
 void java_lang_Class::create_mirror(KlassHandle k, Handle class_loader,
                                     Handle module, Handle protection_domain, TRAPS) {
   assert(k->java_mirror() == NULL, "should only assign mirror once");
@@ -835,25 +870,13 @@
     set_class_loader(mirror(), class_loader());
 
     // set the module field in the java_lang_Class instance
-    // This may be null during bootstrap but will get fixed up later on.
-    set_module(mirror(), module());
+    set_mirror_module_field(k, mirror, module, THREAD);
 
     // Setup indirection from klass->mirror last
     // after any exceptions can happen during allocations.
     if (!k.is_null()) {
       k->set_java_mirror(mirror());
     }
-
-    // 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 {
     if (fixup_mirror_list() == NULL) {
       GrowableArray<Klass*>* list =
--- a/hotspot/src/share/vm/classfile/javaClasses.hpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/classfile/javaClasses.hpp	Mon Sep 19 12:04:28 2016 -0400
@@ -219,6 +219,7 @@
   static void set_class_loader(oop java_class, oop class_loader);
   static void set_component_mirror(oop java_class, oop comp_mirror);
   static void initialize_mirror_fields(KlassHandle k, Handle mirror, Handle protection_domain, TRAPS);
+  static void set_mirror_module_field(KlassHandle K, Handle mirror, Handle module, TRAPS);
  public:
   static void compute_offsets();
 
--- a/hotspot/src/share/vm/classfile/moduleEntry.cpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/classfile/moduleEntry.cpp	Mon Sep 19 12:04:28 2016 -0400
@@ -92,7 +92,7 @@
   // read java.base.  If either of these conditions
   // hold, readability has been established.
   if (!this->is_named() ||
-      (m == ModuleEntryTable::javabase_module())) {
+      (m == ModuleEntryTable::javabase_moduleEntry())) {
     return true;
   }
 
@@ -358,16 +358,27 @@
   }
 
   // Set java.lang.reflect.Module, version and location for java.base
-  ModuleEntry* jb_module = javabase_module();
+  ModuleEntry* jb_module = javabase_moduleEntry();
   assert(jb_module != NULL, "java.base ModuleEntry not defined");
-  jb_module->set_module(boot_loader_data->add_handle(module_handle));
   jb_module->set_version(version);
   jb_module->set_location(location);
+  // Once java.base's ModuleEntry _module field is set with the known
+  // java.lang.reflect.Module, java.base is considered "defined" to the VM.
+  jb_module->set_module(boot_loader_data->add_handle(module_handle));
+
   // 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
+// that must be set with the defining module.  During startup, prior to java.base's
+// 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");
   }
@@ -389,9 +400,7 @@
   for (int i = 0; i < list_length; i++) {
     Klass* k = list->at(i);
     assert(k->is_klass(), "List should only hold classes");
-    Thread* THREAD = Thread::current();
-    KlassHandle kh(THREAD, k);
-    java_lang_Class::fixup_module_field(kh, module_handle);
+    java_lang_Class::fixup_module_field(KlassHandle(k), module_handle);
     k->class_loader_data()->dec_keep_alive();
   }
 
--- a/hotspot/src/share/vm/classfile/moduleEntry.hpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/classfile/moduleEntry.hpp	Mon Sep 19 12:04:28 2016 -0400
@@ -78,11 +78,11 @@
     _must_walk_reads = false;
   }
 
-  Symbol*          name() const          { return literal(); }
-  void             set_name(Symbol* n)   { set_literal(n); }
+  Symbol*          name() const                        { return literal(); }
+  void             set_name(Symbol* n)                 { set_literal(n); }
 
-  jobject          module() const        { return _module; }
-  void             set_module(jobject j) { _module = j; }
+  jobject          module() const                      { return _module; }
+  void             set_module(jobject j)               { _module = j; }
 
   // The shared ProtectionDomain reference is set once the VM loads a shared class
   // originated from the current Module. The referenced ProtectionDomain object is
@@ -217,13 +217,13 @@
 
   // Special handling for unnamed module, one per class loader's ModuleEntryTable
   void create_unnamed_module(ClassLoaderData* loader_data);
-  ModuleEntry* unnamed_module()                           { return _unnamed_module; }
+  ModuleEntry* unnamed_module()                                { return _unnamed_module; }
 
   // Special handling for java.base
-  static ModuleEntry* javabase_module()                   { return _javabase_module; }
-  static void set_javabase_module(ModuleEntry* java_base) { _javabase_module = java_base; }
-  static bool javabase_defined()                          { return ((_javabase_module != NULL) &&
-                                                                    (_javabase_module->module() != NULL)); }
+  static ModuleEntry* javabase_moduleEntry()                   { return _javabase_module; }
+  static void set_javabase_moduleEntry(ModuleEntry* java_base) { _javabase_module = java_base; }
+  static bool javabase_defined()                               { return ((_javabase_module != NULL) &&
+                                                                         (_javabase_module->module() != NULL)); }
   static void finalize_javabase(Handle module_handle, Symbol* version, Symbol* location);
   static void patch_javabase_entries(Handle module_handle);
 
--- a/hotspot/src/share/vm/classfile/modules.cpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/classfile/modules.cpp	Mon Sep 19 12:04:28 2016 -0400
@@ -206,7 +206,7 @@
   assert(pkg_list->length() == 0 || package_table != NULL, "Bad package_table");
 
   // Ensure java.base's ModuleEntry has been created
-  assert(ModuleEntryTable::javabase_module() != NULL, "No ModuleEntry for java.base");
+  assert(ModuleEntryTable::javabase_moduleEntry() != NULL, "No ModuleEntry for java.base");
 
   bool duplicate_javabase = false;
   {
@@ -226,7 +226,7 @@
       for (int x = 0; x < pkg_list->length(); x++) {
         // Some of java.base's packages were added early in bootstrapping, ignore duplicates.
         if (package_table->lookup_only(pkg_list->at(x)) == NULL) {
-          pkg = package_table->locked_create_entry_or_null(pkg_list->at(x), ModuleEntryTable::javabase_module());
+          pkg = package_table->locked_create_entry_or_null(pkg_list->at(x), ModuleEntryTable::javabase_moduleEntry());
           assert(pkg != NULL, "Unable to create a java.base package entry");
         }
         // Unable to have a GrowableArray of TempNewSymbol.  Must decrement the refcount of
@@ -255,9 +255,6 @@
     log_trace(modules)("define_javabase_module(): creation of package %s for module java.base",
                        (pkg_list->at(x))->as_C_string());
   }
-
-  // Patch any previously loaded classes' module field with java.base's jlr.Module.
-  ModuleEntryTable::patch_javabase_entries(module_handle);
 }
 
 void Modules::define_module(jobject module, jstring version,
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp	Mon Sep 19 12:04:28 2016 -0400
@@ -2247,8 +2247,8 @@
         // the java.base module.  If a non-java.base package is erroneously placed
         // in the java.base module it will be caught later when java.base
         // is defined by ModuleEntryTable::verify_javabase_packages check.
-        assert(ModuleEntryTable::javabase_module() != NULL, "java.base module is NULL");
-        _package_entry = loader_data->packages()->lookup(pkg_name, ModuleEntryTable::javabase_module());
+        assert(ModuleEntryTable::javabase_moduleEntry() != NULL, "java.base module is NULL");
+        _package_entry = loader_data->packages()->lookup(pkg_name, ModuleEntryTable::javabase_moduleEntry());
       } else {
         assert(loader_data->modules()->unnamed_module() != NULL, "unnamed module is NULL");
         _package_entry = loader_data->packages()->lookup(pkg_name,
--- a/hotspot/src/share/vm/oops/klass.cpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/oops/klass.cpp	Mon Sep 19 12:04:28 2016 -0400
@@ -530,7 +530,7 @@
       InstanceKlass* ik = (InstanceKlass*) k;
       module_entry = ik->module();
     } else {
-      module_entry = ModuleEntryTable::javabase_module();
+      module_entry = ModuleEntryTable::javabase_moduleEntry();
     }
     // Obtain java.lang.reflect.Module, if available
     Handle module_handle(THREAD, ((module_entry != NULL) ? JNIHandles::resolve(module_entry->module()) : (oop)NULL));
--- a/hotspot/src/share/vm/oops/typeArrayKlass.cpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/oops/typeArrayKlass.cpp	Mon Sep 19 12:04:28 2016 -0400
@@ -72,7 +72,7 @@
   null_loader_data->add_class(ak);
 
   // Call complete_create_array_klass after all instance variables have been initialized.
-  complete_create_array_klass(ak, ak->super(), ModuleEntryTable::javabase_module(), CHECK_NULL);
+  complete_create_array_klass(ak, ak->super(), ModuleEntryTable::javabase_moduleEntry(), CHECK_NULL);
 
   return ak;
 }
@@ -347,7 +347,7 @@
 
 // A TypeArrayKlass is an array of a primitive type, its defining module is java.base
 ModuleEntry* TypeArrayKlass::module() const {
-  return ModuleEntryTable::javabase_module();
+  return ModuleEntryTable::javabase_moduleEntry();
 }
 
 PackageEntry* TypeArrayKlass::package() const {
--- a/hotspot/src/share/vm/utilities/hashtable.inline.hpp	Sun Sep 18 21:10:48 2016 -0400
+++ b/hotspot/src/share/vm/utilities/hashtable.inline.hpp	Mon Sep 19 12:04:28 2016 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -79,8 +79,8 @@
 
 
 template <MEMFLAGS F> inline void HashtableBucket<F>::set_entry(BasicHashtableEntry<F>* l) {
-  // Warning: Preserve store ordering.  The SystemDictionary is read
-  //          without locks.  The new SystemDictionaryEntry must be
+  // Warning: Preserve store ordering.  The PackageEntryTable, ModuleEntryTable and
+  //          SystemDictionary are read without locks.  The new entry must be
   //          complete before other threads can be allowed to see it
   //          via a store to _buckets[index].
   OrderAccess::release_store_ptr(&_entry, l);
@@ -88,8 +88,8 @@
 
 
 template <MEMFLAGS F> inline BasicHashtableEntry<F>* HashtableBucket<F>::get_entry() const {
-  // Warning: Preserve load ordering.  The SystemDictionary is read
-  //          without locks.  The new SystemDictionaryEntry must be
+  // Warning: Preserve load ordering.  The PackageEntryTable, ModuleEntryTable and
+  //          SystemDictionary are read without locks.  The new entry must be
   //          complete before other threads can be allowed to see it
   //          via a store to _buckets[index].
   return (BasicHashtableEntry<F>*) OrderAccess::load_ptr_acquire(&_entry);