changeset 19709:532156733a50

SL handling of boot layer assumes all modules defined to app class loader Improve validation of implAddPackage, used for testing
author alanb
date Sun, 14 May 2017 16:08:55 +0100
parents b9d7773730d2
children 504e969a5102
files src/java.base/share/classes/java/lang/Module.java src/java.base/share/classes/java/util/ServiceLoader.java src/java.base/share/classes/jdk/internal/module/Modules.java
diffstat 3 files changed, 57 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/lang/Module.java	Sun May 14 16:05:55 2017 +0100
+++ b/src/java.base/share/classes/java/lang/Module.java	Sun May 14 16:08:55 2017 +0100
@@ -55,8 +55,10 @@
 
 import jdk.internal.loader.BuiltinClassLoader;
 import jdk.internal.loader.BootLoader;
+import jdk.internal.loader.ClassLoaders;
 import jdk.internal.misc.JavaLangAccess;
 import jdk.internal.misc.SharedSecrets;
+import jdk.internal.module.Checks;
 import jdk.internal.module.ModuleLoaderMap;
 import jdk.internal.module.ServicesCatalog;
 import jdk.internal.module.Resources;
@@ -162,7 +164,6 @@
     }
 
 
-
     /**
      * Returns {@code true} if this module is a named module.
      *
@@ -1025,15 +1026,8 @@
         if (containsPackage(pn))
             return;
 
-        // check package name is legal for named modules
-        if (pn.isEmpty())
-            throw new IllegalArgumentException("Cannot add <unnamed> package");
-        for (int i=0; i<pn.length(); i++) {
-            char c = pn.charAt(i);
-            if (c == '/' || c == ';' || c == '[') {
-                throw new IllegalArgumentException("Illegal character: " + c);
-            }
-        }
+        // check the package name is legal
+        Checks.requirePackageName(pn);
 
         // create extraPackages if needed
         Map<String, Boolean> extraPackages = this.extraPackages;
@@ -1075,18 +1069,21 @@
         Map<String, Module> nameToModule = new HashMap<>();
         Map<String, ClassLoader> moduleToLoader = new HashMap<>();
 
-        boolean isBootLayer = (ModuleLayer.boot() == null);
         Set<ClassLoader> loaders = new HashSet<>();
+        boolean hasPlatformModules = false;
 
         // map each module to a class loader
         for (ResolvedModule resolvedModule : cf.modules()) {
             String name = resolvedModule.name();
             ClassLoader loader = clf.apply(name);
-            if (loader != null) {
-                moduleToLoader.put(name, loader);
+            moduleToLoader.put(name, loader);
+            if (loader == null || loader == ClassLoaders.platformClassLoader()) {
+                hasPlatformModules = true;
+                if (loader == null && !(clf instanceof ModuleLoaderMap.Mapper)) {
+                    throw new IllegalArgumentException("loader can't be 'null'");
+                }
+            } else {
                 loaders.add(loader);
-            } else if (!(clf instanceof ModuleLoaderMap.Mapper)) {
-                throw new IllegalArgumentException("loader can't be 'null'");
             }
         }
 
@@ -1098,7 +1095,7 @@
             URI uri = mref.location().orElse(null);
             ClassLoader loader = moduleToLoader.get(resolvedModule.name());
             Module m;
-            if (loader == null && isBootLayer && name.equals("java.base")) {
+            if (loader == null && name.equals("java.base")) {
                 // java.base is already defined to the VM
                 m = Object.class.getModule();
             } else {
@@ -1157,8 +1154,12 @@
             initExportsAndOpens(m, nameToSource, nameToModule, layer.parents());
         }
 
-        // register the modules in the boot layer
-        if (isBootLayer) {
+        // if there are modules defined to the boot or platform class loaders
+        // then register the modules in the class loader's services catalog
+        if (hasPlatformModules) {
+            ClassLoader pcl = ClassLoaders.platformClassLoader();
+            ServicesCatalog bootCatalog = BootLoader.getServicesCatalog();
+            ServicesCatalog pclCatalog = ServicesCatalog.getServicesCatalog(pcl);
             for (ResolvedModule resolvedModule : cf.modules()) {
                 ModuleReference mref = resolvedModule.reference();
                 ModuleDescriptor descriptor = mref.descriptor();
@@ -1166,13 +1167,11 @@
                     String name = descriptor.name();
                     Module m = nameToModule.get(name);
                     ClassLoader loader = moduleToLoader.get(name);
-                    ServicesCatalog catalog;
                     if (loader == null) {
-                        catalog = BootLoader.getServicesCatalog();
-                    } else {
-                        catalog = ServicesCatalog.getServicesCatalog(loader);
+                        bootCatalog.register(m);
+                    } else if (loader == pcl) {
+                        pclCatalog.register(m);
                     }
-                    catalog.register(m);
                 }
             }
         }
--- a/src/java.base/share/classes/java/util/ServiceLoader.java	Sun May 14 16:05:55 2017 +0100
+++ b/src/java.base/share/classes/java/util/ServiceLoader.java	Sun May 14 16:08:55 2017 +0100
@@ -164,14 +164,15 @@
  *
  * <p> When locating providers using a class loader then any providers in named
  * modules defined to the class loader, or any class loader that is reachable
- * via parent delegation, are located. Additionally, providers in module layers
- * other than the {@link ModuleLayer#boot() boot} layer, where the module layer
- * contains modules defined to the class loader, or any class loader reachable
- * via parent delegation, are also located. For example, suppose there is a
+ * via parent delegation, are located. Additionally, and with the exception
+ * of the bootstrap and {@linkplain ClassLoader#getPlatformClassLoader()
+ * platform} class loaders, if the class loader, or any class loader reachable
+ * via parent delegation, defines modules in one or more module layers then the
+ * providers in the module layers are located. For example, suppose there is a
  * module layer where each module is defined to its own class loader (see {@link
- * ModuleLayer#defineModulesWithManyLoaders defineModulesWithManyLoaders}). If the
- * {@code load} method is invoked to locate providers using any of these class
- * loaders for this layer then it will locate all of the providers in that
+ * ModuleLayer#defineModulesWithManyLoaders defineModulesWithManyLoaders}). If
+ * the {@code load} method is invoked to locate providers using any of these
+ * class loaders for this layer then it will locate all of the providers in that
  * layer, irrespective of their defining class loader.
  *
  * <p> In the case of unnamed modules then the service configuration files are
@@ -957,13 +958,25 @@
         }
 
         /**
+         * Returns the class loader that a module is defined to
+         */
+        private ClassLoader loaderFor(Module module) {
+            SecurityManager sm = System.getSecurityManager();
+            if (sm == null) {
+                return module.getClassLoader();
+            } else {
+                PrivilegedAction<ClassLoader> pa = module::getClassLoader;
+                return AccessController.doPrivileged(pa);
+            }
+        }
+
+        /**
          * Returns an iterator to iterate over the implementations of {@code
          * service} in modules defined to the given class loader or in custom
          * layers with a module defined to this class loader.
          */
         private Iterator<ServiceProvider> iteratorFor(ClassLoader loader) {
-
-            // modules defined to this class loader
+            // modules defined to the class loader
             ServicesCatalog catalog;
             if (loader == null) {
                 catalog = BootLoader.getServicesCatalog();
@@ -977,17 +990,20 @@
                 providers = catalog.findServices(serviceName);
             }
 
-            // modules in custom layers that define modules to the class loader
-            if (loader == null) {
+            // modules in layers that define modules to the class loader
+            ClassLoader platformClassLoader = ClassLoaders.platformClassLoader();
+            if (loader == null || loader == platformClassLoader) {
                 return providers.iterator();
             } else {
                 List<ServiceProvider> allProviders = new ArrayList<>(providers);
-                ModuleLayer bootLayer = ModuleLayer.boot();
                 Iterator<ModuleLayer> iterator = LANG_ACCESS.layers(loader).iterator();
                 while (iterator.hasNext()) {
                     ModuleLayer layer = iterator.next();
-                    if (layer != bootLayer) {
-                        allProviders.addAll(providers(layer));
+                    for (ServiceProvider sp : providers(layer)) {
+                        ClassLoader l = loaderFor(sp.module());
+                        if (l != null && l != platformClassLoader) {
+                            allProviders.add(sp);
+                        }
                     }
                 }
                 return allProviders.iterator();
--- a/src/java.base/share/classes/jdk/internal/module/Modules.java	Sun May 14 16:05:55 2017 +0100
+++ b/src/java.base/share/classes/jdk/internal/module/Modules.java	Sun May 14 16:08:55 2017 +0100
@@ -136,10 +136,12 @@
     public static void addProvides(Module m, Class<?> service, Class<?> impl) {
         ModuleLayer layer = m.getLayer();
 
-        if (layer == null || layer == ModuleLayer.boot()) {
+        PrivilegedAction<ClassLoader> pa = m::getClassLoader;
+        ClassLoader loader = AccessController.doPrivileged(pa);
+
+        ClassLoader platformClassLoader = ClassLoaders.platformClassLoader();
+        if (layer == null || loader == null || loader == platformClassLoader) {
             // update ClassLoader catalog
-            PrivilegedAction<ClassLoader> pa = m::getClassLoader;
-            ClassLoader loader = AccessController.doPrivileged(pa);
             ServicesCatalog catalog;
             if (loader == null) {
                 catalog = BootLoader.getServicesCatalog();