In the prototype that allows the vm to boot with modules on the module path 
that have packages with the same name, I have modified the ModuleBootstrap to 
use an alternate Function<String, ClassLoader> mapping that creates a 
jdk.internal.loader.Loader for the modules that are not in the set of 
boot/platform modules so that duplicate packages in separate modules do not 
cause the vm to fail the boot phase. Attached is the very rough first version 
of the prototype. Most of the changes are just System.err.printf statements to 
debug what is happening during the bootstrap phase.

There are a couple of places that need to know whether a module is in the 
boot/platform set in order to assign the correct ClassLoader.

----- Original Message -----
From: "Alan Bateman" <alan.bate...@oracle.com>
To: "Scott Stark" <sst...@redhat.com>, jigsaw-dev@openjdk.java.net
Sent: Thursday, October 6, 2016 9:29:57 AM
Subject: Re: Adding boot and platform module flags to ResolvedModule

On 06/10/2016 17:15, Scott Stark wrote:

> Hello,
>
> I'm currently prototyping an alternate bootstrap implementation that will 
> allow packages with the same name to exist across modules, and one thing I 
> have run across is the need to know what the boot and platform modules are 
> since there are restrictions on what type of ClassLoader can be used with 
> them. Currently this information is located in the
> jdk.internal.module.ModuleLoaderMap class. In my prototype I just exposed 
> that info via static public methods on ModuleLoaderMap, but it seems like the 
> appropriate place for this is as read only attributes on the 
> java.lang.module.ResolvedModule class.
>
> Does that make sense?
ModuleLoaderMap is generated at build time with the mapping of modules 
to loader. Once the VM initialized then there shouldn't be any further 
need for it and not clear that it would be worth exposing it in the API. 
Maybe you could expand a bit more on what you doing as it's hard to 
understand how you might use this.

-Alan
diff -r e0864f0eb268 src/java.base/share/classes/java/lang/reflect/Layer.java
--- a/src/java.base/share/classes/java/lang/reflect/Layer.java	Thu Sep 15 15:17:35 2016 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/Layer.java	Wed Oct 05 22:52:40 2016 -0700
@@ -285,6 +285,7 @@
         checkGetClassLoaderPermission();
 
         LoaderPool pool = new LoaderPool(cf, this, parentLoader);
+        System.err.printf("defineModulesWithManyLoaders, cf=%s, parentLoader=%s\n", cf, parentLoader);
         try {
             return new Layer(cf, this, pool::loaderFor);
         } catch (IllegalArgumentException e) {
@@ -374,6 +375,7 @@
             return new Layer(cf, this, clf);
         } catch (IllegalArgumentException iae) {
             // IAE is thrown by VM when defining the module fails
+iae.printStackTrace();
             throw new LayerInstantiationException(iae.getMessage());
         }
     }
@@ -415,8 +417,9 @@
             for (String p : descriptor.packages()) {
                 String other = packageToModule.putIfAbsent(p, name);
                 if (other != null) {
-                    throw fail("Package " + p + " in both module "
-                               + name + " and module " + other);
+System.err.printf("Package: %s in both module: %s and %s\n", p, name, other);
+                    //throw fail("Package " + p + " in both module "
+                    //           + name + " and module " + other);
                 }
             }
         }
diff -r e0864f0eb268 src/java.base/share/classes/java/lang/reflect/Module.java
--- a/src/java.base/share/classes/java/lang/reflect/Module.java	Thu Sep 15 15:17:35 2016 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/Module.java	Wed Oct 05 22:52:40 2016 -0700
@@ -52,6 +52,9 @@
 
 import jdk.internal.loader.BuiltinClassLoader;
 import jdk.internal.loader.BootLoader;
+import jdk.internal.loader.ClassLoaders;
+import jdk.internal.loader.Loader;
+import jdk.internal.loader.LoaderPool;
 import jdk.internal.loader.ResourceHelper;
 import jdk.internal.misc.JavaLangAccess;
 import jdk.internal.misc.JavaLangReflectModuleAccess;
@@ -135,7 +138,15 @@
         String vs = Objects.toString(version, null);
         String loc = Objects.toString(uri, null);
 
-        defineModule0(this, vs, loc, array);
+        try {
+            System.err.printf("DefiningModule(%s,%s)\n", name, loader);
+            defineModule0(this, vs, loc, array);
+        } catch (Throwable t) {
+            System.err.printf("Failed to define module: %s, uri:%s, cl: %s, err: %s\n", name, uri, loader, t.getMessage());
+            t.printStackTrace();
+System.err.flush();
+            throw t;
+        }
     }
 
 
@@ -1001,12 +1012,22 @@
 
         boolean isBootLayer = (Layer.boot() == null);
         Set<ClassLoader> loaders = new HashSet<>();
+        ClassLoader appClassLoader = ClassLoaders.appClassLoader();
+        LoaderPool appLoaderPool = null; //new LoaderPool(cf, layer, appClassLoader);
+        HashSet<ResolvedModule> appModules = new HashSet<>();
+        System.err.printf("Module.defineModules(), isBootLayer=%s\n", isBootLayer);
 
         // map each module to a class loader
         for (ResolvedModule resolvedModule : cf.modules()) {
             String name = resolvedModule.name();
             ClassLoader loader = clf.apply(name);
+            System.err.printf("CL(%s) = %s\n", name, loader);
             if (loader != null) {
+                if (loader == appClassLoader) {
+                    loader = new Loader(resolvedModule, appLoaderPool, appClassLoader);
+                    System.err.printf("\tnewCL(%s) = %s\n", name, loader);
+                    appModules.add(resolvedModule);
+                }
                 moduleToLoader.put(name, loader);
                 loaders.add(loader);
             } else if (!isBootLayer) {
@@ -1064,6 +1085,9 @@
             if (descriptor.isAutomatic()) {
                 m.implAddReads(ALL_UNNAMED_MODULE, true);
             }
+            if (appModules.contains(resolvedModule)) {
+                m.implAddReads(ALL_UNNAMED_MODULE, true);
+            }
 
             // exports
             initExports(descriptor, nameToModule, m);
diff -r e0864f0eb268 src/java.base/share/classes/jdk/internal/loader/BuiltinClassLoader.java
--- a/src/java.base/share/classes/jdk/internal/loader/BuiltinClassLoader.java	Thu Sep 15 15:17:35 2016 -0700
+++ b/src/java.base/share/classes/jdk/internal/loader/BuiltinClassLoader.java	Wed Oct 05 22:52:40 2016 -0700
@@ -129,6 +129,9 @@
         ModuleReference mref() { return mref; }
         String name() { return mref.descriptor().name(); }
         URL codeSourceURL() { return codeSourceURL; }
+        public String toString() {
+            return String.format("LoadedModule(BCL=%s), cs:%s, mref=%s", loader.getName(), codeSourceURL, mref);
+        }
     }
 
 
@@ -155,6 +158,7 @@
 
         this.nameToModule = new ConcurrentHashMap<>();
         this.moduleToReader = new ConcurrentHashMap<>();
+        System.err.println("BuiltinClassLoader("+name+"), ucp="+ucp);
     }
 
     /**
@@ -168,6 +172,7 @@
         }
 
         LoadedModule loadedModule = new LoadedModule(this, mref);
+        System.err.printf("BuiltinClassLoader(%s).loadModule(%s)\n", getName(), mn);
         for (String pn : mref.descriptor().packages()) {
             LoadedModule other = packageToModule.putIfAbsent(pn, loadedModule);
             if (other != null) {
@@ -468,8 +473,11 @@
         throws ClassNotFoundException
     {
         Class<?> c = loadClassOrNull(cn, resolve);
-        if (c == null)
+        if (c == null) {
+System.err.printf("BuiltinClassLoader(%s), CNFE(%s), moduleToReader=%s, nameToModule=%s\n",
+        getName(), cn, moduleToReader, nameToModule);
             throw new ClassNotFoundException(cn);
+        }
         return c;
     }
 
diff -r e0864f0eb268 src/java.base/share/classes/jdk/internal/loader/Loader.java
--- a/src/java.base/share/classes/jdk/internal/loader/Loader.java	Thu Sep 15 15:17:35 2016 -0700
+++ b/src/java.base/share/classes/jdk/internal/loader/Loader.java	Wed Oct 05 22:52:40 2016 -0700
@@ -59,6 +59,7 @@
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 
+import jdk.internal.module.ModuleLoaderMap;
 
 /**
  * A class loader that loads classes and resources from a collection of
@@ -86,6 +87,7 @@
     static {
         ClassLoader.registerAsParallelCapable();
     }
+    static boolean didStackTrace = false;
 
     // the loader pool is in a pool, can be null
     private final LoaderPool pool;
@@ -134,6 +136,9 @@
         String name() { return mref.descriptor().name(); }
         URL location() { return url; }
         CodeSource codeSource() { return cs; }
+        public String toString() {
+            return String.format("LoadedModule(%s), URL:%s, mref=%s", name(), url, mref);
+        }
     }
 
 
@@ -145,7 +150,7 @@
                   LoaderPool pool,
                   ClassLoader parent)
     {
-        super(parent);
+        super(resolvedModule.name(), parent);
 
         this.pool = pool;
         this.parent = parent;
@@ -159,8 +164,12 @@
         LoadedModule lm = new LoadedModule(mref);
         descriptor.packages().forEach(pn -> localPackageToModule.put(pn, lm));
         this.localPackageToModule = localPackageToModule;
+System.err.printf("Loader(this=%s/%s; parent=%s, resolvedModule=%s) localPackageToModule: %s\n", getName(), super.toString(), parent, resolvedModule.name(), localPackageToModule);
 
         this.acc = AccessController.getContext();
+        if(!didStackTrace) {
+            didStackTrace = true;
+        }
     }
 
     /**
@@ -193,6 +202,7 @@
         this.localPackageToModule = localPackageToModule;
 
         this.acc = AccessController.getContext();
+System.err.printf("Loader(this=%s; parent=%s, modules=%s) localPackageToModule: %s\n", this, parent, modules, localPackageToModule);
     }
 
 
@@ -208,10 +218,12 @@
      */
     public Loader initRemotePackageMap(Configuration cf, Layer parentLayer) {
 
+        System.err.printf("Loader(%s/%s).initRemotePackageMap, nameToModule=%s\n", getName(), super.toString(), nameToModule);
         for (String name : nameToModule.keySet()) {
 
             ResolvedModule resolvedModule = cf.findModule(name).get();
             assert resolvedModule.configuration() == cf;
+            System.err.printf("\tresolvedModule(%s), reads=%s\n", name, resolvedModule.reads());
 
             for (ResolvedModule other : resolvedModule.reads()) {
                 String mn = other.name();
@@ -228,8 +240,14 @@
                     }
 
                     loader = pool.loaderFor(mn);
+                    if(loader == null) {
+                        loader = ClassLoaders.platformClassLoader();
+                    }
                     assert loader != null;
-
+synchronized (ClassLoader.class) {
+    System.err.printf("\tif#1, Loader(%s) = %s\n", mn, loader);
+    System.err.flush();
+}
                 } else {
 
                     // find the layer contains the module that is read
@@ -247,13 +265,23 @@
                     // boot loader
                     assert layer.findModule(mn).isPresent();
                     loader = layer.findLoader(mn);
-                    if (loader == null)
+                    boolean isBoot = false;
+                    boolean isPlatform = false;
+                    if (loader == null) {
+                        isBoot = ModuleLoaderMap.isBootModule(mn);
+                        isPlatform = ModuleLoaderMap.isPlatformModule(mn);
                         loader = ClassLoaders.platformClassLoader();
+                    }
+synchronized (ClassLoader.class) {
+    System.err.printf("\telse#1, Loader(%s,boot=%s,platform=%s) = %s\n", mn, isBoot, isPlatform, loader);
+    System.err.flush();
+}
                 }
 
                 // find the packages that are exported to the target module
                 String target = resolvedModule.name();
                 ModuleDescriptor descriptor = other.reference().descriptor();
+System.err.printf("\ttarget(%s).exports=%s\n", target, descriptor.exports());
                 for (ModuleDescriptor.Exports e : descriptor.exports()) {
                     boolean delegate;
                     if (e.isQualified()) {
@@ -265,8 +293,10 @@
                         delegate = true;
                     }
 
+System.err.printf("\ttarget(%s).export=%s, isQualified=%s, delegate=%s\n", target, e, e.isQualified(), delegate);
                     if (delegate) {
                         String pn = e.source();
+                        System.err.printf("Adding package=%s to remote map via loader=%s\n", pn, loader);
                         ClassLoader l = remotePackageToLoader.putIfAbsent(pn, loader);
                         if (l != null && l != loader) {
                             throw new IllegalArgumentException("Package "
@@ -288,8 +318,8 @@
                 }
             }
 
+System.err.printf("\t%s/%s, remotePackageToLoader:%s\n", name, super.toString(), remotePackageToLoader);
         }
-
         return this;
     }
 
@@ -463,7 +493,7 @@
             if (c == null) {
 
                 LoadedModule loadedModule = findLoadedModule(cn);
-
+System.err.printf("Loader(%s/%s).loadClass(%s), lm=%s\n", getName(), super.toString(), cn, loadedModule);
                 if (loadedModule != null) {
 
                     // class is in module defined to this class loader
@@ -474,6 +504,7 @@
                     // type in another module or visible via the parent loader
                     String pn = packageName(cn);
                     ClassLoader loader = remotePackageToLoader.get(pn);
+System.err.printf("Loader(%s).remotePackageToLoader(%s), loader=%s\n", getName(), pn, loader);
                     if (loader == null) {
                         // type not in a module read by any of the modules
                         // defined to this loader, so delegate to parent
@@ -483,7 +514,13 @@
                     if (loader == null) {
                         c = BootLoader.loadClassOrNull(cn);
                     } else {
-                        c = loader.loadClass(cn);
+                        try {
+                            c = loader.loadClass(cn);
+                        } catch (ClassNotFoundException e) {
+                            System.err.printf("Failed to load(%s) via: %s\n", cn, loader);
+                            System.err.printf("%s, remotePackageToLoader=%s\n", getName(), remotePackageToLoader);
+                            throw e;
+                        }
                     }
 
                 }
diff -r e0864f0eb268 src/java.base/share/classes/jdk/internal/loader/LoaderPool.java
--- a/src/java.base/share/classes/jdk/internal/loader/LoaderPool.java	Thu Sep 15 15:17:35 2016 -0700
+++ b/src/java.base/share/classes/jdk/internal/loader/LoaderPool.java	Wed Oct 05 22:52:40 2016 -0700
@@ -33,6 +33,8 @@
 import java.util.Map;
 import java.util.stream.Stream;
 
+import jdk.internal.module.ModuleLoaderMap;
+
 /**
  * A pool of class loaders.
  *
@@ -56,8 +58,11 @@
     {
         Map<String, Loader> loaders = new HashMap<>();
         for (ResolvedModule resolvedModule : cf.modules()) {
+            String mn = resolvedModule.name();
+            // Skip boot and platform modules
+            if(ModuleLoaderMap.isBootModule(mn) || ModuleLoaderMap.isPlatformModule(mn))
+                continue;
             Loader loader = new Loader(resolvedModule, this, parentLoader);
-            String mn = resolvedModule.name();
             loaders.put(mn, loader);
         }
         this.loaders = loaders;
diff -r e0864f0eb268 src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
--- a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java	Thu Sep 15 15:17:35 2016 -0700
+++ b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java	Wed Oct 05 22:52:40 2016 -0700
@@ -47,6 +47,8 @@
 
 import jdk.internal.loader.BootLoader;
 import jdk.internal.loader.BuiltinClassLoader;
+import jdk.internal.loader.ClassLoaders;
+import jdk.internal.loader.ModuleToLoader;
 import jdk.internal.misc.SharedSecrets;
 import jdk.internal.perf.PerfCounter;
 
@@ -254,7 +256,7 @@
             needPostResolutionChecks = false;
         }
 
-        PrintStream traceOutput = null;
+        PrintStream traceOutput = System.err;
         if (Boolean.getBoolean("jdk.launcher.traceResolver"))
             traceOutput = System.out;
 
@@ -270,7 +272,8 @@
 
 
         // mapping of modules to class loaders
-        Function<String, ClassLoader> clf = ModuleLoaderMap.mappingFunction(cf);
+        //Function<String, ClassLoader> clf = ModuleLoaderMap.mappingFunction(cf);
+        Function<String, ClassLoader> clf = ModuleToLoader.mappingFunction(cf);
 
         // check that all modules to be mapped to the boot loader will be
         // loaded from the runtime image
@@ -295,7 +298,12 @@
         long t4 = System.nanoTime();
 
         // define modules to VM/runtime
+System.err.printf("+++ ModuleBootstrap.defineModules\n");
         Layer bootLayer = Layer.empty().defineModules(cf, clf);
+        ClassLoader appClassLoader = ClassLoaders.appClassLoader();
+System.err.printf("+++ platformLoader: %s\n", ClassLoaders.platformClassLoader());
+        // Separate out the
+        //Layer bootLayer = Layer.empty().defineModulesWithManyLoaders(cf, null);
 
         PerfCounters.layerCreateTime.addElapsedTimeFrom(t4);
 
@@ -309,7 +317,7 @@
             ClassLoader cl = clf.apply(name);
             if (cl == null) {
                 if (!name.equals(JAVA_BASE)) BootLoader.loadModule(mref);
-            } else {
+            } else if(cl instanceof BuiltinClassLoader){
                 ((BuiltinClassLoader)cl).loadModule(mref);
             }
         }
diff -r e0864f0eb268 src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java
--- a/src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java	Thu Sep 15 15:17:35 2016 -0700
+++ b/src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java	Wed Oct 05 22:52:40 2016 -0700
@@ -27,6 +27,7 @@
 
 import java.lang.module.Configuration;
 import java.lang.module.ResolvedModule;
+import java.lang.reflect.Layer;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -34,13 +35,15 @@
 import java.util.function.Function;
 
 import jdk.internal.loader.ClassLoaders;
+import jdk.internal.loader.Loader;
+import jdk.internal.loader.LoaderPool;
 
 
 /**
  * The module to class loader map.  The list of boot modules and platform modules
  * are generated at build time.
  */
-final class ModuleLoaderMap {
+public final class ModuleLoaderMap {
     /*
      * The list of boot modules and platform modules are generated at build time.
      */
@@ -48,6 +51,25 @@
         = new String[] { "@@BOOT_MODULE_NAMES@@" };
     private static final String[] PLATFORM_MODULES
         = new String[] { "@@PLATFORM_MODULE_NAMES@@" };
+    private static final Set<String> bootModules = new HashSet<>(BOOT_MODULES.length);
+    private static final Set<String> platformModules = new HashSet<>(PLATFORM_MODULES.length);
+
+    static {
+        for (String mn : BOOT_MODULES) {
+            bootModules.add(mn);
+        }
+
+        for (String mn : PLATFORM_MODULES) {
+            platformModules.add(mn);
+        }
+    }
+
+    static public boolean isBootModule(String name) {
+        return bootModules.contains(name);
+    }
+    static public boolean isPlatformModule(String name) {
+        return platformModules.contains(name);
+    }
 
     /**
      * Returns the function to map modules in the given configuration to the
@@ -55,16 +77,6 @@
      */
     static Function<String, ClassLoader> mappingFunction(Configuration cf) {
 
-        Set<String> bootModules = new HashSet<>(BOOT_MODULES.length);
-        for (String mn : BOOT_MODULES) {
-            bootModules.add(mn);
-        }
-
-        Set<String> platformModules = new HashSet<>(PLATFORM_MODULES.length);
-        for (String mn : PLATFORM_MODULES) {
-            platformModules.add(mn);
-        }
-
         ClassLoader platformClassLoader = ClassLoaders.platformClassLoader();
         ClassLoader appClassLoader = ClassLoaders.appClassLoader();
 
 

Reply via email to