From 0bf9cb744cb97c19f743d5970992d6f401dd1688 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 16 Aug 2026 22:03:13 +0200 Subject: [PATCH 1/4] Allow a caller supplied factory to create the class realm Extracted from #144, where a transformer-backed realm needed ClassWorld to register an implementation it does not know about. Duplicate detection, registration and listener notification stay here so callers do not reimplement them. The factory runs before the id is known, so a realm whose id turns out to be taken is closed before the exception is thrown rather than leaked. --- .../plexus/classworlds/ClassWorld.java | 46 +++++++++++++- .../plexus/classworlds/ClassWorldTest.java | 60 +++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java b/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java index 55e77fac..3936c41f 100644 --- a/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java +++ b/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java @@ -24,7 +24,9 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Predicate; +import java.util.function.Supplier; import org.codehaus.plexus.classworlds.realm.ClassRealm; import org.codehaus.plexus.classworlds.realm.DuplicateRealmException; @@ -88,7 +90,49 @@ public synchronized ClassRealm newRealm(String id, ClassLoader classLoader, Pred } else { realm = new FilteredClassRealm(filter, this, id, classLoader); } - realms.put(id, realm); + + return register(realm); + } + + /** + * Adds a class realm obtained from a caller supplied factory, allowing realm implementations that this class + * does not know about. + *

+ * The factory is invoked before the id is known, so a realm whose id is already taken is closed again before + * {@link DuplicateRealmException} is thrown; a failure to close it is attached as a suppressed exception. + * + * @param factory the factory creating the realm, must not be null and must not return + * null + * @return the created class realm + * @throws DuplicateRealmException in case a realm with the id of the created realm does already exist + * @throws IllegalArgumentException if the created realm belongs to a different class world + * @since 2.13.0 + */ + public synchronized ClassRealm newRealm(Supplier factory) throws DuplicateRealmException { + Objects.requireNonNull(factory, "factory cannot be null"); + + ClassRealm realm = Objects.requireNonNull(factory.get(), "factory returned null realm"); + String id = Objects.requireNonNull(realm.getId(), "realm id cannot be null"); + + if (realm.getWorld() != this) { + throw new IllegalArgumentException("realm " + id + " belongs to a different class world"); + } + + if (realms.containsKey(id)) { + DuplicateRealmException duplicate = new DuplicateRealmException(this, id); + try { + realm.close(); + } catch (Exception e) { + duplicate.addSuppressed(e); + } + throw duplicate; + } + + return register(realm); + } + + private ClassRealm register(ClassRealm realm) { + realms.put(realm.getId(), realm); for (ClassWorldListener listener : listeners) { listener.realmCreated(realm); diff --git a/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java b/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java index c4bc7d1e..034ff2d1 100644 --- a/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java +++ b/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java @@ -15,11 +15,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; import java.util.Collection; import java.util.Enumeration; import java.util.function.Predicate; +import java.util.function.Supplier; import org.codehaus.plexus.classworlds.realm.ClassRealm; import org.codehaus.plexus.classworlds.realm.DuplicateRealmException; @@ -29,6 +31,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; @@ -178,6 +181,63 @@ void testNewRealmWithNullFilter() throws Exception { assertEquals("unfiltered", realm.getId()); } + @Test + void testNewRealmWithSupplier() throws Exception { + TestListener listener = new TestListener(); + world.addListener(listener); + + ClassRealm realm = world.newRealm(() -> new CustomClassRealm(world, "custom")); + + assertInstanceOf(CustomClassRealm.class, realm); + assertSame(realm, world.getRealm("custom")); + assertEquals(1, listener.realmCreatedCount); + } + + @Test + void testNewRealmWithSupplierDuplicate() throws Exception { + world.newRealm("custom"); + CustomClassRealm rejected = new CustomClassRealm(world, "custom"); + + DuplicateRealmException e = assertThrows(DuplicateRealmException.class, () -> world.newRealm(() -> rejected)); + + assertEquals("custom", e.getId()); + assertSame(world, e.getWorld()); + assertTrue(rejected.closed, "rejected realm should have been closed"); + } + + @Test + void testNewRealmWithSupplierFromOtherWorld() throws Exception { + ClassWorld otherWorld = new ClassWorld(); + ClassRealm foreign = otherWorld.newRealm("foreign"); + + assertThrows(IllegalArgumentException.class, () -> world.newRealm(() -> foreign)); + assertTrue(world.getRealms().isEmpty()); + } + + @Test + void testNewRealmWithNullSupplier() { + assertThrows(NullPointerException.class, () -> world.newRealm((Supplier) null)); + } + + @Test + void testNewRealmWithSupplierReturningNull() { + assertThrows(NullPointerException.class, () -> world.newRealm(() -> null)); + } + + private static class CustomClassRealm extends ClassRealm { + boolean closed; + + CustomClassRealm(ClassWorld world, String id) { + super(world, id, null); + } + + @Override + public void close() throws IOException { + closed = true; + super.close(); + } + } + @Test void testDisposeRealm() throws Exception { ClassRealm realm = world.newRealm("temp"); From 658959c291322550ed873e504ca4031a23086669 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Mon, 17 Aug 2026 16:11:43 +0200 Subject: [PATCH 2/4] Do not close the realm the factory handed back unregistered A factory is free to return the realm already registered under that id, for instance world.newRealm(() -> world.getClassRealm(id)) or anything that memoises. The duplicate path then closed that realm and left it in the world, so a failed call silently killed a live class loader: after it, getResource returned null and loadClass threw ClassNotFoundException. Only realms that are not the registered instance are closed now. The regression test gives the realm a jar of its own, since closing a realm with no URLs of its own has no observable effect. --- .../codehaus/plexus/classworlds/ClassWorld.java | 17 +++++++++++------ .../plexus/classworlds/ClassWorldTest.java | 13 +++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java b/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java index 3936c41f..63f72c99 100644 --- a/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java +++ b/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java @@ -99,7 +99,9 @@ public synchronized ClassRealm newRealm(String id, ClassLoader classLoader, Pred * does not know about. *

* The factory is invoked before the id is known, so a realm whose id is already taken is closed again before - * {@link DuplicateRealmException} is thrown; a failure to close it is attached as a suppressed exception. + * {@link DuplicateRealmException} is thrown; a failure to close it is attached as a suppressed exception. A + * factory that hands back the realm already registered under that id is the one case where nothing is closed, + * since closing it would leave a dead realm in this world. * * @param factory the factory creating the realm, must not be null and must not return * null @@ -118,12 +120,15 @@ public synchronized ClassRealm newRealm(Supplier factory) throws Dup throw new IllegalArgumentException("realm " + id + " belongs to a different class world"); } - if (realms.containsKey(id)) { + ClassRealm registered = realms.get(id); + if (registered != null) { DuplicateRealmException duplicate = new DuplicateRealmException(this, id); - try { - realm.close(); - } catch (Exception e) { - duplicate.addSuppressed(e); + if (registered != realm) { + try { + realm.close(); + } catch (Exception e) { + duplicate.addSuppressed(e); + } } throw duplicate; } diff --git a/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java b/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java index 034ff2d1..38b61d50 100644 --- a/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java +++ b/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java @@ -205,6 +205,19 @@ void testNewRealmWithSupplierDuplicate() throws Exception { assertTrue(rejected.closed, "rejected realm should have been closed"); } + @Test + void testNewRealmWithSupplierReturningTheRegisteredRealm() throws Exception { + ClassRealm registered = world.newRealm("custom"); + registered.addURL(TestUtil.getTestResourceUrl("a.jar")); + assertNotNull(registered.getResource("a.properties")); + + assertThrows(DuplicateRealmException.class, () -> world.newRealm(() -> world.getClassRealm("custom"))); + + assertSame(registered, world.getRealm("custom")); + assertNotNull(registered.getResource("a.properties"), "the registered realm must not have been closed"); + assertNotNull(registered.loadClass("a.A")); + } + @Test void testNewRealmWithSupplierFromOtherWorld() throws Exception { ClassWorld otherWorld = new ClassWorld(); From 94e8f82d7c176f58a4776327211bca3635e5e9d3 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Mon, 17 Aug 2026 16:16:26 +0200 Subject: [PATCH 3/4] Key the registry on the id that was checked register() called getId() a second time, so a subclass that overrides it could be checked under one key and stored under another. The caller now passes the id it validated. Also documents that the factory runs under the world's monitor and that a realm from another world is rejected without being closed, and stops the foreign-world test leaking its second world. --- .../org/codehaus/plexus/classworlds/ClassWorld.java | 11 +++++++---- .../codehaus/plexus/classworlds/ClassWorldTest.java | 10 ++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java b/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java index 63f72c99..2a356263 100644 --- a/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java +++ b/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java @@ -91,7 +91,7 @@ public synchronized ClassRealm newRealm(String id, ClassLoader classLoader, Pred realm = new FilteredClassRealm(filter, this, id, classLoader); } - return register(realm); + return register(id, realm); } /** @@ -103,6 +103,9 @@ public synchronized ClassRealm newRealm(String id, ClassLoader classLoader, Pred * factory that hands back the realm already registered under that id is the one case where nothing is closed, * since closing it would leave a dead realm in this world. * + * A realm built against another class world is rejected without being closed, since it may be live and + * registered over there. The factory runs while this world's monitor is held. + * * @param factory the factory creating the realm, must not be null and must not return * null * @return the created class realm @@ -133,11 +136,11 @@ public synchronized ClassRealm newRealm(Supplier factory) throws Dup throw duplicate; } - return register(realm); + return register(id, realm); } - private ClassRealm register(ClassRealm realm) { - realms.put(realm.getId(), realm); + private ClassRealm register(String id, ClassRealm realm) { + realms.put(id, realm); for (ClassWorldListener listener : listeners) { listener.realmCreated(realm); diff --git a/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java b/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java index 38b61d50..b4f6d060 100644 --- a/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java +++ b/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java @@ -220,11 +220,13 @@ void testNewRealmWithSupplierReturningTheRegisteredRealm() throws Exception { @Test void testNewRealmWithSupplierFromOtherWorld() throws Exception { - ClassWorld otherWorld = new ClassWorld(); - ClassRealm foreign = otherWorld.newRealm("foreign"); + try (ClassWorld otherWorld = new ClassWorld()) { + ClassRealm foreign = otherWorld.newRealm("foreign"); - assertThrows(IllegalArgumentException.class, () -> world.newRealm(() -> foreign)); - assertTrue(world.getRealms().isEmpty()); + assertThrows(IllegalArgumentException.class, () -> world.newRealm(() -> foreign)); + assertTrue(world.getRealms().isEmpty()); + assertNotNull(otherWorld.getClassRealm("foreign"), "the foreign realm must be left alone"); + } } @Test From 7f9521f5bd179cbcb3380df8d30e5970380f7f0a Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Mon, 17 Aug 2026 16:28:56 +0200 Subject: [PATCH 4/4] Take the id up front and only then call the factory Reshapes the new API from newRealm(Supplier) to createRealm(String, Function). The duplicate check now runs before the factory does, which is the invariant the id based overloads already have: a rejected call has no side effect, so there is no realm to close again and no way to close the one already registered. Not an overload of newRealm because a second two argument newRealm makes newRealm(id, null) ambiguous, which the library itself relies on in ClassRealm.createChildRealm and downstream callers do too. The factory result is checked against the requested id, since the registry key and the realm's own id would otherwise be free to disagree. --- .../plexus/classworlds/ClassWorld.java | 53 +++++++-------- .../plexus/classworlds/ClassWorldTest.java | 64 +++++++++---------- 2 files changed, 55 insertions(+), 62 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java b/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java index 2a356263..cd746b7f 100644 --- a/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java +++ b/src/main/java/org/codehaus/plexus/classworlds/ClassWorld.java @@ -25,8 +25,8 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.Function; import java.util.function.Predicate; -import java.util.function.Supplier; import org.codehaus.plexus.classworlds.realm.ClassRealm; import org.codehaus.plexus.classworlds.realm.DuplicateRealmException; @@ -95,45 +95,40 @@ public synchronized ClassRealm newRealm(String id, ClassLoader classLoader, Pred } /** - * Adds a class realm obtained from a caller supplied factory, allowing realm implementations that this class - * does not know about. + * Adds a class realm built by a caller supplied factory, allowing realm implementations that this class does + * not know about. *

- * The factory is invoked before the id is known, so a realm whose id is already taken is closed again before - * {@link DuplicateRealmException} is thrown; a failure to close it is attached as a suppressed exception. A - * factory that hands back the realm already registered under that id is the one case where nothing is closed, - * since closing it would leave a dead realm in this world. - * - * A realm built against another class world is rejected without being closed, since it may be live and - * registered over there. The factory runs while this world's monitor is held. + * The factory is handed the id and runs only once that id is known to be free, so a rejected call has no side + * effect and no realm has to be closed again. It runs while this world's monitor is held. + *

+ * This is not an overload of {@link #newRealm(String, ClassLoader)} because a second two argument + * newRealm would make the existing newRealm(id, null) calls ambiguous. * - * @param factory the factory creating the realm, must not be null and must not return - * null + * @param id The identifier for this realm, must not be null. + * @param factory the factory building the realm for that id, must not be null and must not + * return null * @return the created class realm - * @throws DuplicateRealmException in case a realm with the id of the created realm does already exist - * @throws IllegalArgumentException if the created realm belongs to a different class world + * @throws DuplicateRealmException in case a realm with the given id does already exist + * @throws IllegalArgumentException if the created realm belongs to a different class world or carries a + * different id * @since 2.13.0 */ - public synchronized ClassRealm newRealm(Supplier factory) throws DuplicateRealmException { + public synchronized ClassRealm createRealm(String id, Function factory) + throws DuplicateRealmException { + Objects.requireNonNull(id, "id cannot be null"); Objects.requireNonNull(factory, "factory cannot be null"); - ClassRealm realm = Objects.requireNonNull(factory.get(), "factory returned null realm"); - String id = Objects.requireNonNull(realm.getId(), "realm id cannot be null"); + if (realms.containsKey(id)) { + throw new DuplicateRealmException(this, id); + } + + ClassRealm realm = Objects.requireNonNull(factory.apply(id), "factory returned null realm"); if (realm.getWorld() != this) { throw new IllegalArgumentException("realm " + id + " belongs to a different class world"); } - - ClassRealm registered = realms.get(id); - if (registered != null) { - DuplicateRealmException duplicate = new DuplicateRealmException(this, id); - if (registered != realm) { - try { - realm.close(); - } catch (Exception e) { - duplicate.addSuppressed(e); - } - } - throw duplicate; + if (!id.equals(realm.getId())) { + throw new IllegalArgumentException("realm for id " + id + " carries id " + realm.getId()); } return register(id, realm); diff --git a/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java b/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java index b4f6d060..2d4b3489 100644 --- a/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java +++ b/src/test/java/org/codehaus/plexus/classworlds/ClassWorldTest.java @@ -15,13 +15,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; import java.util.Collection; import java.util.Enumeration; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; import java.util.function.Predicate; -import java.util.function.Supplier; import org.codehaus.plexus.classworlds.realm.ClassRealm; import org.codehaus.plexus.classworlds.realm.DuplicateRealmException; @@ -31,6 +31,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -182,11 +183,11 @@ void testNewRealmWithNullFilter() throws Exception { } @Test - void testNewRealmWithSupplier() throws Exception { + void testCreateRealmWithFactory() throws Exception { TestListener listener = new TestListener(); world.addListener(listener); - ClassRealm realm = world.newRealm(() -> new CustomClassRealm(world, "custom")); + ClassRealm realm = world.createRealm("custom", id -> new CustomClassRealm(world, id)); assertInstanceOf(CustomClassRealm.class, realm); assertSame(realm, world.getRealm("custom")); @@ -194,63 +195,60 @@ void testNewRealmWithSupplier() throws Exception { } @Test - void testNewRealmWithSupplierDuplicate() throws Exception { - world.newRealm("custom"); - CustomClassRealm rejected = new CustomClassRealm(world, "custom"); - - DuplicateRealmException e = assertThrows(DuplicateRealmException.class, () -> world.newRealm(() -> rejected)); - - assertEquals("custom", e.getId()); - assertSame(world, e.getWorld()); - assertTrue(rejected.closed, "rejected realm should have been closed"); - } - - @Test - void testNewRealmWithSupplierReturningTheRegisteredRealm() throws Exception { + void testCreateRealmWithFactoryDuplicateDoesNotRunTheFactory() throws Exception { ClassRealm registered = world.newRealm("custom"); registered.addURL(TestUtil.getTestResourceUrl("a.jar")); - assertNotNull(registered.getResource("a.properties")); - assertThrows(DuplicateRealmException.class, () -> world.newRealm(() -> world.getClassRealm("custom"))); + AtomicBoolean invoked = new AtomicBoolean(); + DuplicateRealmException e = assertThrows( + DuplicateRealmException.class, + () -> world.createRealm("custom", id -> { + invoked.set(true); + return new CustomClassRealm(world, id); + })); + assertEquals("custom", e.getId()); + assertSame(world, e.getWorld()); + assertFalse(invoked.get(), "the factory must not run for an id that is already taken"); assertSame(registered, world.getRealm("custom")); assertNotNull(registered.getResource("a.properties"), "the registered realm must not have been closed"); assertNotNull(registered.loadClass("a.A")); } @Test - void testNewRealmWithSupplierFromOtherWorld() throws Exception { + void testCreateRealmWithFactoryFromOtherWorld() throws Exception { try (ClassWorld otherWorld = new ClassWorld()) { ClassRealm foreign = otherWorld.newRealm("foreign"); - assertThrows(IllegalArgumentException.class, () -> world.newRealm(() -> foreign)); + assertThrows(IllegalArgumentException.class, () -> world.createRealm("foreign", id -> foreign)); assertTrue(world.getRealms().isEmpty()); assertNotNull(otherWorld.getClassRealm("foreign"), "the foreign realm must be left alone"); } } @Test - void testNewRealmWithNullSupplier() { - assertThrows(NullPointerException.class, () -> world.newRealm((Supplier) null)); + void testCreateRealmWithFactoryReturningForeignId() { + assertThrows( + IllegalArgumentException.class, + () -> world.createRealm("custom", id -> new CustomClassRealm(world, "somethingElse"))); + assertTrue(world.getRealms().isEmpty()); } @Test - void testNewRealmWithSupplierReturningNull() { - assertThrows(NullPointerException.class, () -> world.newRealm(() -> null)); + void testCreateRealmWithNullFactory() { + assertThrows( + NullPointerException.class, () -> world.createRealm("custom", (Function) null)); } - private static class CustomClassRealm extends ClassRealm { - boolean closed; + @Test + void testCreateRealmWithFactoryReturningNull() { + assertThrows(NullPointerException.class, () -> world.createRealm("custom", id -> null)); + } + private static class CustomClassRealm extends ClassRealm { CustomClassRealm(ClassWorld world, String id) { super(world, id, null); } - - @Override - public void close() throws IOException { - closed = true; - super.close(); - } } @Test