8156871: Possible concurrency issue with JVM_AddModuleExports
Summary: Need for single PackageEntry flag to determine a package's unqualifed export state.
Reviewed-by: acorn, ctornqvi, dholmes, jiangli
--- a/hotspot/src/share/vm/classfile/moduleEntry.cpp Wed Jun 15 09:48:24 2016 -0400
+++ b/hotspot/src/share/vm/classfile/moduleEntry.cpp Thu Jun 16 13:34:32 2016 -0400
@@ -201,7 +201,7 @@
}
void ModuleEntryTable::create_unnamed_module(ClassLoaderData* loader_data) {
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
// Each ModuleEntryTable has exactly one unnamed module
if (loader_data->is_the_null_class_loader_data()) {
@@ -227,7 +227,7 @@
ModuleEntry* ModuleEntryTable::new_entry(unsigned int hash, Handle module_handle, Symbol* name,
Symbol* version, Symbol* location,
ClassLoaderData* loader_data) {
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
ModuleEntry* entry = (ModuleEntry*) NEW_C_HEAP_ARRAY(char, entry_size(), mtModule);
// Initialize everything BasicHashtable would
@@ -258,7 +258,7 @@
}
void ModuleEntryTable::add_entry(int index, ModuleEntry* new_entry) {
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
Hashtable<Symbol*, mtModule>::add_entry(index, (HashtableEntry<Symbol*, mtModule>*)new_entry);
}
@@ -268,7 +268,7 @@
Symbol* module_location,
ClassLoaderData* loader_data) {
assert(module_name != NULL, "ModuleEntryTable locked_create_entry_or_null should never be called for unnamed module.");
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
// Check if module already exists.
if (lookup_only(module_name) != NULL) {
return NULL;
@@ -309,7 +309,7 @@
}
void ModuleEntryTable::finalize_javabase(Handle module_handle, Symbol* version, Symbol* location) {
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
ClassLoaderData* boot_loader_data = ClassLoaderData::the_null_class_loader_data();
ModuleEntryTable* module_table = boot_loader_data->modules();
--- a/hotspot/src/share/vm/classfile/modules.cpp Wed Jun 15 09:48:24 2016 -0400
+++ b/hotspot/src/share/vm/classfile/modules.cpp Thu Jun 16 13:34:32 2016 -0400
@@ -568,8 +568,8 @@
to_module_entry->is_named() ?
to_module_entry->name()->as_C_string() : UNNAMED_MODULE);
- // Do nothing if modules are the same or if package is already exported unqualifiedly.
- if (from_module_entry != to_module_entry && !package_entry->is_unqual_exported()) {
+ // Do nothing if modules are the same.
+ if (from_module_entry != to_module_entry) {
package_entry->set_exported(to_module_entry);
}
}
--- a/hotspot/src/share/vm/classfile/packageEntry.cpp Wed Jun 15 09:48:24 2016 -0400
+++ b/hotspot/src/share/vm/classfile/packageEntry.cpp Thu Jun 16 13:34:32 2016 -0400
@@ -49,7 +49,7 @@
// Add a module to the package's qualified export list.
void PackageEntry::add_qexport(ModuleEntry* m) {
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
if (!has_qual_exports_list()) {
// Lazily create a package's qualified exports list.
// Initial size is small, do not anticipate export lists to be large.
@@ -157,7 +157,7 @@
}
PackageEntry* PackageEntryTable::new_entry(unsigned int hash, Symbol* name, ModuleEntry* module) {
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
PackageEntry* entry = (PackageEntry*) NEW_C_HEAP_ARRAY(char, entry_size(), mtModule);
// Initialize everything BasicHashtable would
@@ -180,14 +180,14 @@
}
void PackageEntryTable::add_entry(int index, PackageEntry* new_entry) {
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
Hashtable<Symbol*, mtModule>::add_entry(index, (HashtableEntry<Symbol*, mtModule>*)new_entry);
}
// Create package in loader's package entry table and return the entry.
// If entry already exists, return null. Assume Module lock was taken by caller.
PackageEntry* PackageEntryTable::locked_create_entry_or_null(Symbol* name, ModuleEntry* module) {
- assert_locked_or_safepoint(Module_lock);
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
// Check if package already exists. Return NULL if it does.
if (lookup_only(name) != NULL) {
return NULL;
--- a/hotspot/src/share/vm/classfile/packageEntry.hpp Wed Jun 15 09:48:24 2016 -0400
+++ b/hotspot/src/share/vm/classfile/packageEntry.hpp Thu Jun 16 13:34:32 2016 -0400
@@ -40,11 +40,7 @@
// package is exported to.
//
// Packages can be exported in the following 3 ways:
-// - not exported: the package has not been explicitly qualified to a
-// particular module nor has it been specified to be
-// unqualifiedly exported to all modules. If all states
-// of exportedness are false, the package is considered
-// not exported.
+// - not exported: the package does not have qualified or unqualified exports.
// - qualified exports: the package has been explicitly qualified to at least
// one particular module or has been qualifiedly exported
// to all unnamed modules.
@@ -125,6 +121,7 @@
return _is_exported_unqualified;
}
void set_unqual_exported() {
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");
_is_exported_unqualified = true;
_is_exported_allUnnamed = false;
_qualified_exports = NULL;
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/modules/CompilerUtils.java Thu Jun 16 13:34:32 2016 -0400
@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import javax.tools.JavaCompiler;
+import javax.tools.StandardJavaFileManager;
+import javax.tools.StandardLocation;
+import javax.tools.ToolProvider;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class consists exclusively of static utility methods for invoking the
+ * java compiler.
+ *
+ * This class will eventually move to jdk.testlibrary.
+ */
+
+public final class CompilerUtils {
+ private CompilerUtils() { }
+
+ /**
+ * Compile all the java sources in {@code <source>/**} to
+ * {@code <destination>/**}. The destination directory will be created if
+ * it doesn't exist.
+ *
+ * All warnings/errors emitted by the compiler are output to System.out/err.
+ *
+ * @return true if the compilation is successful
+ *
+ * @throws IOException if there is an I/O error scanning the source tree or
+ * creating the destination directory
+ */
+ public static boolean compile(Path source, Path destination, String ... options)
+ throws IOException
+ {
+ JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+ StandardJavaFileManager jfm = compiler.getStandardFileManager(null, null, null);
+
+ List<Path> sources
+ = Files.find(source, Integer.MAX_VALUE,
+ (file, attrs) -> (file.toString().endsWith(".java")))
+ .collect(Collectors.toList());
+
+ Files.createDirectories(destination);
+ jfm.setLocationFromPaths(StandardLocation.CLASS_OUTPUT,
+ Arrays.asList(destination));
+
+ List<String> opts = Arrays.asList(options);
+ JavaCompiler.CompilationTask task
+ = compiler.getTask(null, jfm, null, opts, null,
+ jfm.getJavaFileObjectsFromPaths(sources));
+
+ return task.call();
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/modules/ModuleStress/ExportModuleStressTest.java Thu Jun 16 13:34:32 2016 -0400
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8156871
+ * @summary package in the boot layer is repeatedly exported to unique module created in layers on top of the boot layer
+ * @modules java.base/jdk.internal.misc
+ * @library /testlibrary /test/lib
+ * @compile ../CompilerUtils.java
+ * @build ExportModuleStressTest
+ * @run main/othervm ExportModuleStressTest
+ */
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import jdk.test.lib.*;
+
+public class ExportModuleStressTest {
+
+ private static final String TEST_SRC = System.getProperty("test.src");
+ private static final String TEST_CLASSES = System.getProperty("test.classes");
+
+ private static final Path SRC_DIR = Paths.get(TEST_SRC, "src");
+ private static final Path MODS_DIR = Paths.get(TEST_CLASSES, "mods");
+
+ /**
+ * Compiles all module definitions used by the test
+ */
+ public static void main(String[] args) throws Exception {
+
+ boolean compiled;
+ // Compile module jdk.test declaration
+ compiled = CompilerUtils.compile(
+ SRC_DIR.resolve("jdk.test"),
+ MODS_DIR.resolve("jdk.test"));
+ if (!compiled) {
+ throw new RuntimeException("Test failed to compile module jdk.test");
+ }
+
+ // Compile module jdk.translet declaration
+ compiled = CompilerUtils.compile(
+ SRC_DIR.resolve("jdk.translet"),
+ MODS_DIR.resolve("jdk.translet"),
+ "-XaddExports:jdk.test/test=jdk.translet",
+ "-mp", MODS_DIR.toString());
+ if (!compiled) {
+ throw new RuntimeException("Test failed to compile module jdk.translet");
+ }
+
+ // Sanity check that the test, jdk.test/test/Main.java
+ // runs without error.
+ ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
+ "-mp", MODS_DIR.toString(),
+ "-m", "jdk.test/test.Main");
+ OutputAnalyzer output = new OutputAnalyzer(pb.start());
+ output.shouldContain("failed: 0")
+ .shouldHaveExitValue(0);
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/modules/ModuleStress/src/jdk.test/module-info.java Thu Jun 16 13:34:32 2016 -0400
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+module jdk.test {
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/modules/ModuleStress/src/jdk.test/test/Main.java Thu Jun 16 13:34:32 2016 -0400
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package test;
+
+import java.lang.module.Configuration;
+import java.lang.module.ModuleFinder;
+import java.lang.reflect.Layer;
+import java.lang.reflect.Method;
+import java.lang.reflect.Module;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.*;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+
+public class Main {
+
+ private static final Path MODS_DIR = Paths.get(System.getProperty("jdk.module.path"));
+ static final String MODULE_NAME = "jdk.translet";
+
+ public static void main(String[] args) throws Exception {
+
+ ModuleFinder finder = ModuleFinder.of(MODS_DIR);
+ Layer layerBoot = Layer.boot();
+
+ Configuration cf = layerBoot
+ .configuration()
+ .resolveRequires(ModuleFinder.of(), finder, Set.of(MODULE_NAME));
+
+ Module testModule = Main.class.getModule();
+ ClassLoader scl = ClassLoader.getSystemClassLoader();
+
+ // Create an unique module/class loader in a layer above the boot layer.
+ // Export this module to the jdk.test/test package.
+ Callable<Void> task = new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ Layer layer = Layer.boot().defineModulesWithOneLoader(cf, scl);
+ Module transletModule = layer.findModule(MODULE_NAME).get();
+ testModule.addExports("test", transletModule);
+ Class<?> c = layer.findLoader(MODULE_NAME).loadClass("translet.Main");
+ Method method = c.getDeclaredMethod("go");
+ method.invoke(null);
+ return null;
+ }
+ };
+
+ List<Future<Void>> results = new ArrayList<>();
+
+ // Repeatedly create the layer above stressing the exportation of
+ // package jdk.test/test to several different modules.
+ ExecutorService pool = Executors.newFixedThreadPool(Math.min(100, Runtime.getRuntime().availableProcessors()*10));
+ try {
+ for (int i = 0; i < 10000; i++) {
+ results.add(pool.submit(task));
+ }
+ } finally {
+ pool.shutdown();
+ }
+
+ int passed = 0;
+ int failed = 0;
+
+ // The failed state should be 0, the created modules in layers above the
+ // boot layer should be allowed access to the contents of the jdk.test/test
+ // package since that package was exported to the transletModule above.
+ for (Future<Void> result : results) {
+ try {
+ result.get();
+ passed++;
+ } catch (Throwable x) {
+ x.printStackTrace();
+ failed++;
+ }
+ }
+
+ System.out.println("passed: " + passed);
+ System.out.println("failed: " + failed);
+ }
+
+ public static void callback() { }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/modules/ModuleStress/src/jdk.translet/module-info.java Thu Jun 16 13:34:32 2016 -0400
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+module jdk.translet {
+ requires jdk.test;
+ exports translet;
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/modules/ModuleStress/src/jdk.translet/translet/Main.java Thu Jun 16 13:34:32 2016 -0400
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package translet;
+
+public class Main {
+ public static void go() {
+ test.Main.callback();
+ }
+}