stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8241427: Coarsen locking in Modules::add_module_exports

Browse Source 
Reviewed-by: lfoltan, hseigel
This commit is contained in:
Claes Redestad 2020-03-26 00:02:08 +01:00
parent 60fae77974
commit 0d22ad6400
6 changed files with 57 additions and 84 deletions

109
src/hotspot/share/classfile/modules.cpp

@ -108,13 +108,16 @@ static ModuleEntry* get_module_entry(jobject module, TRAPS) {
return java_lang_Module::module_entry(m);
}
static PackageEntry* get_package_entry(ModuleEntry* module_entry, const char* package_name, TRAPS) {
ResourceMark rm(THREAD);
if (package_name == NULL) return NULL;
static PackageEntry* get_locked_package_entry(ModuleEntry* module_entry, const char* package_name, TRAPS) {
assert(Module_lock->owned_by_self(), "should have the Module_lock");
assert(package_name != NULL, "Precondition");
TempNewSymbol pkg_symbol = SymbolTable::new_symbol(package_name);
PackageEntryTable* package_entry_table = module_entry->loader_data()->packages();
assert(package_entry_table != NULL, "Unexpected null package entry table");
return package_entry_table->lookup_only(pkg_symbol);
PackageEntry* package_entry = package_entry_table->locked_lookup_only(pkg_symbol);
assert(package_entry == NULL || package_entry->module() == module_entry, "Unexpectedly found a package linked to another module");
return package_entry;
}
static PackageEntry* get_package_entry_by_name(Symbol* package,
@ -525,32 +528,35 @@ void Modules::add_module_exports(jobject from_module, const char* package_name,
}
}
PackageEntry *package_entry = get_package_entry(from_module_entry, package_name, CHECK);
ResourceMark rm(THREAD);
PackageEntry* package_entry = NULL;
{
MutexLocker ml(THREAD, Module_lock);
package_entry = get_locked_package_entry(from_module_entry, package_name, CHECK);
// Do nothing if modules are the same
// If the package is not found we'll throw an exception later
if (from_module_entry != to_module_entry &&
package_entry != NULL) {
package_entry->set_exported(to_module_entry);
}
}
// Handle errors and logging outside locked section
if (package_entry == NULL) {
ResourceMark rm(THREAD);
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Package %s not found in from_module %s",
package_name != NULL ? package_name : "",
from_module_entry->name()->as_C_string()));
}
if (package_entry->module() != from_module_entry) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Package: %s found in module %s, not in from_module: %s",
if (log_is_enabled(Debug, module)) {
ResourceMark rm(THREAD);
log_debug(module)("add_module_exports(): package %s in module %s is exported to module %s",
package_entry->name()->as_C_string(),
package_entry->module()->name()->as_C_string(),
from_module_entry->name()->as_C_string()));
}
log_debug(module)("add_module_exports(): package %s in module %s is exported to module %s",
package_entry->name()->as_C_string(),
from_module_entry->name()->as_C_string(),
to_module_entry == NULL ? "NULL" :
from_module_entry->name()->as_C_string(),
to_module_entry == NULL ? "NULL" :
to_module_entry->is_named() ?
to_module_entry->name()->as_C_string() : UNNAMED_MODULE);
// Do nothing if modules are the same.
if (from_module_entry != to_module_entry) {
package_entry->set_exported(to_module_entry);
to_module_entry->name()->as_C_string() : UNNAMED_MODULE);
}
}
@ -668,21 +674,6 @@ jobject Modules::get_named_module(Handle h_loader, const char* package_name, TRA
return NULL;
}
// This method is called by JFR and by the above method.
jobject Modules::get_module(Symbol* package_name, Handle h_loader, TRAPS) {
const PackageEntry* const pkg_entry =
get_package_entry_by_name(package_name, h_loader, THREAD);
const ModuleEntry* const module_entry = (pkg_entry != NULL ? pkg_entry->module() : NULL);
if (module_entry != NULL &&
module_entry->module() != NULL) {
return JNIHandles::make_local(THREAD, module_entry->module());
}
return NULL;
}
// Export package in module to all unnamed modules.
void Modules::add_module_exports_to_all_unnamed(jobject module, const char* package_name, TRAPS) {
if (module == NULL) {
@ -699,29 +690,35 @@ void Modules::add_module_exports_to_all_unnamed(jobject module, const char* pack
"module is invalid");
}
if (module_entry->is_named()) { // No-op for unnamed module.
PackageEntry *package_entry = get_package_entry(module_entry, package_name, CHECK);
ResourceMark rm(THREAD);
if (package_entry == NULL) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Package %s not found in module %s",
package_name != NULL ? package_name : "",
module_entry->name()->as_C_string()));
}
if (package_entry->module() != module_entry) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Package: %s found in module %s, not in module: %s",
package_entry->name()->as_C_string(),
package_entry->module()->name()->as_C_string(),
module_entry->name()->as_C_string()));
}
// No-op for unnamed module and open modules
if (!module_entry->is_named() || module_entry->is_open())
return;
PackageEntry* package_entry = NULL;
{
MutexLocker m1(THREAD, Module_lock);
package_entry = get_locked_package_entry(module_entry, package_name, CHECK);
// Mark package as exported to all unnamed modules.
if (package_entry != NULL) {
package_entry->set_is_exported_allUnnamed();
}
}
// Handle errors and logging outside locked section
if (package_entry == NULL) {
ResourceMark rm(THREAD);
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Package %s not found in module %s",
package_name != NULL ? package_name : "",
module_entry->name()->as_C_string()));
}
if (log_is_enabled(Debug, module)) {
ResourceMark rm(THREAD);
log_debug(module)("add_module_exports_to_all_unnamed(): package %s in module"
" %s is exported to all unnamed modules",
package_entry->name()->as_C_string(),
module_entry->name()->as_C_string());
// Mark package as exported to all unnamed modules.
package_entry->set_is_exported_allUnnamed();
}
}

5
src/hotspot/share/classfile/modules.hpp

@ -107,11 +107,6 @@ public:
// java/lang/ClassLoader.
static jobject get_named_module(Handle h_loader, const char* package, TRAPS);
// If package is defined by loader, return the
// java.lang.Module object for the module in which the package is defined.
// Returns NULL if package is invalid or not defined by loader.
static jobject get_module(Symbol* package_name, Handle h_loader, TRAPS);
// Marks the specified package as exported to all unnamed modules.
// If either module or package is null then NullPointerException is thrown.
// If module or package is bad, or module is unnamed, or package is not in

11
src/hotspot/share/classfile/packageEntry.cpp

@ -89,7 +89,7 @@ void PackageEntry::set_export_walk_required(ClassLoaderData* m_loader_data) {
// Set the package's exported states based on the value of the ModuleEntry.
void PackageEntry::set_exported(ModuleEntry* m) {
MutexLocker m1(Module_lock);
assert(Module_lock->owned_by_self(), "should have the Module_lock");
if (is_unqual_exported()) {
// An exception could be thrown, but choose to simply ignore.
// Illegal to convert an unqualified exported package to be qualifiedly exported
@ -109,12 +109,8 @@ void PackageEntry::set_exported(ModuleEntry* m) {
// Set the package as exported to all unnamed modules unless the package is
// already unqualifiedly exported.
void PackageEntry::set_is_exported_allUnnamed() {
if (module()->is_open()) {
// No-op for open modules since all packages are unqualifiedly exported
return;
}
MutexLocker m1(Module_lock);
assert(!module()->is_open(), "should have been checked already");
assert(Module_lock->owned_by_self(), "should have the Module_lock");
if (!is_unqual_exported()) {
_export_flags = PKG_EXP_ALLUNNAMED;
}
@ -129,7 +125,6 @@ void PackageEntry::purge_qualified_exports() {
if (_must_walk_exports &&
_qualified_exports != NULL &&
!_qualified_exports->is_empty()) {
ModuleEntry* pkg_module = module();
// This package's _must_walk_exports flag will be reset based
// on the remaining live modules on the exports list.

2
src/hotspot/share/classfile/systemDictionary.cpp

@ -143,7 +143,7 @@ void SystemDictionary::compute_java_loaders(TRAPS) {
}
ClassLoaderData* SystemDictionary::register_loader(Handle class_loader) {
if (class_loader() == NULL) return ClassLoaderData::the_null_class_loader_data();
if (class_loader.is_null()) return ClassLoaderData::the_null_class_loader_data();
return ClassLoaderDataGraph::find_or_create(class_loader);
}

7
test/hotspot/jtreg/runtime/modules/AccessCheckAllUnnamed.java

@ -86,13 +86,6 @@ public class AccessCheckAllUnnamed {
// Expected
}
try {
ModuleHelper.AddModuleExportsToAllUnnamed(m2x, "p3");
throw new RuntimeException("Failed to get the expected IAE for package in other module");
} catch(IllegalArgumentException e) {
// Expected
}
try {
ModuleHelper.AddModuleExportsToAllUnnamed(m2x, "p4");
throw new RuntimeException("Failed to get the expected IAE for package not in module");

7
test/hotspot/jtreg/runtime/modules/JVMAddModuleExportsToAll.java

@ -86,13 +86,6 @@ public class JVMAddModuleExportsToAll {
// Expected
}
try {
ModuleHelper.AddModuleExportsToAll(m2x, "p3");
throw new RuntimeException("Failed to get the expected IAE for package that is in another module");
} catch(IllegalArgumentException e) {
// Expected
}
try {
ModuleHelper.AddModuleExportsToAll(m2x, "p4");
throw new RuntimeException("Failed to get the expected IAE for package not in any module");