Merge

jdk/src/java.base/share/classes/java/lang/Module.java

@@ -43,6 +43,7 @@ import java.security.PrivilegedAction;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -55,8 +56,10 @@ import java.util.stream.Stream;
 
 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.IllegalAccessLogger;
 import jdk.internal.module.ModuleLoaderMap;
 import jdk.internal.module.ServicesCatalog;
 import jdk.internal.module.Resources;
@@ -162,7 +165,6 @@ public final class Module implements AnnotatedElement {
     }
 
 
-
     /**
      * Returns {@code true} if this module is a named module.
      *
@@ -249,12 +251,10 @@ public final class Module implements AnnotatedElement {
 
     // special Module to mean "all unnamed modules"
     private static final Module ALL_UNNAMED_MODULE = new Module(null);
+    private static final Set<Module> ALL_UNNAMED_MODULE_SET = Set.of(ALL_UNNAMED_MODULE);
 
     // special Module to mean "everyone"
     private static final Module EVERYONE_MODULE = new Module(null);
-
-    // set contains EVERYONE_MODULE, used when a package is opened or
-    // exported unconditionally
     private static final Set<Module> EVERYONE_SET = Set.of(EVERYONE_MODULE);
 
 
@@ -534,12 +534,12 @@ public final class Module implements AnnotatedElement {
             return true;
 
         // all packages are exported/open to self
-        if (other == this && containsPackage(pn))
+        if (other == this && descriptor.packages().contains(pn))
             return true;
 
         // all packages in open and automatic modules are open
         if (descriptor.isOpen() || descriptor.isAutomatic())
-            return containsPackage(pn);
+            return descriptor.packages().contains(pn);
 
         // exported/opened via module declaration/descriptor
         if (isStaticallyExportedOrOpen(pn, other, open))
@@ -555,42 +555,48 @@ public final class Module implements AnnotatedElement {
 
     /**
      * Returns {@code true} if this module exports or opens a package to
-     * the given module via its module declaration.
+     * the given module via its module declaration or CLI options.
      */
     private boolean isStaticallyExportedOrOpen(String pn, Module other, boolean open) {
-        // package is open to everyone or <other>
+        // test if package is open to everyone or <other>
         Map<String, Set<Module>> openPackages = this.openPackages;
-        if (openPackages != null) {
-            Set<Module> targets = openPackages.get(pn);
-            if (targets != null) {
-                if (targets.contains(EVERYONE_MODULE))
-                    return true;
-                if (other != EVERYONE_MODULE && targets.contains(other))
-                    return true;
-            }
+        if (openPackages != null && allows(openPackages.get(pn), other)) {
+            return true;
         }
 
         if (!open) {
-            // package is exported to everyone or <other>
+            // test package is exported to everyone or <other>
             Map<String, Set<Module>> exportedPackages = this.exportedPackages;
-            if (exportedPackages != null) {
-                Set<Module> targets = exportedPackages.get(pn);
-                if (targets != null) {
-                    if (targets.contains(EVERYONE_MODULE))
-                        return true;
-                    if (other != EVERYONE_MODULE && targets.contains(other))
-                        return true;
-                }
+            if (exportedPackages != null && allows(exportedPackages.get(pn), other)) {
+                return true;
             }
         }
 
         return false;
     }
 
+    /**
+     * Returns {@code true} if targets is non-null and contains EVERYONE_MODULE
+     * or the given module. Also returns true if the given module is an unnamed
+     * module and targets contains ALL_UNNAMED_MODULE.
+     */
+    private boolean allows(Set<Module> targets, Module module) {
+       if (targets != null) {
+           if (targets.contains(EVERYONE_MODULE))
+               return true;
+           if (module != EVERYONE_MODULE) {
+               if (targets.contains(module))
+                   return true;
+               if (!module.isNamed() && targets.contains(ALL_UNNAMED_MODULE))
+                   return true;
+           }
+        }
+        return false;
+    }
 
     /**
-     * Returns {@code true} if this module reflectively exports or opens given
-     * package package to the given module.
+     * Returns {@code true} if this module reflectively exports or opens the
+     * given package to the given module.
      */
     private boolean isReflectivelyExportedOrOpen(String pn, Module other, boolean open) {
         // exported or open to all modules
@@ -632,6 +638,22 @@ public final class Module implements AnnotatedElement {
         return false;
     }
 
+    /**
+     * Returns {@code true} if this module reflectively exports the
+     * given package to the given module.
+     */
+    boolean isReflectivelyExported(String pn, Module other) {
+        return isReflectivelyExportedOrOpen(pn, other, false);
+    }
+
+    /**
+     * Returns {@code true} if this module reflectively opens the
+     * given package to the given module.
+     */
+    boolean isReflectivelyOpened(String pn, Module other) {
+        return isReflectivelyExportedOrOpen(pn, other, true);
+    }
+
 
     /**
      * If the caller's module is this module then update this module to export
@@ -800,7 +822,7 @@ public final class Module implements AnnotatedElement {
     }
 
     /**
-     * Updates this module to export a package to all unnamed modules.
+     * Updates this module to open a package to all unnamed modules.
      *
      * @apiNote Used by the --add-opens command line option.
      */
@@ -808,7 +830,6 @@ public final class Module implements AnnotatedElement {
         implAddExportsOrOpens(pn, Module.ALL_UNNAMED_MODULE, true, true);
     }
 
-
     /**
      * Updates a module to export or open a module to another module.
      *
@@ -825,12 +846,31 @@ public final class Module implements AnnotatedElement {
         if (!isNamed() || descriptor.isOpen() || descriptor.isAutomatic())
             return;
 
-        // nothing to do if already exported/open to other
-        if (implIsExportedOrOpen(pn, other, open))
-            return;
+        // check if the package is already exported/open to other
+        if (implIsExportedOrOpen(pn, other, open)) {
+
+            // if the package is exported/open for illegal access then we need
+            // to record that it has also been exported/opened reflectively so
+            // that the IllegalAccessLogger doesn't emit a warning.
+            boolean needToAdd = false;
+            if (!other.isNamed()) {
+                IllegalAccessLogger l = IllegalAccessLogger.illegalAccessLogger();
+                if (l != null) {
+                    if (open) {
+                        needToAdd = l.isOpenForIllegalAccess(this, pn);
+                    } else {
+                        needToAdd = l.isExportedForIllegalAccess(this, pn);
+                    }
+                }
+            }
+            if (!needToAdd) {
+                // nothing to do
+                return;
+            }
+        }
 
         // can only export a package in the module
-        if (!containsPackage(pn)) {
+        if (!descriptor.packages().contains(pn)) {
             throw new IllegalArgumentException("package " + pn
                                                + " not in contents");
         }
@@ -850,7 +890,6 @@ public final class Module implements AnnotatedElement {
         Map<String, Boolean> map = reflectivelyExports
             .computeIfAbsent(this, other,
                              (m1, m2) -> new ConcurrentHashMap<>());
-
         if (open) {
             map.put(pn, Boolean.TRUE);  // may need to promote from FALSE to TRUE
         } else {
@@ -858,6 +897,38 @@ public final class Module implements AnnotatedElement {
         }
     }
 
+    /**
+     * Updates a module to open all packages returned by the given iterator to
+     * all unnamed modules.
+     *
+     * @apiNote Used during startup to open packages for illegal access.
+     */
+    void implAddOpensToAllUnnamed(Iterator<String> iterator) {
+        if (jdk.internal.misc.VM.isModuleSystemInited()) {
+            throw new IllegalStateException("Module system already initialized");
+        }
+
+        // replace this module's openPackages map with a new map that opens
+        // the packages to all unnamed modules.
+        Map<String, Set<Module>> openPackages = this.openPackages;
+        if (openPackages == null) {
+            openPackages = new HashMap<>();
+        } else {
+            openPackages = new HashMap<>(openPackages);
+        }
+        while (iterator.hasNext()) {
+            String pn = iterator.next();
+            Set<Module> prev = openPackages.putIfAbsent(pn, ALL_UNNAMED_MODULE_SET);
+            if (prev != null) {
+                prev.add(ALL_UNNAMED_MODULE);
+            }
+
+            // update VM to export the package
+            addExportsToAllUnnamed0(this, pn);
+        }
+        this.openPackages = openPackages;
+    }
+
 
     // -- services --
 
@@ -947,19 +1018,6 @@ public final class Module implements AnnotatedElement {
 
     // -- packages --
 
-    // Additional packages that are added to the module at run-time.
-    private volatile Map<String, Boolean> extraPackages;
-
-    private boolean containsPackage(String pn) {
-        if (descriptor.packages().contains(pn))
-            return true;
-        Map<String, Boolean> extraPackages = this.extraPackages;
-        if (extraPackages != null && extraPackages.containsKey(pn))
-            return true;
-        return false;
-    }
-
-
     /**
      * Returns the set of package names for the packages in this module.
      *
@@ -974,86 +1032,19 @@ public final class Module implements AnnotatedElement {
      */
     public Set<String> getPackages() {
         if (isNamed()) {
-
-            Set<String> packages = descriptor.packages();
-            Map<String, Boolean> extraPackages = this.extraPackages;
-            if (extraPackages == null) {
-                return packages;
-            } else {
-                return Stream.concat(packages.stream(),
-                                     extraPackages.keySet().stream())
-                        .collect(Collectors.toSet());
-            }
-
+            return descriptor.packages();
         } else {
             // unnamed module
             Stream<Package> packages;
             if (loader == null) {
                 packages = BootLoader.packages();
             } else {
-                packages = SharedSecrets.getJavaLangAccess().packages(loader);
+                packages = loader.packages();
             }
             return packages.map(Package::getName).collect(Collectors.toSet());
         }
     }
 
-    /**
-     * Add a package to this module without notifying the VM.
-     *
-     * @apiNote This method is VM white-box testing.
-     */
-    void implAddPackageNoSync(String pn) {
-        implAddPackage(pn.replace('/', '.'), false);
-    }
-
-    /**
-     * Add a package to this module.
-     *
-     * If {@code syncVM} is {@code true} then the VM is notified. This method is
-     * a no-op if this is an unnamed module or the module already contains the
-     * package.
-     *
-     * @throws IllegalArgumentException if the package name is not legal
-     * @throws IllegalStateException if the package is defined to another module
-     */
-    private void implAddPackage(String pn, boolean syncVM) {
-        // no-op if unnamed module
-        if (!isNamed())
-            return;
-
-        // no-op if module contains the package
-        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);
-            }
-        }
-
-        // create extraPackages if needed
-        Map<String, Boolean> extraPackages = this.extraPackages;
-        if (extraPackages == null) {
-            synchronized (this) {
-                extraPackages = this.extraPackages;
-                if (extraPackages == null)
-                    this.extraPackages = extraPackages = new ConcurrentHashMap<>();
-            }
-        }
-
-        // update VM first in case it fails. This is a no-op if another thread
-        // beats us to add the package first
-        if (syncVM) {
-            // throws IllegalStateException if defined to another module
-            addPackage0(this, pn);
-        }
-        extraPackages.putIfAbsent(pn, Boolean.TRUE);
-    }
-
 
     // -- creating Module objects --
 
@@ -1072,18 +1063,22 @@ public final class Module implements AnnotatedElement {
         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()) {
+                if (!(clf instanceof ModuleLoaderMap.Mapper)) {
+                    throw new IllegalArgumentException("loader can't be 'null'"
+                            + " or the platform class loader");
+                }
+                hasPlatformModules = true;
+            } else {
                 loaders.add(loader);
-            } else if (!(clf instanceof ModuleLoaderMap.Mapper)) {
-                throw new IllegalArgumentException("loader can't be 'null'");
             }
         }
 
@@ -1095,7 +1090,7 @@ public final class Module implements AnnotatedElement {
             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 +1152,12 @@ public final class Module implements AnnotatedElement {
             }
         }
 
-        // 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 +1165,11 @@ public final class Module implements AnnotatedElement {
                     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);
                 }
             }
         }
@@ -1577,7 +1574,4 @@ public final class Module implements AnnotatedElement {
 
     // JVM_AddModuleExportsToAllUnnamed
     private static native void addExportsToAllUnnamed0(Module from, String pn);
-
-    // JVM_AddModulePackage
-    private static native void addPackage0(Module m, String pn);
 }