From ad8d7d78e35b006cbf3fdb55459809808bb397c0 Mon Sep 17 00:00:00 2001 From: Sebastian Hartte Date: Sat, 16 Dec 2023 12:50:18 +0100 Subject: [PATCH 1/3] Allow ModuleClassLoader to delegate class-loading to parent for unit testing without class-loader isolation. --- src/main/java/cpw/mods/cl/ModuleClassLoader.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/cpw/mods/cl/ModuleClassLoader.java b/src/main/java/cpw/mods/cl/ModuleClassLoader.java index 6a47ed2..1de1398 100644 --- a/src/main/java/cpw/mods/cl/ModuleClassLoader.java +++ b/src/main/java/cpw/mods/cl/ModuleClassLoader.java @@ -1,6 +1,7 @@ package cpw.mods.cl; import cpw.mods.util.LambdaExceptionUtils; +import org.jetbrains.annotations.Nullable; import java.io.IOException; import java.io.InputStream; @@ -64,10 +65,15 @@ private static void bindToLayer(ModuleClassLoader classLoader, ModuleLayer layer private final Map resolvedRoots; private final Map packageLookup; private final Map parentLoaders; - private ClassLoader fallbackClassLoader = ClassLoader.getPlatformClassLoader(); + private ClassLoader fallbackClassLoader; public ModuleClassLoader(final String name, final Configuration configuration, final List parentLayers) { - super(name, null); + this(name, configuration, parentLayers, null); + } + + public ModuleClassLoader(final String name, final Configuration configuration, final List parentLayers, @Nullable ClassLoader parentLoader) { + super(name, parentLoader); + this.fallbackClassLoader = Objects.requireNonNullElse(parentLoader, ClassLoader.getPlatformClassLoader()); this.configuration = configuration; this.packageLookup = new HashMap<>(); this.resolvedRoots = configuration.modules().stream() From 9c36d038bfb3b7a34b06cb774ee87d43bc5aa758 Mon Sep 17 00:00:00 2001 From: Sebastian Hartte Date: Fri, 12 Jan 2024 20:31:56 +0100 Subject: [PATCH 2/3] Document the side-effects of using the constructor. --- src/main/java/cpw/mods/cl/ModuleClassLoader.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/main/java/cpw/mods/cl/ModuleClassLoader.java b/src/main/java/cpw/mods/cl/ModuleClassLoader.java index 1de1398..1fbb8a8 100644 --- a/src/main/java/cpw/mods/cl/ModuleClassLoader.java +++ b/src/main/java/cpw/mods/cl/ModuleClassLoader.java @@ -2,6 +2,7 @@ import cpw.mods.util.LambdaExceptionUtils; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; import java.io.IOException; import java.io.InputStream; @@ -71,6 +72,20 @@ public ModuleClassLoader(final String name, final Configuration configuration, f this(name, configuration, parentLayers, null); } + /** + * This constructor allows setting the parent {@linkplain ClassLoader classloader}. Use this with caution since + * it will allow loading of classes from the classpath directly if the {@linkplain ClassLoader#getSystemClassLoader() system classloader} + * is reachable from the given parent classloader. + *

+ * Generally classes that are in packages covered by reachable modules are preferably loaded from these modules. + * If a class-path entry is not shadowed by a module, specifying a parent class-loader may lead to those + * classes now being loadable instead of throwing a {@link ClassNotFoundException}. + *

+ * This relaxed classloader isolation is used in unit-testing, where testing libraries are loaded on the + * system class-loader outside our control (by the Gradle test runner). We must not reload these classes + * inside the module layers again, otherwise tests throw incompatible exceptions or may not be found at all. + */ + @VisibleForTesting public ModuleClassLoader(final String name, final Configuration configuration, final List parentLayers, @Nullable ClassLoader parentLoader) { super(name, parentLoader); this.fallbackClassLoader = Objects.requireNonNullElse(parentLoader, ClassLoader.getPlatformClassLoader()); From fcd39b9447e742c321b21ecd60d7c3a2e08adfb3 Mon Sep 17 00:00:00 2001 From: Sebastian Hartte Date: Sat, 13 Jan 2024 19:06:46 +0100 Subject: [PATCH 3/3] Correct Javadoc tags --- src/main/java/cpw/mods/cl/ModuleClassLoader.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/cpw/mods/cl/ModuleClassLoader.java b/src/main/java/cpw/mods/cl/ModuleClassLoader.java index 1fbb8a8..c7c248c 100644 --- a/src/main/java/cpw/mods/cl/ModuleClassLoader.java +++ b/src/main/java/cpw/mods/cl/ModuleClassLoader.java @@ -76,11 +76,11 @@ public ModuleClassLoader(final String name, final Configuration configuration, f * This constructor allows setting the parent {@linkplain ClassLoader classloader}. Use this with caution since * it will allow loading of classes from the classpath directly if the {@linkplain ClassLoader#getSystemClassLoader() system classloader} * is reachable from the given parent classloader. - *

+ *

* Generally classes that are in packages covered by reachable modules are preferably loaded from these modules. * If a class-path entry is not shadowed by a module, specifying a parent class-loader may lead to those * classes now being loadable instead of throwing a {@link ClassNotFoundException}. - *

+ *

* This relaxed classloader isolation is used in unit-testing, where testing libraries are loaded on the * system class-loader outside our control (by the Gradle test runner). We must not reload these classes * inside the module layers again, otherwise tests throw incompatible exceptions or may not be found at all.