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
--- 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);