From 05c67edc386f095d83a73d3b9ff7f5791b49dd96 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Mon, 5 Oct 2026 16:39:26 +0300 Subject: [PATCH 1/2] Unregister the published customizer class on release() release() dropped the engine but left the class of its customizer registered. The REST, CREST and SSH configurations put an ExpandoMetaClass on that class, which on Groovy 2.4.21 keeps the class, its GroovyClassLoader with every compiled script, and the released configuration alive: one loader leaked per facade dispose or configuration change. getGroovyScriptEngine() now keeps the customizer class of the published engine, and release() removes it with InvokerHelper.removeClass after the release closure has run. Fixes #154 --- .../scriptedcommon/ScriptedConfiguration.java | 17 +++++- .../ScriptedConfigurationTest.java | 52 ++++++++++++++----- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index 117fb7a63..6f10156e1 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -666,6 +666,12 @@ public void release() { clone.call(); releaseClosure = null; } + if (null != publishedCustomizerClass) { + // Drops the metaclass createCustomizerScript() may have put on the class, + // which would otherwise keep this engine's loader alive. + InvokerHelper.removeClass(publishedCustomizerClass); + publishedCustomizerClass = null; + } groovyScriptEngine = null; propertyBag.clear(); loggerCache.clear(); @@ -791,6 +797,9 @@ protected Log getLogger(final Class clazz) { */ private GroovyScriptEngine customizingGroovyScriptEngine = null; + /** The customizer class of the published engine; {@link #release()} unregisters it. Guarded by {@code this}. */ + private Class publishedCustomizerClass = null; + /** * Synchronised for the whole initialisation, not double-checked: the * customizer script runs against the half-initialised engine (it may call @@ -814,12 +823,14 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { new GroovyScriptEngine(getRoots(compilerConfiguration, loader), loader); // If the customizer fails, the engine is dropped and the next call retries. + final Class customizerClass; customizingGroovyScriptEngine = engine; try { - initializeCustomizer(); + customizerClass = initializeCustomizer(); } finally { customizingGroovyScriptEngine = null; } + publishedCustomizerClass = customizerClass; groovyScriptEngine = engine; } return groovyScriptEngine; @@ -830,8 +841,9 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { * synchronized to avoid deadlock. The customizer runs while this * configuration's monitor is held: it must not wait for another thread * that calls getGroovyScriptEngine(), evaluate() or loadScript(). + * Returns the customizer class, or null if there is none. */ - private void initializeCustomizer() { + private Class initializeCustomizer() { Class customizerClass = null; try { customizerClass = getCustomizerClass(); @@ -841,6 +853,7 @@ private void initializeCustomizer() { binding.setVariable(LOGGER, getLogger(customizerClass)); createCustomizerScript(customizerClass, binding).run(); } + return customizerClass; } catch (Throwable t) { if (null != customizerClass) { // The retry compiles a new class; unregister this one, together with any diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfigurationTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfigurationTest.java index 404918062..b4586192f 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfigurationTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfigurationTest.java @@ -125,20 +125,7 @@ public void testFailedCustomizerClassIsUnregistered() { throw new IllegalStateException("customizer failed"); }; final AtomicReference customizerClass = new AtomicReference<>(); - // Registers a metaclass on the customizer class, as the REST, CREST and SSH - // configurations do; a registered metaclass keeps the class's loader alive. - configuration = new ScriptedConfiguration() { - @Override - protected Script createCustomizerScript(Class clazz, Binding binding) { - customizerClass.set(clazz); - ExpandoMetaClass metaClass = new ExpandoMetaClass(clazz, false, true); - metaClass.initialize(); - GroovySystem.getMetaClassRegistry().setMetaClass(clazz, metaClass); - return super.createCustomizerScript(clazz, binding); - } - }; - configuration.setScriptRoots(new String[] { scriptRoot.getAbsolutePath() }); - configuration.setCustomizerScriptFileName("Customizer.groovy"); + configuration = registeringMetaClassOnCustomizer(customizerClass); try { configuration.getGroovyScriptEngine(); @@ -151,6 +138,43 @@ protected Script createCustomizerScript(Class clazz, Binding binding) { assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()).isNull(); } + @Test + public void testReleaseUnregistersThePublishedCustomizerClass() { + customizer = () -> { + }; + final AtomicReference customizerClass = new AtomicReference<>(); + configuration = registeringMetaClassOnCustomizer(customizerClass); + + assertThat(configuration.getGroovyScriptEngine()).isNotNull(); + assertThat(customizerClass.get()).isNotNull(); + assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()) + .as("metaclass of the published customizer before release()").isNotNull(); + + configuration.release(); + assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()).isNull(); + } + + /** + * A configuration that registers a metaclass on its customizer class, as the REST, CREST and + * SSH configurations do; a registered metaclass keeps the class's loader alive. + */ + private ScriptedConfiguration registeringMetaClassOnCustomizer( + final AtomicReference customizerClass) { + ScriptedConfiguration result = new ScriptedConfiguration() { + @Override + protected Script createCustomizerScript(Class clazz, Binding binding) { + customizerClass.set(clazz); + ExpandoMetaClass metaClass = new ExpandoMetaClass(clazz, false, true); + metaClass.initialize(); + GroovySystem.getMetaClassRegistry().setMetaClass(clazz, metaClass); + return super.createCustomizerScript(clazz, binding); + } + }; + result.setScriptRoots(new String[] { scriptRoot.getAbsolutePath() }); + result.setCustomizerScriptFileName("Customizer.groovy"); + return result; + } + @Test public void testRetriedCRESTCustomizerDropsTheReleaseClosureOfTheFailedAttempt() throws Exception { final String test = ScriptedConfigurationTest.class.getName(); From ff4d1dbace4a3f06ed77d5f251f696ab6e7abcd9 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Tue, 6 Oct 2026 15:30:25 +0300 Subject: [PATCH 2/2] Unregister the customizer class even when the release closure throws release() ran the release closure with no try/finally around it, so a release {} that throws skipped InvokerHelper.removeClass. The caller, OperationalContext.dispose(), logs the failure and never calls release() again, which left the loader pinned on that path. The unregistration and groovyScriptEngine = null now run in a finally block. New tests pin the closure that throws, the super.release() call in the real CREST and REST release() overrides, and the clearing of publishedCustomizerClass. --- .../scriptedcommon/ScriptedConfiguration.java | 29 ++++--- .../ScriptedConfigurationTest.java | 84 +++++++++++++++++++ 2 files changed, 101 insertions(+), 12 deletions(-) diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index 6f10156e1..dd7b6a89f 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -660,19 +660,24 @@ public ConcurrentMap getPropertyBag() { public void release() { synchronized (this) { Closure c = getReleaseClosure(); - if (null != c) { - Closure clone = c.rehydrate(this, this, this); - clone.setResolveStrategy(Closure.DELEGATE_FIRST); - clone.call(); - releaseClosure = null; - } - if (null != publishedCustomizerClass) { - // Drops the metaclass createCustomizerScript() may have put on the class, - // which would otherwise keep this engine's loader alive. - InvokerHelper.removeClass(publishedCustomizerClass); - publishedCustomizerClass = null; + try { + if (null != c) { + Closure clone = c.rehydrate(this, this, this); + clone.setResolveStrategy(Closure.DELEGATE_FIRST); + clone.call(); + releaseClosure = null; + } + } finally { + // Runs even when the release closure throws: the caller logs the failure and + // never calls release() again. + if (null != publishedCustomizerClass) { + // Drops the metaclass createCustomizerScript() may have put on the class, + // which would otherwise keep this engine's loader alive. + InvokerHelper.removeClass(publishedCustomizerClass); + publishedCustomizerClass = null; + } + groovyScriptEngine = null; } - groovyScriptEngine = null; propertyBag.clear(); loggerCache.clear(); logger.ok("Shared state ScriptedConfiguration is successfully released"); diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfigurationTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfigurationTest.java index b4586192f..8aec6e04e 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfigurationTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfigurationTest.java @@ -30,12 +30,15 @@ import java.util.concurrent.atomic.AtomicReference; import org.codehaus.groovy.reflection.ClassInfo; +import org.codehaus.groovy.runtime.InvokerHelper; import org.forgerock.openicf.connectors.scriptedcrest.ScriptedCRESTConfiguration; +import org.forgerock.openicf.connectors.scriptedrest.ScriptedRESTConfiguration; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import groovy.lang.Binding; +import groovy.lang.Closure; import groovy.lang.ExpandoMetaClass; import groovy.lang.GroovySystem; import groovy.lang.Script; @@ -152,6 +155,87 @@ public void testReleaseUnregistersThePublishedCustomizerClass() { configuration.release(); assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()).isNull(); + + // A released configuration must not touch a class it no longer publishes. + ExpandoMetaClass again = new ExpandoMetaClass(customizerClass.get(), false, true); + again.initialize(); + GroovySystem.getMetaClassRegistry().setMetaClass(customizerClass.get(), again); + try { + configuration.release(); + assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()) + .as("metaclass registered after the first release()").isNotNull(); + } finally { + InvokerHelper.removeClass(customizerClass.get()); + } + } + + @Test + public void testThrowingReleaseClosureStillUnregistersTheCustomizerClass() { + customizer = () -> { + }; + final AtomicReference customizerClass = new AtomicReference<>(); + configuration = registeringMetaClassOnCustomizer(customizerClass); + assertThat(configuration.getGroovyScriptEngine()).isNotNull(); + assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()) + .as("metaclass of the published customizer before release()").isNotNull(); + configuration.setReleaseClosure(new Closure(this) { + public Void doCall() { + throw new IllegalStateException("release failed"); + } + }); + + try { + configuration.release(); + fail("The release closure failure must reach the caller"); + } catch (IllegalStateException e) { + assertThat(e).hasMessage("release failed"); + } + assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()).isNull(); + } + + @Test + public void testCRESTReleaseUnregistersTheCustomizeMetaClass() throws Exception { + final AtomicReference customizerClass = new AtomicReference<>(); + assertReleaseUnregistersTheCustomizeMetaClass(new ScriptedCRESTConfiguration() { + @Override + protected Script createCustomizerScript(Class clazz, Binding binding) { + customizerClass.set(clazz); + return super.createCustomizerScript(clazz, binding); + } + }, customizerClass); + } + + @Test + public void testRESTReleaseUnregistersTheCustomizeMetaClass() throws Exception { + final AtomicReference customizerClass = new AtomicReference<>(); + assertReleaseUnregistersTheCustomizeMetaClass(new ScriptedRESTConfiguration() { + @Override + protected Script createCustomizerScript(Class clazz, Binding binding) { + customizerClass.set(clazz); + return super.createCustomizerScript(clazz, binding); + } + }, customizerClass); + } + + /** + * Runs a real {@code customize {}} DSL, which registers a metaclass on the customizer class, + * and checks that the subclass's own {@code release()} unregisters it. + */ + private void assertReleaseUnregistersTheCustomizeMetaClass(ScriptedConfiguration subject, + AtomicReference customizerClass) throws Exception { + Files.write(new File(scriptRoot, "Customizer.groovy").toPath(), + "customize {\n}\n".getBytes(StandardCharsets.UTF_8)); + configuration = subject; + configuration.setScriptRoots(new String[] { scriptRoot.getAbsolutePath() }); + configuration.setCustomizerScriptFileName("Customizer.groovy"); + + assertThat(configuration.getGroovyScriptEngine()).isNotNull(); + assertThat(customizerClass.get()).isNotNull(); + assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()) + .as("metaclass registered by customize {} before release()").isNotNull(); + + configuration.release(); + assertThat(ClassInfo.getClassInfo(customizerClass.get()).getStrongMetaClass()).isNull(); } /**