8150668: Layer.defineModulesXXX with a Configuration containing java.base throws undocumented exception
authoralanb
Wed, 25 May 2016 19:58:03 +0100
changeset 38564 8c49d605b024
parent 38561 bf65323537b2
child 38565 e97761823da2
8150668: Layer.defineModulesXXX with a Configuration containing java.base throws undocumented exception Reviewed-by: chegar, mchung
jdk/src/java.base/share/classes/java/lang/reflect/Layer.java
jdk/src/java.base/share/classes/java/lang/reflect/Module.java
jdk/test/java/lang/reflect/Layer/BasicLayerTest.java
jdk/test/java/lang/reflect/Layer/LayerAndLoadersTest.java
--- a/jdk/src/java.base/share/classes/java/lang/reflect/Layer.java	Wed May 25 17:07:13 2016 +0300
+++ b/jdk/src/java.base/share/classes/java/lang/reflect/Layer.java	Wed May 25 19:58:03 2016 +0100
@@ -75,11 +75,12 @@
  *
  * <p> A Java virtual machine has at least one non-empty layer, the {@link
  * #boot() boot} layer, that is created when the Java virtual machine is
- * started. The <em>system modules</em>, including {@code java.base}, are in
- * the boot layer. The modules in the boot layer are mapped to the bootstrap
- * class loader and other class loaders that are built-in into the Java virtual
- * machine. The boot layer will often be the {@link #parent() parent} when
- * creating additional layers. </p>
+ * started. The boot layer contains module {@code java.base} and is the only
+ * layer in the Java virtual machine with a module named "{@code java.base}".
+ * The modules in the boot layer are mapped to the bootstrap class loader and
+ * other class loaders that are <a href="../ClassLoader.html#builtinLoaders">
+ * built-in</a> into the Java virtual machine. The boot layer will often be
+ * the {@link #parent() parent} when creating additional layers. </p>
  *
  * <p> As when creating a {@code Configuration},
  * {@link ModuleDescriptor#isAutomatic() automatic} modules receive
@@ -204,7 +205,8 @@
      *         for this layer
      * @throws LayerInstantiationException
      *         If all modules cannot be defined to the same class loader for any
-     *         of the reasons listed above
+     *         of the reasons listed above or the layer cannot be created because
+     *         the configuration contains a module named "{@code java.base}"
      * @throws SecurityException
      *         If {@code RuntimePermission("createClassLoader")} or
      *         {@code RuntimePermission("getClassLoader")} is denied by
@@ -219,14 +221,13 @@
         checkCreateClassLoaderPermission();
         checkGetClassLoaderPermission();
 
-        Loader loader;
         try {
-            loader = new Loader(cf.modules(), parentLoader);
+            Loader loader = new Loader(cf.modules(), parentLoader);
             loader.initRemotePackageMap(cf, this);
+            return new Layer(cf, this, mn -> loader);
         } catch (IllegalArgumentException e) {
             throw new LayerInstantiationException(e.getMessage());
         }
-        return new Layer(cf, this, mn -> loader);
     }
 
 
@@ -266,6 +267,9 @@
      * @throws IllegalArgumentException
      *         If the parent of the given configuration is not the configuration
      *         for this layer
+     * @throws LayerInstantiationException
+     *         If the layer cannot be created because the configuration contains
+     *         a module named "{@code java.base}"
      * @throws SecurityException
      *         If {@code RuntimePermission("createClassLoader")} or
      *         {@code RuntimePermission("getClassLoader")} is denied by
@@ -281,7 +285,11 @@
         checkGetClassLoaderPermission();
 
         LoaderPool pool = new LoaderPool(cf, this, parentLoader);
-        return new Layer(cf, this, pool::loaderFor);
+        try {
+            return new Layer(cf, this, pool::loaderFor);
+        } catch (IllegalArgumentException e) {
+            throw new LayerInstantiationException(e.getMessage());
+        }
     }
 
 
@@ -330,7 +338,8 @@
      *         for this layer
      * @throws LayerInstantiationException
      *         If creating the {@code Layer} fails for any of the reasons
-     *         listed above
+     *         listed above or the layer cannot be created because the
+     *         configuration contains a module named "{@code java.base}"
      * @throws SecurityException
      *         If {@code RuntimePermission("getClassLoader")} is denied by
      *         the security manager
--- a/jdk/src/java.base/share/classes/java/lang/reflect/Module.java	Wed May 25 17:07:13 2016 +0300
+++ b/jdk/src/java.base/share/classes/java/lang/reflect/Module.java	Wed May 25 19:58:03 2016 +0100
@@ -513,7 +513,7 @@
      * package {@code pn} to the given module.
      *
      * <p> This method has no effect if the package is already exported to the
-     * given module. If also has no effect if invoked on an unnamed module (as
+     * given module. It also has no effect if invoked on an unnamed module (as
      * unnamed modules export all packages). </p>
      *
      * @param  pn
@@ -866,7 +866,7 @@
             URI uri = mref.location().orElse(null);
 
             Module m;
-            if (loader == null && name.equals("java.base")) {
+            if (loader == null && name.equals("java.base") && Layer.boot() == null) {
                 m = Object.class.getModule();
             } else {
                 m = new Module(layer, loader, descriptor, uri);
--- a/jdk/test/java/lang/reflect/Layer/BasicLayerTest.java	Wed May 25 17:07:13 2016 +0300
+++ b/jdk/test/java/lang/reflect/Layer/BasicLayerTest.java	Wed May 25 19:58:03 2016 +0100
@@ -103,7 +103,7 @@
 
 
     /**
-     * Exercise Layer.create, created on an empty layer
+     * Exercise Layer defineModules, created with empty layer as parent
      */
     public void testLayerOnEmpty() {
         ModuleDescriptor descriptor1
@@ -184,7 +184,7 @@
 
 
     /**
-     * Exercise Layer.create, created over the boot layer
+     * Exercise Layer defineModules, created with boot layer as parent
      */
     public void testLayerOnBoot() {
         ModuleDescriptor descriptor1
@@ -247,8 +247,8 @@
 
 
     /**
-     * Layer.create with a configuration of two modules that have the same
-     * module-private package.
+     * Exercise Layer defineModules with a configuration of two modules that
+     * have the same module-private package.
      */
     public void testSameConcealedPackage() {
         ModuleDescriptor descriptor1
@@ -281,8 +281,8 @@
 
 
     /**
-     * Layer.create with a configuration with a partitioned graph. The same
-     * package is exported in both partitions.
+     * Exercise Layer defineModules with a configuration that is a partitioned
+     * graph. The same package is exported in both partitions.
      */
     public void testSameExportInPartitionedGraph() {
 
@@ -338,9 +338,9 @@
 
 
     /**
-     * Layer.create with a configuration that contains a module that has a
-     * concealed package that is the same name as a non-exported package
-     * in a parent layer.
+     * Exercise Layer defineModules with a configuration that contains a module
+     * that has a concealed package that is the same name as a non-exported
+     * package in a parent layer.
      */
     public void testConcealSamePackageAsBootLayer() {
 
@@ -667,9 +667,9 @@
 
 
     /**
-     * Attempt to use Layer.create to create a layer with a module defined to a
-     * class loader that already has a module of the same name defined to the
-     * class loader.
+     * Attempt to use Layer defineModules to create a layer with a module
+     * defined to a class loader that already has a module of the same name
+     * defined to the class loader.
      */
     @Test(expectedExceptions = { LayerInstantiationException.class })
     public void testModuleAlreadyDefinedToLoader() {
@@ -696,9 +696,9 @@
 
 
     /**
-     * Attempt to use Layer.create to create a Layer with a module containing
-     * package {@code p} where the class loader already has a module defined
-     * to it containing package {@code p}.
+     * Attempt to use Layer defineModules to create a Layer with a module
+     * containing package {@code p} where the class loader already has a module
+     * defined to it containing package {@code p}.
      */
     @Test(expectedExceptions = { LayerInstantiationException.class })
     public void testPackageAlreadyInNamedModule() {
@@ -738,8 +738,9 @@
 
 
     /**
-     * Attempt to use Layer.create to create a Layer with a module containing
-     * a package in which a type is already loaded by the class loader.
+     * Attempt to use Layer defineModules to create a Layer with a module
+     * containing a package in which a type is already loaded by the class
+     * loader.
      */
     @Test(expectedExceptions = { LayerInstantiationException.class })
     public void testPackageAlreadyInUnnamedModule() throws Exception {
@@ -763,6 +764,46 @@
 
 
     /**
+     * Attempt to create a Layer with a module named "java.base".
+     */
+    public void testLayerWithJavaBase() {
+        ModuleDescriptor descriptor
+            = new ModuleDescriptor.Builder("java.base")
+                .exports("java.lang")
+                .build();
+
+        ModuleFinder finder = ModuleUtils.finderOf(descriptor);
+
+        Configuration cf = Layer.boot()
+            .configuration()
+            .resolveRequires(finder, ModuleFinder.of(), Set.of("java.base"));
+        assertTrue(cf.modules().size() == 1);
+
+        ClassLoader scl = ClassLoader.getSystemClassLoader();
+
+        try {
+            Layer.boot().defineModules(cf, loader -> null );
+            assertTrue(false);
+        } catch (LayerInstantiationException e) { }
+
+        try {
+            Layer.boot().defineModules(cf, loader -> new ClassLoader() { });
+            assertTrue(false);
+        } catch (LayerInstantiationException e) { }
+
+        try {
+            Layer.boot().defineModulesWithOneLoader(cf, scl);
+            assertTrue(false);
+        } catch (LayerInstantiationException e) { }
+
+        try {
+            Layer.boot().defineModulesWithManyLoaders(cf, scl);
+            assertTrue(false);
+        } catch (LayerInstantiationException e) { }
+    }
+
+
+    /**
      * Parent of configuration != configuration of parent Layer
      */
     @Test(expectedExceptions = { IllegalArgumentException.class })
@@ -812,7 +853,6 @@
 
     @Test(expectedExceptions = { NullPointerException.class })
     public void testCreateWithNull2() {
-        ClassLoader loader = new ClassLoader() { };
         Configuration cf = resolveRequires(Layer.boot().configuration(), ModuleFinder.of());
         Layer.boot().defineModules(cf, null);
     }
--- a/jdk/test/java/lang/reflect/Layer/LayerAndLoadersTest.java	Wed May 25 17:07:13 2016 +0300
+++ b/jdk/test/java/lang/reflect/Layer/LayerAndLoadersTest.java	Wed May 25 19:58:03 2016 +0100
@@ -70,7 +70,7 @@
 
 
     /**
-     * Basic test of Layer.defineModulesWithOneLoader
+     * Basic test of Layer defineModulesWithOneLoader
      *
      * Test scenario:
      *   m1 requires m2 and m3
@@ -99,7 +99,7 @@
 
 
     /**
-     * Basic test of Layer.defineModulesWithManyLoaders
+     * Basic test of Layer defineModulesWithManyLoaders
      *
      * Test scenario:
      *   m1 requires m2 and m3
@@ -131,7 +131,7 @@
 
 
     /**
-     * Basic test of Layer.defineModulesWithOneLoader where one of the modules
+     * Basic test of Layer defineModulesWithOneLoader where one of the modules
      * is a service provider module.
      *
      * Test scenario:
@@ -172,8 +172,8 @@
 
 
     /**
-     * Basic test of Layer.defineModulesWithManyLoaders where one of the modules
-     * is a service provider module.
+     * Basic test of Layer defineModulesWithManyLoaders where one of the
+     * modules is a service provider module.
      *
      * Test scenario:
      *    m1 requires m2 and m3
@@ -224,7 +224,7 @@
 
 
     /**
-     * Tests that the class loaders created by Layer.createWithXXX delegate
+     * Tests that the class loaders created by defineModulesWithXXX delegate
      * to the given parent class loader.
      */
     public void testDelegationToParent() throws Exception {
@@ -254,7 +254,7 @@
 
 
     /**
-     * Test Layer.createWithXXX when modules that have overlapping packages.
+     * Test defineModulesWithXXX when modules that have overlapping packages.
      *
      * Test scenario:
      *   m1 exports p
@@ -288,7 +288,7 @@
 
 
     /**
-     * Test Layer.createWithXXX with split delegation.
+     * Test Layer defineModulesWithXXX with split delegation.
      *
      * Test scenario:
      *   layer1: m1 exports p, m2 exports p
@@ -319,7 +319,8 @@
 
         ModuleFinder finder2 = ModuleUtils.finderOf(descriptor3, descriptor4);
 
-        Configuration cf2 = cf1.resolveRequires(finder2, ModuleFinder.of(), Set.of("m3", "m4"));
+        Configuration cf2 = cf1.resolveRequires(finder2, ModuleFinder.of(),
+                                                Set.of("m3", "m4"));
 
         // package p cannot be supplied by two class loaders
         try {
@@ -335,8 +336,8 @@
 
 
     /**
-     * Test Layer.createWithXXX when the modules that override same named
-     * modules in the parent layer.
+     * Test Layer defineModulesWithXXX when the modules that override same
+     * named modules in the parent layer.
      *
      * Test scenario:
      *   layer1: m1, m2, m3 => same loader
@@ -350,7 +351,8 @@
         checkLayer(layer1, "m1", "m2", "m3");
 
         ModuleFinder finder = ModuleFinder.of(MODS_DIR);
-        Configuration cf2 = cf1.resolveRequires(finder, ModuleFinder.of(), Set.of("m1"));
+        Configuration cf2 = cf1.resolveRequires(finder, ModuleFinder.of(),
+                                                Set.of("m1"));
 
         Layer layer2 = layer1.defineModulesWithOneLoader(cf2, null);
         checkLayer(layer2, "m1", "m2", "m3");
@@ -383,8 +385,8 @@
 
 
     /**
-     * Test Layer.createWithXXX when the modules that override same named
-     * modules in the parent layer.
+     * Test Layer defineModulesWithXXX when the modules that override same
+     * named modules in the parent layer.
      *
      * Test scenario:
      *   layer1: m1, m2, m3 => loader pool
@@ -398,7 +400,8 @@
         checkLayer(layer1, "m1", "m2", "m3");
 
         ModuleFinder finder = ModuleFinder.of(MODS_DIR);
-        Configuration cf2 = cf1.resolveRequires(finder, ModuleFinder.of(), Set.of("m1"));
+        Configuration cf2 = cf1.resolveRequires(finder, ModuleFinder.of(),
+                                                Set.of("m1"));
 
         Layer layer2 = layer1.defineModulesWithManyLoaders(cf2, null);
         checkLayer(layer2, "m1", "m2", "m3");
@@ -477,8 +480,8 @@
 
 
     /**
-     * Test Layer.createWithXXX when the modules that override same named
-     * modules in the parent layer.
+     * Test Layer defineModulesWithXXX when the modules that override same
+     * named modules in the parent layer.
      *
      * layer1: m1, m2, m3 => same loader
      * layer2: m1, m3 => same loader
@@ -492,7 +495,8 @@
 
         ModuleFinder finder = finderFor("m1", "m3");
 
-        Configuration cf2 = cf1.resolveRequires(finder, ModuleFinder.of(), Set.of("m1"));
+        Configuration cf2 = cf1.resolveRequires(finder, ModuleFinder.of(),
+                                                Set.of("m1"));
 
         Layer layer2 = layer1.defineModulesWithOneLoader(cf2, null);
         checkLayer(layer2, "m1", "m3");
@@ -513,8 +517,8 @@
 
 
     /**
-     * Test Layer.createWithXXX when the modules that override same named
-     * modules in the parent layer.
+     * Test Layer defineModulesWithXXX when the modules that override same
+     * named modules in the parent layer.
      *
      * layer1: m1, m2, m3 => loader pool
      * layer2: m1, m3 => loader pool
@@ -528,7 +532,8 @@
 
         ModuleFinder finder = finderFor("m1", "m3");
 
-        Configuration cf2 = cf1.resolveRequires(finder, ModuleFinder.of(), Set.of("m1"));
+        Configuration cf2 = cf1.resolveRequires(finder, ModuleFinder.of(),
+                                                Set.of("m1"));
 
         Layer layer2 = layer1.defineModulesWithManyLoaders(cf2, null);
         checkLayer(layer2, "m1", "m3");