8163406: The fixup_module_list must be protected by Module_lock when inserting new entries

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
This commit is contained in:
Lois Foltan 2016-09-19 12:04:28 -04:00
parent eb52950704
commit 368585d0db
10 changed files with 75 additions and 45 deletions

View File

@ -1358,7 +1358,7 @@ ClassFileStream* ClassLoader::search_module_entries(const GrowableArray<ModuleCl
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 @@ void ClassLoader::create_javabase() {
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);
}
}

View File

@ -773,6 +773,41 @@ void java_lang_Class::initialize_mirror_fields(KlassHandle k,
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 @@ void java_lang_Class::create_mirror(KlassHandle k, Handle class_loader,
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 =

View File

@ -219,6 +219,7 @@ class java_lang_Class : AllStatic {
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();

View File

@ -92,7 +92,7 @@ bool ModuleEntry::can_read(ModuleEntry* m) const {
// 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 @@ void ModuleEntryTable::finalize_javabase(Handle module_handle, Symbol* version,
}
// 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 @@ void ModuleEntryTable::patch_javabase_entries(Handle module_handle) {
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();
}

View File

@ -78,11 +78,11 @@ public:
_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 @@ public:
// 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);

View File

@ -206,7 +206,7 @@ static void define_javabase_module(jobject module, jstring version,
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 @@ static void define_javabase_module(jobject module, jstring version,
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 @@ static void define_javabase_module(jobject module, jstring version,
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,

View File

@ -2247,8 +2247,8 @@ void InstanceKlass::set_package(ClassLoaderData* loader_data, TRAPS) {
// 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,

View File

@ -530,7 +530,7 @@ void Klass::restore_unshareable_info(ClassLoaderData* loader_data, Handle protec
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));

View File

@ -72,7 +72,7 @@ TypeArrayKlass* TypeArrayKlass::create_klass(BasicType type,
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 @@ const char* TypeArrayKlass::internal_name() const {
// 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 {

View File

@ -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 BasicHashtableEntry<F>* BasicHashtable<F>::bucket(i
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 void HashtableBucket<F>::set_entry(BasicHashtableEn
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);