8172288: Fix Jigsaw related module/package error messages and throw correct exceptions
Summary: Reword error messages and throw IllegalStateExceptions where appropriate
Reviewed-by: alanb, acorn, lfoltan, gtriantafill
--- a/hotspot/src/share/vm/classfile/modules.cpp Thu Jan 12 19:34:29 2017 +0000
+++ b/hotspot/src/share/vm/classfile/modules.cpp Fri Jan 13 07:19:03 2017 -0500
@@ -1,5 +1,5 @@
/*
-* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2017, 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
@@ -239,7 +239,7 @@
}
}
if (duplicate_javabase) {
- THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
+ THROW_MSG(vmSymbols::java_lang_InternalError(),
"Module " JAVA_BASE_NAME " is already defined");
}
@@ -262,6 +262,20 @@
}
}
+// Caller needs ResourceMark.
+void throw_dup_pkg_exception(const char* module_name, PackageEntry* package, TRAPS) {
+ const char* package_name = package->name()->as_C_string();
+ if (package->module()->is_named()) {
+ THROW_MSG(vmSymbols::java_lang_IllegalStateException(),
+ err_msg("Package %s for module %s is already in another module, %s, defined to the class loader",
+ package_name, module_name, package->module()->name()->as_C_string()));
+ } else {
+ THROW_MSG(vmSymbols::java_lang_IllegalStateException(),
+ err_msg("Package %s for module %s is already in the unnamed module defined to the class loader",
+ package_name, module_name));
+ }
+}
+
void Modules::define_module(jobject module, jstring version,
jstring location, jobjectArray packages, TRAPS) {
ResourceMark rm(THREAD);
@@ -347,7 +361,6 @@
// Create symbol* entry for module name.
TempNewSymbol module_symbol = SymbolTable::new_symbol(module_name, CHECK);
- int dupl_pkg_index = -1;
bool dupl_modules = false;
// Create symbol* entry for module version.
@@ -373,6 +386,7 @@
assert(loader_data != NULL, "class loader data shouldn't be null");
PackageEntryTable* package_table = NULL;
+ PackageEntry* existing_pkg = NULL;
{
MutexLocker ml(Module_lock, THREAD);
@@ -382,13 +396,12 @@
// Check that none of the packages exist in the class loader's package table.
for (int x = 0; x < pkg_list->length(); x++) {
- if (package_table->lookup_only(pkg_list->at(x)) != NULL) {
+ existing_pkg = package_table->lookup_only(pkg_list->at(x));
+ if (existing_pkg != NULL) {
// This could be because the module was already defined. If so,
// report that error instead of the package error.
if (module_table->lookup_only(module_symbol) != NULL) {
dupl_modules = true;
- } else {
- dupl_pkg_index = x;
}
break;
}
@@ -396,9 +409,8 @@
} // if (num_packages > 0)...
// Add the module and its packages.
- if (!dupl_modules && dupl_pkg_index == -1) {
+ if (!dupl_modules && existing_pkg == NULL) {
// Create the entry for this module in the class loader's module entry table.
-
ModuleEntry* module_entry = module_table->locked_create_entry_or_null(module_handle, module_symbol,
version_symbol, location_symbol, loader_data);
@@ -426,13 +438,10 @@
// any errors ?
if (dupl_modules) {
- THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
+ THROW_MSG(vmSymbols::java_lang_IllegalStateException(),
err_msg("Module %s is already defined", module_name));
- }
- if (dupl_pkg_index != -1) {
- THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
- err_msg("Package %s for module %s already exists for class loader",
- pkg_list->at(dupl_pkg_index)->as_C_string(), module_name));
+ } else if (existing_pkg != NULL) {
+ throw_dup_pkg_exception(module_name, existing_pkg, CHECK);
}
if (log_is_enabled(Debug, modules)) {
@@ -776,21 +785,19 @@
PackageEntryTable* package_table = loader_data->packages();
assert(package_table != NULL, "Missing package_table");
- bool pkg_exists = false;
+ PackageEntry* existing_pkg = NULL;
{
MutexLocker ml(Module_lock, THREAD);
// Check that the package does not exist in the class loader's package table.
- if (!package_table->lookup_only(pkg_symbol)) {
+ existing_pkg = package_table->lookup_only(pkg_symbol);
+ if (existing_pkg == NULL) {
PackageEntry* pkg = package_table->locked_create_entry_or_null(pkg_symbol, module_entry);
assert(pkg != NULL, "Unable to create a module's package entry");
- } else {
- pkg_exists = true;
}
}
- if (pkg_exists) {
- THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
- err_msg("Package %s already exists for class loader", package_name));
+ if (existing_pkg != NULL) {
+ throw_dup_pkg_exception(module_entry->name()->as_C_string(), existing_pkg, CHECK);
}
}
--- a/hotspot/test/runtime/modules/JVMAddModulePackage.java Thu Jan 12 19:34:29 2017 +0000
+++ b/hotspot/test/runtime/modules/JVMAddModulePackage.java Fri Jan 13 07:19:03 2017 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 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
@@ -84,11 +84,11 @@
// Expected
}
- // Existing package, expect an IAE
+ // Existing package, expect an ISE
try {
ModuleHelper.AddModulePackage(module1, "yourpackage");
- throw new RuntimeException("Failed to get the expected IAE");
- } catch(IllegalArgumentException e) {
+ throw new RuntimeException("Failed to get the expected ISE");
+ } catch(IllegalStateException e) {
// Expected
}
--- a/hotspot/test/runtime/modules/JVMDefineModule.java Thu Jan 12 19:34:29 2017 +0000
+++ b/hotspot/test/runtime/modules/JVMDefineModule.java Fri Jan 13 07:19:03 2017 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 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
@@ -127,27 +127,27 @@
}
}
- // Duplicate module name, expect an IAE
- m = ModuleHelper.ModuleObject("module.name", cl, new String[] { "mypackage6" });
+ // Duplicate module name, expect an ISE
+ m = ModuleHelper.ModuleObject("Module_A", cl, new String[] { "mypackage6" });
assertNotNull(m, "Module should not be null");
ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "mypackage6" });
try {
ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "mypackage6a" });
- throw new RuntimeException("Failed to get IAE for duplicate module");
- } catch(IllegalArgumentException e) {
- if (!e.getMessage().contains("Module module.name is already defined")) {
- throw new RuntimeException("Failed to get expected IAE message for duplicate module: " + e.getMessage());
+ throw new RuntimeException("Failed to get ISE for duplicate module");
+ } catch(IllegalStateException e) {
+ if (!e.getMessage().contains("Module Module_A is already defined")) {
+ throw new RuntimeException("Failed to get expected ISE message for duplicate module: " + e.getMessage());
}
}
- // Package is already defined for class loader, expect an IAE
+ // Package is already defined for class loader, expect an ISE
m = ModuleHelper.ModuleObject("dupl.pkg.module", cl, new String[] { "mypackage6b" });
try {
ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "mypackage6" });
- throw new RuntimeException("Failed to get IAE for existing package");
- } catch(IllegalArgumentException e) {
- if (!e.getMessage().contains("Package mypackage6 for module dupl.pkg.module already exists for class loader")) {
- throw new RuntimeException("Failed to get expected IAE message for duplicate package: " + e.getMessage());
+ throw new RuntimeException("Failed to get ISE for existing package");
+ } catch(IllegalStateException e) {
+ if (!e.getMessage().contains("Package mypackage6 for module dupl.pkg.module is already in another module, Module_A, defined to the class loader")) {
+ throw new RuntimeException("Failed to get expected ISE message for duplicate package: " + e.getMessage());
}
}