gnodet-bot commented on code in PR #11029: URL: https://github.com/apache/maven/pull/11029#discussion_r4216710527
########## impl/maven-classworlds/src/main/java/org/codehaus/plexus/classworlds/realm/ClassRealm.java: ########## @@ -0,0 +1,556 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.plexus.classworlds.realm; + +/* + * Copyright 2001-2006 Codehaus Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.io.Closeable; +import java.io.IOException; +import java.io.PrintStream; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Collection; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.SortedSet; +import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import org.codehaus.plexus.classworlds.ClassWorld; +import org.codehaus.plexus.classworlds.strategy.Strategy; +import org.codehaus.plexus.classworlds.strategy.StrategyFactory; + +/** + * The class loading gateway. Each class realm has access to a base class loader, imports form zero or more other class + * loaders, an optional parent class loader and of course its own class path. When queried for a class/resource, a class + * realm will always query its base class loader first before it delegates to a pluggable strategy. The strategy in turn + * controls the order in which imported class loaders, the parent class loader and the realm itself are searched. The + * base class loader is assumed to be capable of loading of the bootstrap classes. + * + * @author <a href="mailto:[email protected]";>bob mcwhirter</a> + * @author Jason van Zyl + */ +public class ClassRealm extends URLClassLoader implements org.apache.maven.api.classworlds.ClassRealm { + + private ClassWorld world; + + private String id; + + private SortedSet<Entry> foreignImports; + + private SortedSet<Entry> parentImports; + + private Strategy strategy; + + private ClassLoader parentClassLoader; + + private ModuleLayer moduleLayer; + + private ModuleLayer.Controller moduleLayerController; + + private static final boolean IS_PARALLEL_CAPABLE = Closeable.class.isAssignableFrom(URLClassLoader.class); Review Comment: ⚠️ **`IS_PARALLEL_CAPABLE` is always `true` on Java 17+ — this is dead code** `Closeable.class.isAssignableFrom(URLClassLoader.class)` has been `true` since Java 7. This field drives six conditional branches (lines 108, 110, 279, 432, 441, 551) that will always take the `true` path on any supported Maven JDK (17+). The `false` branch (e.g. `lockMap = null`, skipping `registerAsParallelCapable()`) is unreachable. Consider removing the field and all conditional branches, keeping only the always-true path. This was fine to copy from the upstream classworlds when supporting JDK 5/6, but it's dead weight here. ########## api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/ClassRealm.java: ########## @@ -0,0 +1,253 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.api.classworlds; + +import java.io.Closeable; +import java.net.URL; + +import org.apache.maven.api.annotations.Experimental; +import org.apache.maven.api.annotations.Nonnull; +import org.apache.maven.api.annotations.Nullable; + +/** + * A class loading realm that provides isolated class loading with controlled imports and exports. + * <p> + * A ClassRealm represents an isolated class loading environment with its own classpath + * and controlled access to classes from other realms through imports. + * </p> + * + * @since 4.1.0 + */ +@Experimental +public interface ClassRealm extends Closeable { + + /** + * Returns the unique identifier for this realm. + * + * @return the realm identifier + */ + @Nonnull + String getId(); Review Comment: 💡 **API naming convention: Maven 4 uses noun-style accessors, not `getX()`** The new `api/maven-api-classworlds` API interface uses traditional JavaBean-style getters (`getId()`, `getWorld()`, `getClassLoader()`, `getStrategy()`, `getURLs()`, etc.), but Maven 4's API convention — as established in `maven-api-core` — uses noun-style accessors without the `get` prefix (`id()`, `world()`, `classLoader()`, etc.). This is worth deciding up front before this interface gets adopted, since it'll be an API break to fix later. See `org.apache.maven.api.Project`, `Session`, `Artifact`, etc. for the established pattern. ########## impl/maven-classworlds/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java: ########## @@ -0,0 +1,303 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.plexus.classworlds; + +/* + * Copyright 2001-2006 Codehaus Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.io.Closeable; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; + +import org.codehaus.plexus.classworlds.realm.ClassRealm; +import org.codehaus.plexus.classworlds.realm.DuplicateRealmException; +import org.codehaus.plexus.classworlds.realm.FilteredClassRealm; +import org.codehaus.plexus.classworlds.realm.NoSuchRealmException; + +/** + * A collection of <code>ClassRealm</code>s, indexed by id. + * + * @author <a href="mailto:[email protected]";>bob mcwhirter</a> + */ +public class ClassWorld implements org.apache.maven.api.classworlds.ClassWorld, Closeable { + private Map<String, ClassRealm> realms; + + private final List<ClassWorldListener> listeners = new ArrayList<>(); + + private ModuleLayer moduleLayer; + + private ModuleLayer.Controller moduleLayerController; + + public ClassWorld(String realmId, ClassLoader classLoader) { + this(); + + try { + newRealm(realmId, classLoader); + } catch (DuplicateRealmException e) { + // Will never happen as we are just creating the world. + } + } + + public ClassWorld() { + this.realms = new LinkedHashMap<>(); + } + + public ClassRealm newRealm(String id) throws DuplicateRealmException { + return newRealm(id, getClass().getClassLoader()); + } + + public ClassRealm newRealm(String id, ClassLoader classLoader) throws DuplicateRealmException { + return newRealm(id, classLoader, null); + } + + /** + * Adds a class realm with filtering. + * Only resources/classes whose name matches a given predicate are exposed. + * @param id The identifier for this realm, must not be <code>null</code>. + * @param classLoader The base class loader for this realm, may be <code>null</code> to use the bootstrap class + * loader. + * @param filter a predicate to apply to each resource name to determine if it should be loaded through this class loader + * @return the created class realm + * @throws DuplicateRealmException in case a realm with the given id does already exist + * @since 2.7.0 + * @see FilteredClassRealm + */ + public synchronized ClassRealm newRealm(String id, ClassLoader classLoader, Predicate<String> filter) + throws DuplicateRealmException { + if (realms.containsKey(id)) { + throw new DuplicateRealmException(this, id); + } + + ClassRealm realm; + + if (filter == null) { + realm = new ClassRealm(this, id, classLoader); + } else { + realm = new FilteredClassRealm(filter, this, id, classLoader); + } + realms.put(id, realm); + + for (ClassWorldListener listener : listeners) { + listener.realmCreated(realm); + } + + return realm; + } + + /** + * Closes all contained class realms. + * @since 2.7.0 + */ + @Override + public synchronized void close() throws IOException { + realms.values().stream().forEach(this::disposeRealm); + realms.clear(); + } + + public synchronized void disposeRealm(String id) throws NoSuchRealmException { + ClassRealm realm = realms.remove(id); + + if (realm != null) { + disposeRealm(realm); + } else { + throw new NoSuchRealmException(this, id); + } + } + + private void disposeRealm(ClassRealm realm) { + try { + realm.close(); + } catch (IOException ignore) { + } + for (ClassWorldListener listener : listeners) { + listener.realmDisposed(realm); + } + } + + public synchronized ClassRealm getRealm(String id) throws NoSuchRealmException { + if (realms.containsKey(id)) { + return realms.get(id); + } + + throw new NoSuchRealmException(this, id); + } + + public synchronized Collection<ClassRealm> getRealms() { + return Collections.unmodifiableList(new ArrayList<>(realms.values())); + } + + public synchronized void setModuleLayer(ModuleLayer moduleLayer, ModuleLayer.Controller controller) { + this.moduleLayer = moduleLayer; + this.moduleLayerController = controller; + } + + public ModuleLayer getModuleLayer() { + return moduleLayer; + } + + public ModuleLayer.Controller getModuleLayerController() { + return moduleLayerController; + } + + /** + * Exports a package from a named module to the given target module. + * Looks up the source module in the runtime layer first, then the boot layer. + * Only runtime layer modules can be modified via the Controller; boot layer + * modules require {@code --add-exports} in the launcher script. + * + * @param moduleName the source module name + * @param packageName the package to export + * @param target the target module to export to + */ + public synchronized void addExports(String moduleName, String packageName, Module target) { + applyModuleAccess(moduleName, packageName, target, false); + } + + /** + * Opens a package from a named module to the given target module for deep reflection. + * Looks up the source module in the runtime layer first, then the boot layer. + * Only runtime layer modules can be modified via the Controller; boot layer + * modules require {@code --add-opens} in the launcher script. + * + * @param moduleName the source module name + * @param packageName the package to open + * @param target the target module to open to + */ + public synchronized void addOpens(String moduleName, String packageName, Module target) { + applyModuleAccess(moduleName, packageName, target, true); + } + + /** + * Adds a reads edge from the named source module to the given target module. + * Only runtime layer modules can be modified via the Controller. + * + * @param sourceModuleName the source module name + * @param target the target module to read + */ + public synchronized void addReads(String sourceModuleName, Module target) { + Module source = findModule(sourceModuleName); + if (source == null || moduleLayerController == null || isBootLayerModule(source)) { + return; + } + moduleLayerController.addReads(source, target); + } + + private void applyModuleAccess(String moduleName, String packageName, Module target, boolean open) { + Module source = findModule(moduleName); + if (source == null || moduleLayerController == null || isBootLayerModule(source)) { + return; + } + if (open) { + moduleLayerController.addOpens(source, packageName, target); + } else { + moduleLayerController.addExports(source, packageName, target); + } + } + + private Module findModule(String moduleName) { + if (moduleLayer != null) { + Module m = moduleLayer.findModule(moduleName).orElse(null); + if (m != null) { + return m; + } + } + return ModuleLayer.boot().findModule(moduleName).orElse(null); + } + + private boolean isBootLayerModule(Module module) { + return module.getLayer() == ModuleLayer.boot(); + } + + // from exports branch + public synchronized ClassRealm getClassRealm(String id) { + if (realms.containsKey(id)) { + return realms.get(id); + } + + return null; + } + + public synchronized void addListener(ClassWorldListener listener) { + // TODO ideally, use object identity, not equals + if (!listeners.contains(listener)) { + listeners.add(listener); + } + } + + public synchronized void removeListener(ClassWorldListener listener) { + listeners.remove(listener); + } + + // API interface methods - newRealm with filter is already implemented above + + @Override + public void addListener(org.apache.maven.api.classworlds.ClassWorldListener listener) { + if (listener instanceof ClassWorldListener) { + addListener((ClassWorldListener) listener); + } else { + // Wrap the API listener + addListener(new ClassWorldListener() { + @Override + public void realmCreated(ClassRealm realm) { + listener.realmCreated(realm); + } + + @Override + public void realmDisposed(ClassRealm realm) { + listener.realmDisposed(realm); + } + + @Override + public void realmCreated(org.apache.maven.api.classworlds.ClassRealm realm) { + listener.realmCreated(realm); + } + + @Override + public void realmDisposed(org.apache.maven.api.classworlds.ClassRealm realm) { + listener.realmDisposed(realm); + } + }); + } + } + + @Override + public void removeListener(org.apache.maven.api.classworlds.ClassWorldListener listener) { Review Comment: ⚠️ **`removeListener` silently discards API listener removals** This implementation is a no-op: ```java @Override public void removeListener(org.apache.maven.api.classworlds.ClassWorldListener listener) { // For now, we'll need to track wrapped listeners if this becomes important // This is a limitation of the current design } ``` Code that calls `addListener` on the API-level `ClassWorld` and then `removeListener` will leak the listener permanently — no exception thrown, no warning logged. This violates the `ClassWorld` API contract and is a source of memory leaks and unexpected event delivery. The fix is to maintain a `IdentityHashMap<api.ClassWorldListener, impl.ClassWorldListener>` mapping when wrapping listeners in `addListener`, and look it up in `removeListener`. If you're intentionally deferring this, at least throw `UnsupportedOperationException` so callers know immediately rather than silently. ########## apache-maven/src/assembly/maven/bin/mvn.cmd: ########## @@ -388,25 +388,30 @@ goto processArgs :endHandleArgs call :processArgs %* -for %%i in ("%MAVEN_HOME%"\boot\plexus-classworlds-*) do set LAUNCHER_JAR="%%i" +set LAUNCHER_JAR="%MAVEN_HOME%\boot\*" set LAUNCHER_CLASS=org.codehaus.plexus.classworlds.launcher.Launcher +set MODULES_DIR="%MAVEN_HOME%\lib\modules" if "%MAVEN_MAIN_CLASS%"=="" @set MAVEN_MAIN_CLASS=org.apache.maven.cling.MavenCling Review Comment: 🐛 **`MAVEN_ARGS` guard deleted from `mvn.cmd` — regression for Windows users of sub-commands** The Linux `mvn` script retains the guard that prevents `MAVEN_ARGS` from being passed to sub-commands (`--up`, `--enc`, `--shell`): ```bash # MAVEN_ARGS is only passed for the default Maven build command (MavenCling), # not for sub-commands like --up, --enc, or --shell which have their own options. if [ "$MAVEN_MAIN_CLASS" = "org.apache.maven.cling.MavenCling" ]; then eval exec "$cmd" '$MAVEN_ARGS' '"$@"' else eval exec "$cmd" '"$@"' fi ``` But `mvn.cmd` now passes `%MAVEN_ARGS%` unconditionally to all invocations (line ~423). Windows users who set `MAVEN_ARGS` for their regular builds will have those args incorrectly forwarded to sub-commands, breaking them. Please restore the guard in `mvn.cmd` to keep parity with the Linux script. ########## impl/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java: ########## @@ -259,6 +273,101 @@ public ClassRealm createPluginRealm( return createRealm(getKey(plugin, false), RealmType.Plugin, parent, parentImports, foreignImports, artifacts); } + @Override + public ClassRealm createModularPluginRealm(Plugin plugin, ClassLoader parent, List<Artifact> artifacts) Review Comment: 💡 **`createModularPluginRealm` skips `callDelegates()` and `wireRealm()` — intentional?** The existing `createRealm()` path calls `callDelegates()` and `wireRealm()` before `populateRealm()`: ```java callDelegates(classRealm, type, parent, parentImports, foreignImports, constituents); wireRealm(classRealm, parentImports, foreignImports); populateRealm(classRealm, constituents); ``` The new `createModularPluginRealm()` only calls `populateRealm()` and `applyModuleAccessDescriptors()` — `callDelegates()` and `wireRealm()` are not called. This means `ClassRealmManagerDelegate` implementations will not be notified of modular plugin realms being created, and foreign imports won't be wired. If this is intentional (modular plugins don't need delegate-based wiring), please document why. If not, modular plugin realms may be missing extensions or type imports that delegates would have configured. ########## impl/maven-classworlds/src/main/java/org/codehaus/plexus/classworlds/realm/ClassRealm.java: ########## @@ -0,0 +1,556 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.plexus.classworlds.realm; + +/* + * Copyright 2001-2006 Codehaus Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.io.Closeable; +import java.io.IOException; +import java.io.PrintStream; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Collection; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.SortedSet; +import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import org.codehaus.plexus.classworlds.ClassWorld; +import org.codehaus.plexus.classworlds.strategy.Strategy; +import org.codehaus.plexus.classworlds.strategy.StrategyFactory; + +/** + * The class loading gateway. Each class realm has access to a base class loader, imports form zero or more other class + * loaders, an optional parent class loader and of course its own class path. When queried for a class/resource, a class + * realm will always query its base class loader first before it delegates to a pluggable strategy. The strategy in turn + * controls the order in which imported class loaders, the parent class loader and the realm itself are searched. The + * base class loader is assumed to be capable of loading of the bootstrap classes. + * + * @author <a href="mailto:[email protected]";>bob mcwhirter</a> + * @author Jason van Zyl + */ +public class ClassRealm extends URLClassLoader implements org.apache.maven.api.classworlds.ClassRealm { + + private ClassWorld world; + + private String id; + + private SortedSet<Entry> foreignImports; + + private SortedSet<Entry> parentImports; + + private Strategy strategy; + + private ClassLoader parentClassLoader; + + private ModuleLayer moduleLayer; + + private ModuleLayer.Controller moduleLayerController; + + private static final boolean IS_PARALLEL_CAPABLE = Closeable.class.isAssignableFrom(URLClassLoader.class); + + private final ConcurrentMap<String, Object> lockMap; + + /** + * Creates a new class realm. + * + * @param world The class world this realm belongs to, must not be <code>null</code>. + * @param id The identifier for this realm, must not be <code>null</code>. + * @param baseClassLoader The base class loader for this realm, may be <code>null</code> to use the bootstrap class + * loader. + */ + public ClassRealm(ClassWorld world, String id, ClassLoader baseClassLoader) { + super(new URL[0], baseClassLoader); + + this.world = world; + + this.id = id; + + foreignImports = new TreeSet<>(); + + strategy = StrategyFactory.getStrategy(this); + + lockMap = IS_PARALLEL_CAPABLE ? new ConcurrentHashMap<>() : null; + + if (IS_PARALLEL_CAPABLE) { + // We must call super.getClassLoadingLock at least once + // to avoid NPE in super.loadClass. + super.getClassLoadingLock(getClass().getName()); + } + } + + public String getId() { + return this.id; + } + + public ClassWorld getWorld() { + return this.world; + } + + /** + * Returns the underlying ClassLoader for this realm. + * <p> + * This method allows access to the actual ClassLoader implementation + * while maintaining API abstraction. Since ClassRealm extends URLClassLoader, + * this method returns {@code this}. + * </p> + * + * @return the underlying ClassLoader (this instance) + */ + public ClassLoader getClassLoader() { + return this; + } + + public void importFromParent(String packageName) { + if (parentImports == null) { + parentImports = new TreeSet<>(); + } + + parentImports.add(new Entry(null, packageName)); + } + + boolean isImportedFromParent(String name) { + if (parentImports != null && !parentImports.isEmpty()) { + for (Entry entry : parentImports) { + if (entry.matches(name)) { + return true; + } + } + + return false; + } + + return true; + } + + public void importFrom(String realmId, String packageName) throws NoSuchRealmException { + importFrom(getWorld().getRealm(realmId), packageName); + } + + public void importFrom(ClassLoader classLoader, String packageName) { + foreignImports.add(new Entry(classLoader, packageName)); + } + + public ClassLoader getImportClassLoader(String name) { + for (Entry entry : foreignImports) { + if (entry.matches(name)) { + return entry.getClassLoader(); + } + } + + return null; + } + + public Collection<ClassRealm> getImportRealms() { + Collection<ClassRealm> importRealms = new HashSet<>(); + + for (Entry entry : foreignImports) { + if (entry.getClassLoader() instanceof ClassRealm) { + importRealms.add((ClassRealm) entry.getClassLoader()); + } + } + + return importRealms; + } + + public Strategy getStrategy() { + return strategy; + } + + public void setParentClassLoader(ClassLoader parentClassLoader) { + this.parentClassLoader = parentClassLoader; + } + + public ClassLoader getParentClassLoader() { + return parentClassLoader; + } + + public void setParentRealm(ClassRealm realm) { + this.parentClassLoader = realm; + } + + public ClassRealm getParentRealm() { + return (parentClassLoader instanceof ClassRealm) ? (ClassRealm) parentClassLoader : null; + } + + // Implementation of the original method signature for backward compatibility + public ClassRealm createChildRealm(String id) throws DuplicateRealmException { + ClassRealm childRealm = getWorld().newRealm(id, (ClassLoader) null); + childRealm.setParentRealm(this); + return childRealm; + } + + public void addURL(URL url) { + String urlStr = url.toExternalForm(); + + if (urlStr.startsWith("jar:") && urlStr.endsWith("!/")) { + urlStr = urlStr.substring(4, urlStr.length() - 2); + + try { + url = new URL(urlStr); + } catch (MalformedURLException e) { + e.printStackTrace(); + } + } + + super.addURL(url); + } + + public void addExports(String moduleName, String packageName) { + world.addExports(moduleName, packageName, getUnnamedModule()); + } + + public void addOpens(String moduleName, String packageName) { + world.addOpens(moduleName, packageName, getUnnamedModule()); + } + + public void addReads(String moduleName) { + world.addReads(moduleName, getUnnamedModule()); + } + + /** + * Sets the ModuleLayer and Controller for this realm. + * Called when the plugin is loaded as a JPMS module. + */ + public void setModuleLayer(ModuleLayer moduleLayer, ModuleLayer.Controller controller) { + this.moduleLayer = moduleLayer; + this.moduleLayerController = controller; + } + + @Override + public ModuleLayer getModuleLayer() { + return moduleLayer; + } + + public ModuleLayer.Controller getModuleLayerController() { + return moduleLayerController; + } + + @Override + public boolean isModular() { + return moduleLayer != null; + } + + // ---------------------------------------------------------------------- + // We delegate to the Strategy here so that we can change the behavior + // of any existing ClassRealm. + // ---------------------------------------------------------------------- + + public Class<?> loadClass(String name) throws ClassNotFoundException { + return loadClass(name, false); + } + + protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (IS_PARALLEL_CAPABLE) { + return unsynchronizedLoadClass(name, resolve); + + } else { + synchronized (this) { + return unsynchronizedLoadClass(name, resolve); + } + } + } + + private Class<?> unsynchronizedLoadClass(String name, boolean resolve) throws ClassNotFoundException { + try { + // first, try loading bootstrap classes + return super.loadClass(name, resolve); + } catch (ClassNotFoundException e) { + // next, try loading via imports, self and parent as controlled by strategy + return strategy.loadClass(name); + } + } + + // overwrites + // https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#findClass(java.lang.String,java.lang.String) + // introduced in Java9 + protected Class<?> findClass(String moduleName, String name) { + if (moduleName != null) { + return null; + } + try { + return findClassInternal(name); + } catch (ClassNotFoundException e) { + try { + return strategy.getRealm().findClass(name); + } catch (ClassNotFoundException nestedException) { + return null; + } + } + } + + protected Class<?> findClass(String name) throws ClassNotFoundException { + /* + * NOTE: This gets only called from ClassLoader.loadClass(Class, boolean) while we try to check for bootstrap + * stuff. Don't scan our class path yet, loadClassFromSelf() will do this later when called by the strategy. + */ + throw new ClassNotFoundException(name); + } + + protected Class<?> findClassInternal(String name) throws ClassNotFoundException { + return super.findClass(name); + } + + public URL getResource(String name) { + URL resource = super.getResource(name); + return resource != null ? resource : strategy.getResource(name); + } + + public URL findResource(String name) { + return super.findResource(name); + } + + public Enumeration<URL> getResources(String name) throws IOException { + Collection<URL> resources = new LinkedHashSet<>(Collections.list(super.getResources(name))); + resources.addAll(Collections.list(strategy.getResources(name))); + return Collections.enumeration(resources); + } + + public Enumeration<URL> findResources(String name) throws IOException { + return super.findResources(name); + } + + // ---------------------------------------------------------------------------- + // Display methods + // ---------------------------------------------------------------------------- + + public void display() { + display(System.out); + } + + public void display(PrintStream out) { + out.println("-----------------------------------------------------"); + + for (ClassRealm cr = this; cr != null; cr = (ClassRealm) cr.getParentRealm()) { + out.println("realm = " + cr.getId()); + out.println("strategy = " + cr.getStrategy().getClass().getName()); + + showUrls(cr, out); + + out.println(); + } + + out.println("-----------------------------------------------------"); + } + + private static void showUrls(ClassRealm classRealm, PrintStream out) { + URL[] urls = classRealm.getURLs(); + + for (int i = 0; i < urls.length; i++) { + out.println("urls[" + i + "] = " + urls[i]); + } + + out.println("Number of foreign imports: " + classRealm.foreignImports.size()); + + for (Entry entry : classRealm.foreignImports) { + out.println("import: " + entry); + } + + if (classRealm.parentImports != null) { + out.println("Number of parent imports: " + classRealm.parentImports.size()); + + for (Entry entry : classRealm.parentImports) { + out.println("import: " + entry); + } + } + } + + public String toString() { + return "ClassRealm[" + getId() + ", parent: " + getParentClassLoader() + "]"; + } + + // --------------------------------------------------------------------------------------------- + // Search methods that can be ordered by strategies to load a class + // --------------------------------------------------------------------------------------------- + + public Class<?> loadClassFromImport(String name) { + ClassLoader importClassLoader = getImportClassLoader(name); + + if (importClassLoader != null) { + try { + return importClassLoader.loadClass(name); + } catch (ClassNotFoundException e) { + return null; + } + } + + return null; + } + + public Class<?> loadClassFromSelf(String name) { + synchronized (getClassRealmLoadingLock(name)) { + try { + Class<?> clazz = findLoadedClass(name); + + if (clazz == null) { + clazz = findClassInternal(name); + } + + return clazz; + } catch (ClassNotFoundException e) { + return null; + } + } + } + + private Object getClassRealmLoadingLock(String name) { + if (IS_PARALLEL_CAPABLE) { + return getClassLoadingLock(name); + } else { + return this; + } + } + + @Override + protected Object getClassLoadingLock(String name) { + if (IS_PARALLEL_CAPABLE) { + Object newLock = new Object(); + Object lock = lockMap.putIfAbsent(name, newLock); + return (lock == null) ? newLock : lock; + } + return this; + } + + public Class<?> loadClassFromParent(String name) { + ClassLoader parent = getParentClassLoader(); + + if (parent != null && isImportedFromParent(name)) { + try { + return parent.loadClass(name); + } catch (ClassNotFoundException e) { + return null; + } + } + + return null; + } + + // --------------------------------------------------------------------------------------------- + // Search methods that can be ordered by strategies to get a resource + // --------------------------------------------------------------------------------------------- + + public URL loadResourceFromImport(String name) { + ClassLoader importClassLoader = getImportClassLoader(name); + + if (importClassLoader != null) { + return importClassLoader.getResource(name); + } + + return null; + } + + public URL loadResourceFromSelf(String name) { + return findResource(name); + } + + public URL loadResourceFromParent(String name) { + ClassLoader parent = getParentClassLoader(); + + if (parent != null && isImportedFromParent(name)) { + return parent.getResource(name); + } else { + return null; + } + } + + // --------------------------------------------------------------------------------------------- + // Search methods that can be ordered by strategies to get resources + // --------------------------------------------------------------------------------------------- + + public Enumeration<URL> loadResourcesFromImport(String name) { + ClassLoader importClassLoader = getImportClassLoader(name); + + if (importClassLoader != null) { + try { + return importClassLoader.getResources(name); + } catch (IOException e) { + return null; + } + } + + return null; + } + + public Enumeration<URL> loadResourcesFromSelf(String name) { + try { + return findResources(name); + } catch (IOException e) { + return null; + } + } + + public Enumeration<URL> loadResourcesFromParent(String name) { + ClassLoader parent = getParentClassLoader(); + + if (parent != null && isImportedFromParent(name)) { + try { + return parent.getResources(name); + } catch (IOException e) { + // eat it + } + } + + return null; + } + + @Override + public void close() throws IOException { + if (moduleLayer != null) { + // defineModulesWithOneLoader uses a single shared ClassLoader for all modules in the layer. + // Close it once to release JAR file handles. + moduleLayer.modules().stream().findFirst().ifPresent(m -> { Review Comment: 💡 **Inconsistent handling of `defineModulesWithOneLoader` invariant in `close()`** `close()` uses `findFirst().ifPresent(...)` — silently doing nothing if the module layer is empty: ```java moduleLayer.modules().stream().findFirst().ifPresent(m -> { ... }); ``` But `getEffectiveClassLoader()` in `DefaultMavenPluginManager` uses `.findFirst().orElseThrow()` on the same invariant (that `defineModulesWithOneLoader` always produces at least one module). If the invariant truly holds, `close()` should throw on an empty layer too (you'd want to know about a corrupted realm). If empty layers are possible, `getEffectiveClassLoader()` should handle that case gracefully. Pick one and apply it consistently. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
