From 0a68cc20b3fc45733dbc65905507fb4976e45f97 Mon Sep 17 00:00:00 2001 From: Paul King Date: Thu, 20 Aug 2026 14:33:43 +1000 Subject: [PATCH 1/2] =?UTF-8?q?GROOVY-12281:=20soft=20GroovyClassValue=20m?= =?UTF-8?q?ode=20=E2=80=94=20soft=20values=20with=20resurrection=20(opt-in?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds -Dgroovy.use.classvalue=soft: java.lang.ClassValue keeps the per-Class fast path for all keys, but each association stores a bootstrap-loaded SoftReference to the ClassInfo, cutting the only strong chain from immortal platform classes to Groovy's loader (JDK-8136353 / GROOVY-12142). Correctness pieces, addressing the review findings on the v1 assessment: - Resurrection: a weak-key/weak-value side map is the identity authority; computeValue re-associates a still-reachable value instead of replacing it, so a fresh instance can only exist once no guard can observe the old one (kills the split-brain sequence; classic call-site version guards stay sound, so legacy groovy-callsite jars on the classpath are unaffected). - Dirty roots: ClassInfos carrying non-reconstructible state (installed MetaClass, per-instance MetaClasses, registry-written MOP arrays) are strong-rooted Groovy-side; the MOP-write hook also enforces the registry-rooting invariant (non-empty DGM arrays => never collectible) by construction. ClassInfo.remove() unroots, keeping hard-detach undeploy semantics. - Per-Class indy domain continuity: in soft mode SwitchPoint domains are adopted from a weak-Class-keyed map, so a successor ClassInfo shares its predecessor's domain and mutations deterministically retire guards that captured only the SwitchPoint (POJO direct dispatch); domain reclaim moves from ClassInfo-death to Class-death. Indy-only; default mode unchanged. Tests: GroovyClassValueSoftTest (unit); ClassInfoSoftModeTest/Probe (child JVM, deterministic forced clearing: resurrection identity+version, DGM rooting, EMC and per-instance survival, true collection + fresh dispatch, classic CallSiteArray soundness across clear + EMC change, predecessor SwitchPoint retirement); ClassInfoSoftModeStressTest/Probe (child JVM, real GC clearing: 128MB heap + SoftRefLRUPolicyMSPerMB=0, concurrent indy + classic dispatch racing the collector through ~130 EMC generations — 66M dispatches, 33/36 platform-receiver ClassInfos collected and recreated mid-run, zero invariant violations). build-logic forwards -Pgroovy.use.classvalue to test JVMs so whole suites can soak a non-default mode; the full core suite passes under soft mode (16,800 tests). --- .../groovy/org.apache.groovy-tested.gradle | 7 + .../groovy/runtime/indy/IndyInvalidation.java | 71 ++++ .../groovy/reflection/CachedClass.java | 4 + .../codehaus/groovy/reflection/ClassInfo.java | 110 ++++++- .../reflection/GroovyClassValueFactory.java | 21 +- .../reflection/GroovyClassValueSoft.java | 149 +++++++++ .../reflection/ClassInfoSoftModeProbe.groovy | 203 ++++++++++++ .../ClassInfoSoftModeStressProbe.groovy | 309 ++++++++++++++++++ .../ClassInfoSoftModeStressTest.groovy | 56 ++++ .../reflection/ClassInfoSoftModeTest.groovy | 49 +++ .../GroovyClassValueSoftTest.groovy | 135 ++++++++ 11 files changed, 1107 insertions(+), 7 deletions(-) create mode 100644 src/main/java/org/codehaus/groovy/reflection/GroovyClassValueSoft.java create mode 100644 src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeProbe.groovy create mode 100644 src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeStressProbe.groovy create mode 100644 src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeStressTest.groovy create mode 100644 src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeTest.groovy create mode 100644 src/test/groovy/org/codehaus/groovy/reflection/GroovyClassValueSoftTest.groovy diff --git a/build-logic/src/main/groovy/org.apache.groovy-tested.gradle b/build-logic/src/main/groovy/org.apache.groovy-tested.gradle index 08a6db6c947..2606bdbd760 100644 --- a/build-logic/src/main/groovy/org.apache.groovy-tested.gradle +++ b/build-logic/src/main/groovy/org.apache.groovy-tested.gradle @@ -92,6 +92,13 @@ tasks.withType(Test).configureEach { if (closurePack) { systemProperty 'groovy.target.closure.pack', closurePack } + // Forward the global ClassValue mode (GROOVY-12142/GROOVY-12281: false = weak-key map, + // soft = soft values with resurrection) to the test JVM so whole suites can be run + // against a non-default mode, e.g. -Pgroovy.use.classvalue=soft. Set only when present. + def classValueMode = findProperty('groovy.use.classvalue') ?: System.getProperty('groovy.use.classvalue') + if (classValueMode) { + systemProperty 'groovy.use.classvalue', classValueMode + } def testdb = System.properties['groovy.testdb.props'] if (testdb) { systemProperty 'groovy.testdb.props', testdb diff --git a/src/main/java/org/apache/groovy/runtime/indy/IndyInvalidation.java b/src/main/java/org/apache/groovy/runtime/indy/IndyInvalidation.java index 6879789c724..d77c7332ee8 100644 --- a/src/main/java/org/apache/groovy/runtime/indy/IndyInvalidation.java +++ b/src/main/java/org/apache/groovy/runtime/indy/IndyInvalidation.java @@ -24,6 +24,7 @@ import groovy.lang.MetaClassImpl; import groovy.lang.MetaClassRegistryChangeEvent; import org.apache.groovy.util.SystemUtil; +import org.apache.groovy.util.concurrent.ManagedIdentityConcurrentMap; import org.codehaus.groovy.reflection.ClassInfo; import org.codehaus.groovy.runtime.NullObject; import org.codehaus.groovy.util.ManagedReference; @@ -135,6 +136,76 @@ public void finalizeReference() { } } + // ------------------------------------------------------------------------- + // Soft ClassValue mode: per-Class domain continuity (GROOVY-12281) + // ------------------------------------------------------------------------- + + /** + * Per-Class domain map used only when {@code groovy.use.classvalue=soft}. + * In soft mode a ClassInfo instance is replaceable — it can be soft-collected + * and a successor created for the same class — while POJO direct-dispatch + * guards capture only the SwitchPoint's internal invoker, never the + * ClassInfo. Keying the domain by the {@code Class} (weak identity keys, + * strong domain values) makes domain identity survive instance turnover: + * the successor adopts its predecessor's domain, so an invalidation applied + * through the successor deterministically retires guards linked under the + * predecessor, with no dependency on lazy reference-queue processing. + *

+ * The map holds no classes alive (weak keys) and no foreign loaders + * (values reference nothing owner-ward); entries for dead classes are + * purged by the map, and their domains retired by the Class-keyed reclaim + * ({@link #anchorClassDomainToClass}) so the live-SwitchPoint registry + * cannot accumulate zombies. + */ + private static final ManagedIdentityConcurrentMap, SwitchPointInvalidator> CLASS_DOMAINS = + new ManagedIdentityConcurrentMap<>(); + + /** + * Returns the canonical domain for {@code type}, seeding the per-Class map + * with {@code candidate} on first touch. Soft-mode only (callers gate). + * + * @param type the class whose domain is requested (must not be {@code null}) + * @param candidate the caller-owned domain to install if none is mapped yet + * @return the canonical per-Class domain + */ + public static SwitchPointInvalidator classDomainFor(final Class type, final SwitchPointInvalidator candidate) { + return CLASS_DOMAINS.getOrPut(type, candidate); + } + + /** + * Anchors a reclaim reference keyed to the {@code Class} rather than a + * ClassInfo instance: soft mode retires a domain only when the class dies + * (no receiver can reach an installed guard afterwards), because ClassInfo + * instances are replaceable and their successors adopt the same domain. + * + * @param owner the class owning the domain (must not be {@code null}) + * @param domain the class-level domain (must not be {@code null}) + */ + public static void anchorClassDomainToClass(final Class owner, final SwitchPointInvalidator domain) { + domain.setReclaimAnchor(new ClassDomainReclaim(owner, domain)); + } + + /** + * Weak reference to a domain's owning {@code Class} whose collection + * retires the domain (soft ClassValue mode). Mirrors {@link DomainReclaim} + * with the owner lifetime moved from the replaceable ClassInfo instance to + * the stable per-Class identity. + */ + private static final class ClassDomainReclaim extends ManagedReference> { + private final SwitchPointInvalidator domain; + + ClassDomainReclaim(final Class owner, final SwitchPointInvalidator domain) { + super(ReferenceBundle.getWeakBundle(), owner); + this.domain = domain; + } + + @Override + public void finalizeReference() { + domain.invalidate(); + super.finalizeReference(); + } + } + // ------------------------------------------------------------------------- // Width: exact class // ------------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/reflection/CachedClass.java b/src/main/java/org/codehaus/groovy/reflection/CachedClass.java index d5d37f3297d..7c6fcb33948 100644 --- a/src/main/java/org/codehaus/groovy/reflection/CachedClass.java +++ b/src/main/java/org/codehaus/groovy/reflection/CachedClass.java @@ -572,6 +572,8 @@ private void updateSetNewMopMethods(List arr) { } else classInfo.newMetaMethods = classInfo.dgmMetaMethods; + // GROOVY-12281 soft mode: MOP-array writes make the ClassInfo non-reclaimable + classInfo.updateReclaimability(); } /** @@ -622,6 +624,8 @@ private void updateAddNewMopMethods(List arr) { res.addAll(Arrays.asList(classInfo.newMetaMethods)); res.addAll(arr); classInfo.newMetaMethods = res.toArray(MetaMethod.EMPTY_ARRAY); + // GROOVY-12281 soft mode: MOP-array writes make the ClassInfo non-reclaimable + classInfo.updateReclaimability(); var theClass = classInfo.getCachedClass().getTheClass(); if (theClass == Closure.class || theClass == Class.class) { ClosureMetaClass.resetCachedMetaClasses(); diff --git a/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java b/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java index 39a802bacab..0d40ff51b5b 100644 --- a/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java +++ b/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java @@ -61,6 +61,8 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; /** @@ -110,6 +112,29 @@ public class ClassInfo implements Finalizable { private static final ManagedConcurrentLinkedQueue modifiedExpandos = new ManagedConcurrentLinkedQueue(weakBundle); + /** + * Whether {@link #globalClassValue} stores its values softly with + * resurrection ({@code -Dgroovy.use.classvalue=soft}, GROOVY-12281 + * investigation prototype). Soft mode needs two cooperating pieces here: + * strong roots for non-reconstructible state ({@link #nonReclaimableRoots}) + * and per-Class indy domain continuity (see {@link #indyDomain()}). + */ + private static final boolean SOFT_CLASS_VALUES = GroovyClassValueFactory.isSoftMode(); + + /** + * Soft-mode strong roots for ClassInfos whose state could not be + * reconstructed if the instance were soft-collected and later recreated: + * an installed class-level MetaClass ({@link #setStrongMetaClass}), + * per-instance MetaClasses, or registry-written DGM/extension method + * arrays ({@link CachedClass#setNewMopMethods}/{@code addNewMopMethods}). + * The set itself is Groovy-loaded, so it contributes no foreign-loader + * pinning: everything in it dies with Groovy's own loader, which is the + * lifetime every ClassInfo has today under the default strong ClassValue. + * {@code null} unless soft mode is active. + */ + private static final Set nonReclaimableRoots = + SOFT_CLASS_VALUES ? ConcurrentHashMap.newKeySet() : null; + private static final GroovyClassValue globalClassValue = GroovyClassValueFactory.createGroovyClassValue(new ComputeValue(){ /** * Creates a new {@code ClassInfo} for the given class type. @@ -189,6 +214,34 @@ private void bumpGenerationLocal() { invalidateIndySwitchPoint(); } + /** + * Resolves the SwitchPoint domain all indy guard operations act on. + *

+ * Default mode: the instance-owned {@link #indySwitchPointDomain}, whose + * lifetime equals this ClassInfo's — sound because the strong ClassValue + * keeps the instance alive as long as its class. + *

+ * Soft mode (GROOVY-12281): the per-Class domain from + * {@link IndyInvalidation#classDomainFor}, seeded with the local domain on + * first touch. Domain identity then survives ClassInfo recreation: a + * successor instance adopts its predecessor's domain, so a MetaClass + * change applied through the successor deterministically retires guards + * that were linked under the predecessor (POJO direct-dispatch guards + * capture only the SwitchPoint invoker, never this instance, so they can + * outlive it — the lazy {@code DomainReclaim} pump alone would leave an + * unbounded stale-guard window). When the class itself has been collected + * no receiver can reach any guard, so the local domain suffices. + */ + private SwitchPointInvalidator indyDomain() { + if (SOFT_CLASS_VALUES) { + Class type = getTheClass(); + if (type != null) { + return IndyInvalidation.classDomainFor(type, indySwitchPointDomain); + } + } + return indySwitchPointDomain; + } + /** * Returns the SwitchPoint for monomorphic indy MOP guards on this class: * the current generation of the class-level domain, which covers both the @@ -199,11 +252,22 @@ private void bumpGenerationLocal() { */ @Internal public SwitchPoint getIndySwitchPoint() { + SwitchPointInvalidator domain = indyDomain(); if (!indyDomainAnchored) { - IndyInvalidation.anchorClassDomain(this, indySwitchPointDomain); + if (SOFT_CLASS_VALUES) { + // Soft mode retires domains when the Class dies, not when a + // ClassInfo instance does: instances are replaceable (their + // successor adopts the same domain), classes are not. + Class type = getTheClass(); + if (type != null) { + IndyInvalidation.anchorClassDomainToClass(type, domain); + } + } else { + IndyInvalidation.anchorClassDomain(this, indySwitchPointDomain); + } indyDomainAnchored = true; } - return indySwitchPointDomain.getSwitchPoint(); + return domain.getSwitchPoint(); } /** @@ -215,7 +279,7 @@ public SwitchPoint getIndySwitchPoint() { */ @Internal public void invalidateIndySwitchPoint() { - indySwitchPointDomain.invalidate(); + indyDomain().invalidate(); } /** @@ -227,7 +291,7 @@ public void invalidateIndySwitchPoint() { */ @Internal public void collectLiveIndySwitchPoints(final List out) { - SwitchPoint live = indySwitchPointDomain.detachLive(); + SwitchPoint live = indyDomain().detachLive(); if (live != null) { out.add(live); } @@ -242,7 +306,7 @@ public void collectLiveIndySwitchPoints(final List out) { */ @Internal public SwitchPoint detachLiveIndySwitchPoint() { - return indySwitchPointDomain.detachLive(); + return indyDomain().detachLive(); } /** @@ -326,6 +390,15 @@ public static ClassInfo getClassInfo (Class cls) { * from cache */ public static void remove(Class cls) { + if (SOFT_CLASS_VALUES && globalClassValue instanceof GroovyClassValueSoft) { + // A hard detach must also unroot the detached instance, or an + // undeployed class's ClassInfo would stay pinned by its own + // non-reclaimable root — the exact leak remove() exists to break. + ClassInfo current = ((GroovyClassValueSoft) globalClassValue).getIfPresent(cls); + if (current != null) { + nonReclaimableRoots.remove(current); + } + } globalClassValue.remove(cls); } @@ -400,6 +473,31 @@ public void setStrongMetaClass(MetaClass answer) { } replaceWeakMetaClassRef(null); + updateReclaimability(); + } + + /** + * Soft-mode bookkeeping (GROOVY-12281): keeps this ClassInfo strongly + * rooted while it carries state that could not be reconstructed after + * soft collection — an installed class-level MetaClass, per-instance + * MetaClasses, or registry-written DGM/extension method arrays. The DGM + * condition also enforces the registry-rooting invariant by construction + * rather than by audit: any instance holding non-empty MOP arrays is + * non-collectible, so a recreated instance never needs to rebuild them. + * Removal is conservative: lingering weak entries in the per-instance map + * merely delay unrooting, which is the safe direction (today's default + * roots every ClassInfo forever). No-op unless soft mode is active. + */ + void updateReclaimability() { + if (!SOFT_CLASS_VALUES) return; + if (strongMetaClass != null + || (perInstanceMetaClassMap != null && !perInstanceMetaClassMap.isEmpty()) + || dgmMetaMethods.length != 0 + || newMetaMethods.length != 0) { + nonReclaimableRoots.add(this); + } else { + nonReclaimableRoots.remove(this); + } } /** @@ -431,6 +529,7 @@ public void setWeakMetaClass(MetaClass answer) { newRef = new ManagedReference (softBundle,answer); } replaceWeakMetaClassRef(newRef); + updateReclaimability(); } private void replaceWeakMetaClassRef(ManagedReference newRef) { @@ -688,6 +787,7 @@ public void setPerInstanceMetaClass(Object obj, MetaClass metaClass) { perInstanceMetaClassMap.remove(obj); } } + updateReclaimability(); } /** diff --git a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java index 879c3773bb6..b12a725376a 100644 --- a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java +++ b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java @@ -31,10 +31,27 @@ class GroovyClassValueFactory { * {@code -Dgroovy.use.classvalue=false} at JVM startup to use a weak-key * map instead; the default remains ClassValue for its per-Class fast path. */ - private static final boolean USE_CLASSVALUE = Boolean.parseBoolean(SystemUtil.getSystemPropertySafe("groovy.use.classvalue", "true")); + private static final String CLASSVALUE_MODE = SystemUtil.getSystemPropertySafe("groovy.use.classvalue", "true"); + + /** + * GROOVY-12281: whether values are stored behind + * {@link java.lang.ref.SoftReference}s with resurrection semantics + * ({@code -Dgroovy.use.classvalue=soft}). Soft mode needs cooperation from + * {@link ClassInfo} (strong roots for non-reconstructible state, per-Class + * indy domain continuity), which is why it is exposed package-wide rather + * than kept local to {@link #createGroovyClassValue}. + */ + static boolean isSoftMode() { + return "soft".equalsIgnoreCase(CLASSVALUE_MODE); + } public static GroovyClassValue createGroovyClassValue(ComputeValue computeValue) { - return (USE_CLASSVALUE) + // GROOVY-12281: "soft" keeps ClassValue for all keys but holds values softly with + // resurrection, so immortal keys hold no strong chain to the value's loader. + if (isSoftMode()) { + return new GroovyClassValueSoft<>(computeValue); + } + return Boolean.parseBoolean(CLASSVALUE_MODE) ? new GroovyClassValueJava7<>(computeValue) : new GroovyClassValueMapBased<>(computeValue); } diff --git a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueSoft.java b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueSoft.java new file mode 100644 index 00000000000..a179924ec20 --- /dev/null +++ b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueSoft.java @@ -0,0 +1,149 @@ +/* + * 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.groovy.reflection; + +import org.apache.groovy.util.concurrent.ConcurrentReferenceHashMap; +import org.apache.groovy.util.concurrent.ConcurrentReferenceHashMap.Option; +import org.apache.groovy.util.concurrent.ConcurrentReferenceHashMap.ReferenceType; + +import java.lang.ref.SoftReference; +import java.util.EnumSet; + +/** + * GROOVY-12281 investigation prototype ({@code -Dgroovy.use.classvalue=soft}): + * keeps {@code java.lang.ClassValue}'s per-{@code Class} fast path but stores + * each value behind a {@link SoftReference}, so an association on an immortal + * platform class (for example {@code String}) no longer holds a strong chain + * to the value's class loader (JDK-8136353 / GROOVY-12142). The stored wrapper + * is a bootstrap-loaded {@code java.lang.ref.SoftReference}, and the per-Class + * entry references the defining {@code ClassValue} only weakly (via its + * {@code Version}; the map key is a bootstrap {@code Identity} object), so the + * only path from an immortal key to the Groovy-loaded value is softly + * reachable and is cleared under memory pressure once nothing else keeps the + * Groovy island alive. + *

+ * Correctness relies on resurrection: a weak-key/weak-value side map + * is the identity authority. {@code computeValue} consults it first, so when + * the {@code ClassValue}'s soft reference has been cleared but the value is + * still reachable anywhere — for example captured by a linked call site — the + * same instance is re-associated rather than a fresh one created. A fresh + * instance can only be created once the old one is weakly unreachable, at + * which point no guard, cache or call site can still observe the old + * instance, so "two live generations of one association" cannot arise. + *

+ * {@link #remove(Class)} stays a hard detach (the identity entry is + * purged too), preserving the documented undeploy semantics of + * {@link ClassInfo#remove(Class)}: the next {@code get} creates a fresh value + * even if the old one is still reachable somewhere. + * + * @param the value type + */ +class GroovyClassValueSoft implements GroovyClassValue { + + private final ComputeValue computeValue; + + /** + * Identity authority: weak identity keys (no key class is pinned by this + * map) and weak values (the map keeps nothing alive by itself; it only + * remembers a value for as long as something else does). + */ + private final ConcurrentReferenceHashMap, T> canonical = + new ConcurrentReferenceHashMap<>(ReferenceType.WEAK, ReferenceType.WEAK, + EnumSet.of(Option.IDENTITY_COMPARISONS)); + + /** + * Striped locks for the canonical-miss path. Creation must be mutually + * exclusive per key: without it two racing threads could each create and + * leak a distinct instance through the uncached fallback in + * {@link #get(Class)}, breaking the identity guarantee the side map + * exists to provide. (The map's own {@code putIfAbsent} cannot be used + * for this: on an entry whose collected value has not yet been purged it + * reports success without storing the replacement.) + */ + private final Object[] creationLocks = new Object[64]; + + private final ClassValue> store = new ClassValue>() { + @Override + protected SoftReference computeValue(final Class type) { + return new SoftReference<>(canonical(type)); + } + }; + + GroovyClassValueSoft(final ComputeValue computeValue) { + this.computeValue = computeValue; + for (int i = 0; i < creationLocks.length; i++) { + creationLocks[i] = new Object(); + } + } + + @Override + public T get(final Class type) { + T value = store.get(type).get(); + if (value != null) return value; + // The soft reference was cleared: drop the memoized wrapper and + // recompute once — resurrection returns the canonical instance when + // it is still alive (GROOVY-12280 remove-and-recompute pattern). + store.remove(type); + value = store.get(type).get(); + if (value != null) return value; + // Pathological memory pressure cleared the fresh wrapper before we + // could dereference it: serve the canonical instance uncached; a + // later get() re-tries the cache. Bounded, and identity-safe because + // the side map, not the ClassValue, is the identity authority. + return canonical(type); + } + + @Override + public void remove(final Class type) { + canonical.remove(type); + store.remove(type); + } + + /** + * Returns the current canonical value without creating one — used by + * detach paths that must act on the existing instance (for example + * un-rooting it) but must not resurrect or create anything as a side + * effect. + * + * @param type the key class + * @return the live canonical value, or {@code null} if none + */ + T getIfPresent(final Class type) { + return canonical.get(type); + } + + /** + * Returns the canonical value for {@code type}: the still-live existing + * instance when there is one, otherwise a freshly computed instance + * published under the key's creation lock. + */ + private T canonical(final Class type) { + T existing = canonical.get(type); + if (existing != null) return existing; + Object lock = creationLocks[System.identityHashCode(type) & (creationLocks.length - 1)]; + synchronized (lock) { + existing = canonical.get(type); + if (existing != null) return existing; + T fresh = computeValue.computeValue(type); + // Unconditional put: replaces a stale (value-collected) entry. + canonical.put(type, fresh); + return fresh; + } + } +} diff --git a/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeProbe.groovy b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeProbe.groovy new file mode 100644 index 00000000000..7b9b41abe06 --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeProbe.groovy @@ -0,0 +1,203 @@ +/* + * 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.groovy.reflection + +import groovy.lang.GroovySystem + +import java.lang.invoke.SwitchPoint +import java.lang.ref.SoftReference +import java.lang.ref.WeakReference +import java.util.zip.Adler32 +import java.util.zip.CRC32 +import java.util.zip.Inflater + +/** + * GROOVY-12281 child-process probe for {@code -Dgroovy.use.classvalue=soft}: + * exercises the global {@link ClassInfo#globalClassValue} in soft mode. + * Invoked from {@link ClassInfoSoftModeTest}; prints {@code OK} on success. + *

+ * Forced clears simulate GC clearing of the memoized SoftReference. A manual + * {@code clear()} is a conservative superset of GC behavior (a collector never + * clears a reference to a strongly reachable object), so passing here implies + * correctness under any GC schedule. + */ +final class ClassInfoSoftModeProbe { + + static class IndyHost { + def ping() { 'pong' } + } + + /** Clears the global store's memoized SoftReference for {@code type}. */ + private static void clearSoft(Class type) { + def gcvField = ClassInfo.getDeclaredField('globalClassValue') + gcvField.accessible = true + def gcv = gcvField.get(null) + assert gcv instanceof GroovyClassValueSoft : "probe requires -Dgroovy.use.classvalue=soft, found ${gcv.getClass().simpleName}" + def storeField = GroovyClassValueSoft.getDeclaredField('store') + storeField.accessible = true + ClassValue store = storeField.get(gcv) + ((SoftReference) store.get(type)).clear() + } + + private static boolean awaitCollected(WeakReference ref) { + for (int i = 0; i < 100 && ref.get() != null; i++) { + System.gc() + byte[][] pressure = new byte[64][] + for (int j = 0; j < pressure.length; j++) { + pressure[j] = new byte[1 << 16] + } + Thread.sleep(10) + } + return ref.get() == null + } + + static void main(String[] args) { + resurrectionPreservesIdentityAndVersion() + dgmTargetClassInfoIsRooted() + strongMetaClassSurvivesClear() + perInstanceMetaClassSurvivesClear() + pristineClassInfoIsCollectedAndRecreatedWorking() + classicCallSiteStaysSoundAcrossClearAndRelinksOnChange() + indyDomainContinuityAcrossRecreation() + println 'OK' + } + + /** + * The split-brain check: a ClassInfo captured by any holder must be + * returned as-is after the ClassValue's soft reference clears, with its + * version untouched, so captured version guards stay sound. + */ + private static void resurrectionPreservesIdentityAndVersion() { + ClassInfo before = ClassInfo.getClassInfo(String) // strong local ref: "captured by a call site" + int version = before.version + clearSoft(String) + ClassInfo after = ClassInfo.getClassInfo(String) + assert after.is(before) : 'live ClassInfo must be resurrected, not replaced' + assert after.version == version : 'resurrection must not disturb the version guard stamp' + } + + /** + * The enforced E3 invariant: a ClassInfo holding registry-written DGM/extension + * method arrays is strongly rooted, so it can never be soft-collected and a + * recreated instance never needs to rebuild those arrays. + */ + private static void dgmTargetClassInfoIsRooted() { + def weak = new WeakReference(ClassInfo.getClassInfo(String)) + clearSoft(String) + System.gc() + assert weak.get() != null : 'DGM-target ClassInfo must be rooted (non-reclaimable)' + assert ClassInfo.getClassInfo(String).is(weak.get()) + assert 'abc'.reverse() == 'cba' : 'String DGM dispatch intact' + } + + /** User metaclass customizations must survive value clearing (dirty root). */ + private static void strongMetaClassSurvivesClear() { + CRC32.metaClass.twiddle = { -> 42 } + try { + def weak = new WeakReference(ClassInfo.getClassInfo(CRC32)) + clearSoft(CRC32) + System.gc() + assert weak.get() != null : 'ClassInfo with installed MetaClass must be rooted' + assert new CRC32().twiddle() == 42 : 'EMC customization must survive the clear' + } finally { + GroovySystem.metaClassRegistry.removeMetaClass(CRC32) + } + } + + /** Per-instance metaclasses are equally non-reconstructible state. */ + private static void perInstanceMetaClassSurvivesClear() { + def receiver = new Adler32() + receiver.metaClass.spin = { -> 7 } + try { + def weak = new WeakReference(ClassInfo.getClassInfo(Adler32)) + clearSoft(Adler32) + System.gc() + assert weak.get() != null : 'ClassInfo with per-instance MetaClass must be rooted' + assert receiver.spin() == 7 : 'per-instance customization must survive the clear' + } finally { + receiver.metaClass = null + } + } + + /** + * The unpinning payoff: a pristine ClassInfo (no user MetaClass state, no + * DGM arrays) really is collected once cleared, and dispatch afterwards + * works against a fresh instance — hierarchy DGM methods included. + */ + private static void pristineClassInfoIsCollectedAndRecreatedWorking() { + def weak = new WeakReference(ClassInfo.getClassInfo(Inflater)) + clearSoft(Inflater) + assert awaitCollected(weak) : 'pristine ClassInfo should be collectable once cleared' + def inflater = new Inflater() + try { + assert inflater.with { 'fresh' } == 'fresh' : 'Object-hierarchy DGM dispatch on the fresh ClassInfo' + } finally { + inflater.end() + } + } + + /** + * Legacy classic call sites (groovy-callsite on the classpath, e.g. from + * jars compiled by older Groovy) capture the ClassInfo instance and its + * version at link time. Resurrection keeps that capture sound across a + * clear, and a later MetaClass change must still be observed via the + * version guard on the same instance. Without resurrection the stale site + * would answer 'cba' forever. + */ + private static void classicCallSiteStaysSoundAcrossClearAndRelinksOnChange() { + // groovy-callsite is runtime-only for core (as for legacy-compiled jars + // in the wild), so drive it reflectively + def csaClass = Class.forName('org.codehaus.groovy.runtime.callsite.CallSiteArray') + def csa = csaClass.getConstructor(Class, String[]).newInstance(ClassInfoSoftModeProbe, ['reverse'] as String[]) + def noparam = csaClass.NOPARAM + assert csa.array[0].call('abc', noparam) == 'cba' + // the linked site (csa.array[0] after the first call) now captures ClassInfo(String)+version + clearSoft(String) + System.gc() + assert csa.array[0].call('abc', noparam) == 'cba' : 'linked site stays correct across the clear' + String.metaClass.reverse = { -> 'emc' } + try { + assert csa.array[0].call('abc', noparam) == 'emc' : 'version guard on the resurrected instance must observe the change' + } finally { + GroovySystem.metaClassRegistry.removeMetaClass(String) + } + assert csa.array[0].call('abc', noparam) == 'cba' + } + + /** + * The E4 re-homing check: indy guards capture only the SwitchPoint, so a + * guard can outlive its ClassInfo. Domain identity is keyed by Class, so a + * mutation applied through the successor ClassInfo must deterministically + * retire a SwitchPoint handed out by the collected predecessor. Without + * re-homing, the successor would invalidate a fresh domain and this + * SwitchPoint would survive, leaving the stale guard installed until the + * lazy reference-queue pump. + */ + private static void indyDomainContinuityAcrossRecreation() { + ClassInfo before = ClassInfo.getClassInfo(IndyHost) + SwitchPoint sp = before.indySwitchPoint + def weak = new WeakReference(before) + before = null + clearSoft(IndyHost) + assert awaitCollected(weak) : 'unrooted ClassInfo with a linked domain should be collectable' + ClassInfo successor = ClassInfo.getClassInfo(IndyHost) + successor.incVersion() + assert sp.hasBeenInvalidated() : "predecessor's SwitchPoint must be retired by a mutation through the successor" + } +} diff --git a/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeStressProbe.groovy b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeStressProbe.groovy new file mode 100644 index 00000000000..d9f67aed0c9 --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeStressProbe.groovy @@ -0,0 +1,309 @@ +/* + * 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.groovy.reflection + +import groovy.lang.GroovySystem + +import java.lang.ref.WeakReference +import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.CountDownLatch +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicLong + +/** + * GROOVY-12281 stress probe for {@code -Dgroovy.use.classvalue=soft} under + * real GC clearing (unlike {@link ClassInfoSoftModeProbe}, which + * clears deterministically): run with a small heap and + * {@code -XX:SoftRefLRUPolicyMSPerMB=0} so every collection clears whatever + * soft references are not strongly protected, while dispatch, MetaClass + * mutation and class churn race the collector. Invoked from + * {@link ClassInfoSoftModeStressTest}; prints {@code OK} on success. + * + * Invariants asserted: + *

+ */ +final class ClassInfoSoftModeStressProbe { + + private static final int CHURN_CLASSES = 100 + private static final long STRESS_MILLIS = 20_000 + private static final long QUIESCE_TIMEOUT_MILLIS = 10_000 + + private static final ConcurrentLinkedQueue errors = new ConcurrentLinkedQueue<>() + private static final AtomicBoolean mutating = new AtomicBoolean(true) + private static final AtomicInteger installedGen = new AtomicInteger() + private static volatile int finalGen = -1 + private static final AtomicLong dispatches = new AtomicLong() + + static void main(String[] args) { + def gcv = ClassInfo.getDeclaredField('globalClassValue').tap { accessible = true }.get(null) + assert gcv instanceof GroovyClassValueSoft : + "probe requires -Dgroovy.use.classvalue=soft, found ${gcv.getClass().simpleName}" + + // POGO group: Groovy-defined classes. Their ClassInfos are self-pinned + // while the class lives (POGO instances hold their MetaClass and the + // generated $staticMetaClass field holds it from the Class), so they + // stress link/relink correctness, not collection. + def gcl = new GroovyClassLoader(ClassInfoSoftModeStressProbe.classLoader) + def churnReceivers = new Object[CHURN_CLASSES] + for (int i = 0; i < CHURN_CLASSES; i++) { + def cls = gcl.parseClass("class Churn$i { int id() { $i } }", "Churn${i}.groovy") + churnReceivers[i] = cls.getDeclaredConstructor().newInstance() + } + + // collection group: JAVA receivers — no MetaClass instance field, no + // $staticMetaClass, and POJO indy guards capture only Class objects — + // so their pristine ClassInfos are exactly the population soft mode + // is allowed to reap and recreate under pressure (the ticket's + // platform-receiver scenario). + Object[] javaReceivers = [ + new ArrayList(), new LinkedList(), new HashMap(), new TreeMap(), + new HashSet(), new TreeSet(), new ArrayDeque(), new PriorityQueue(), + new Stack(), new Vector(), new Hashtable(), new StringBuilder('x'), + new StringBuffer('y'), new Random(42), new StringJoiner(','), + new IdentityHashMap(), new WeakHashMap(), new LinkedHashMap(), + new LinkedHashSet(), new java.util.concurrent.ConcurrentHashMap(), + new java.util.concurrent.ConcurrentLinkedDeque(), new java.util.concurrent.CopyOnWriteArrayList(), + new java.util.concurrent.atomic.AtomicInteger(7), new java.util.concurrent.atomic.AtomicLong(9), + new java.util.zip.CRC32(), new java.util.zip.Adler32(), + new java.text.StringCharacterIterator('z'), new java.awt.Point(1, 2), + new java.awt.Dimension(3, 4), new java.awt.Rectangle(5, 6), + new java.io.ByteArrayOutputStream(), new java.io.StringWriter(), + new java.net.InetSocketAddress(80), new java.util.Formatter(), + new java.util.EventObject('e'), new java.util.SimpleTimeZone(0, 'UTC'), + ] as Object[] + long[] javaExpected = new long[javaReceivers.length] + def javaInfos = new WeakReference[javaReceivers.length] + for (int i = 0; i < javaReceivers.length; i++) { + javaExpected[i] = javaReceivers[i].hashCode() // receivers are never mutated + javaInfos[i] = new WeakReference(ClassInfo.getClassInfo(javaReceivers[i].getClass())) + } + + // mutated group: an EMC on a platform class — the ticket's exact + // shape — whose ClassInfo the dirty root must keep alive throughout + installGen(1) + ClassInfo mutatedInfoAtStart = ClassInfo.getClassInfo(BitSet) + def mutatedInfoRef = new WeakReference(mutatedInfoAtStart) + + List workers = [] + def startGate = new CountDownLatch(1) + def done = new CountDownLatch(6) + + // 2 indy dispatchers over the POGO classes (one megamorphic site) + 2.times { t -> + workers << worker(startGate, done, "pogo-$t") { + for (int i = 0; ; i++) { + int k = (i * 31 + t) % CHURN_CLASSES + def r = churnReceivers[k] + int got = r.id() + if (got != k) { + throw new IllegalStateException("POGO dispatch returned $got for Churn$k") + } + dispatches.incrementAndGet() + if (shouldStop()) return + } + } + } + + // 1 indy dispatcher over the Java receivers, racing collection and + // recreation of their ClassInfos + workers << worker(startGate, done, 'java-churn') { + for (int i = 0; ; i++) { + int k = (i * 17 + 3) % javaReceivers.length + long got = javaReceivers[k].hashCode() + if (got != javaExpected[k]) { + throw new IllegalStateException("Java dispatch returned $got for ${javaReceivers[k].getClass().name}, expected ${javaExpected[k]}") + } + dispatches.incrementAndGet() + if (shouldStop()) return + } + } + + // 2 indy dispatchers on the mutated platform class + 2.times { t -> + workers << worker(startGate, done, "mutated-$t") { + def receiver = new BitSet() + int lastSeen = 0 + while (true) { + int gen = receiver.probe() + checkGen(gen, lastSeen, "mutated-$t") + lastSeen = Math.max(lastSeen, gen) + dispatches.incrementAndGet() + if (shouldStop() && lastSeen == finalGen) return + if (shouldStop() && quiesceExpired()) { + throw new IllegalStateException("mutated-$t stuck at gen $lastSeen, final is $finalGen") + } + } + } + } + + // 1 classic dispatcher on the same class, via CallSiteArray as a + // legacy-compiled jar would dispatch (groovy-callsite is runtime-only) + workers << worker(startGate, done, 'classic') { + def csaClass = Class.forName('org.codehaus.groovy.runtime.callsite.CallSiteArray') + def csa = csaClass.getConstructor(Class, String[]).newInstance(ClassInfoSoftModeStressProbe, ['probe'] as String[]) + def noparam = csaClass.NOPARAM + def receiver = new BitSet() + int lastSeen = 0 + while (true) { + int gen = (int) csa.array[0].call(receiver, noparam) + checkGen(gen, lastSeen, 'classic') + lastSeen = Math.max(lastSeen, gen) + dispatches.incrementAndGet() + if (shouldStop() && lastSeen == finalGen) return + if (shouldStop() && quiesceExpired()) { + throw new IllegalStateException("classic stuck at gen $lastSeen, final is $finalGen") + } + } + } + + // mutator: replace the EMC method with an increasing generation + def mutator = new Thread({ + startGate.await() + long end = System.currentTimeMillis() + STRESS_MILLIS + while (System.currentTimeMillis() < end) { + installGen(installedGen.get() + 1) + Thread.sleep(150) + } + int last = installedGen.get() + 1 + installGen(last) + finalGen = last // publish, then let dispatchers converge + quiesceStart = System.currentTimeMillis() + mutating.set(false) + }, 'mutator') + mutator.daemon = true + + // pressure: keep the collector busy; with SoftRefLRUPolicyMSPerMB=0 + // every GC clears all unprotected soft references + def pressure = new Thread({ + startGate.await() + def hog = new byte[24][] + int i = 0, gcTick = 0 + while (!done.await(0, java.util.concurrent.TimeUnit.MILLISECONDS)) { + try { + hog[i++ % hog.length] = new byte[256 << 10] + } catch (OutOfMemoryError e) { + hog = new byte[24][] // release and back off + } + if (++gcTick % 64 == 0) { + System.gc() + Thread.sleep(20) + } + } + }, 'pressure') + pressure.daemon = true + + workers*.start(); mutator.start(); pressure.start() + startGate.countDown() + + long deadline = System.currentTimeMillis() + STRESS_MILLIS + QUIESCE_TIMEOUT_MILLIS + 10_000 + for (t in workers) { + long left = deadline - System.currentTimeMillis() + t.join(Math.max(1, left)) + if (t.alive) errors.add("worker ${t.name} did not finish") + } + + // dirty root must have preserved the mutated class's identity + ClassInfo mutatedInfoAtEnd = ClassInfo.getClassInfo(BitSet) + if (!mutatedInfoAtEnd.is(mutatedInfoAtStart)) { + errors.add('dirty-rooted ClassInfo(BitSet) was replaced during the run') + } + if (mutatedInfoRef.get() == null) { + errors.add('dirty-rooted ClassInfo(BitSet) was collected during the run') + } + + // the stress must have been live: Java-receiver ClassInfos really collected + int collected = javaInfos.count { it.get() == null } + // and dispatch still works on every class afterwards + for (int k = 0; k < CHURN_CLASSES; k++) { + if (churnReceivers[k].id() != k) { + errors.add("post-run dispatch broken for Churn$k") + } + } + for (int k = 0; k < javaReceivers.length; k++) { + if (javaReceivers[k].hashCode() != javaExpected[k]) { + errors.add("post-run dispatch broken for ${javaReceivers[k].getClass().name}") + } + } + + GroovySystem.metaClassRegistry.removeMetaClass(BitSet) + + println "dispatches=${dispatches.get()} generations=${installedGen.get()} javaInfosCollected=$collected/${javaInfos.length}" + if (collected == 0) { + errors.add('no Java-receiver ClassInfo was ever collected — the stress did not exercise real GC clearing') + } + if (errors.isEmpty()) { + println 'OK' + } else { + errors.each { println "ERROR: $it" } + System.exit(1) + } + } + + private static volatile long quiesceStart = Long.MAX_VALUE + + private static boolean shouldStop() { + !mutating.get() + } + + private static boolean quiesceExpired() { + System.currentTimeMillis() - quiesceStart > QUIESCE_TIMEOUT_MILLIS + } + + private static void checkGen(int gen, int lastSeen, String who) { + if (gen < 1 || gen > installedGen.get()) { + throw new IllegalStateException("$who observed gen $gen outside installed range 1..${installedGen.get()}") + } + if (gen < lastSeen) { + throw new IllegalStateException("$who observed gen $gen after already seeing $lastSeen") + } + } + + private static void installGen(int gen) { + installedGen.set(gen) + BitSet.metaClass.probe = { -> gen } + } + + private static Thread worker(CountDownLatch startGate, CountDownLatch done, String name, Closure body) { + def t = new Thread({ + startGate.await() + try { + body() + } catch (Throwable e) { + errors.add("$name: ${e.getClass().simpleName}: ${e.message}") + } finally { + done.countDown() + } + }, name) + t.daemon = true + t + } +} diff --git a/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeStressTest.groovy b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeStressTest.groovy new file mode 100644 index 00000000000..3d9918834f4 --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeStressTest.groovy @@ -0,0 +1,56 @@ +/* + * 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.groovy.reflection + +import org.junit.jupiter.api.Test + +import java.util.concurrent.TimeUnit + +import static org.junit.jupiter.api.Assertions.assertEquals +import static org.junit.jupiter.api.Assertions.assertTrue + +/** + * GROOVY-12281: races soft-mode ClassInfo management against real GC + * clearing in a child JVM (small heap, {@code SoftRefLRUPolicyMSPerMB=0}) — + * see {@link ClassInfoSoftModeStressProbe} for the invariants. Runtime is + * ~30s by design; the deterministic-clearing companion is + * {@link ClassInfoSoftModeTest}. + */ +final class ClassInfoSoftModeStressTest { + + @Test + void softModeSurvivesRealGcClearingUnderConcurrency() { + def javaBin = System.getProperty('java.home') + '/bin/java' + def cp = System.getProperty('java.class.path') + def pb = new ProcessBuilder( + javaBin, + '-Dgroovy.use.classvalue=soft', + '-Xmx128m', + '-XX:SoftRefLRUPolicyMSPerMB=0', + '-cp', cp, + 'org.codehaus.groovy.reflection.ClassInfoSoftModeStressProbe') + pb.redirectErrorStream(true) + def proc = pb.start() + def out = new StringBuilder() + proc.inputStream.eachLine { out.append(it).append('\n') } + assertTrue(proc.waitFor(120, TimeUnit.SECONDS), "stress probe timed out: $out") + assertEquals(0, proc.exitValue(), "stress probe failed: $out") + assertTrue(out.contains('OK'), out.toString()) + } +} diff --git a/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeTest.groovy b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeTest.groovy new file mode 100644 index 00000000000..f8ca44177bc --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeTest.groovy @@ -0,0 +1,49 @@ +/* + * 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.groovy.reflection + +import org.junit.jupiter.api.Test + +import static org.junit.jupiter.api.Assertions.assertEquals +import static org.junit.jupiter.api.Assertions.assertTrue + +/** + * GROOVY-12281: soft-value global ClassValue mode + * ({@code -Dgroovy.use.classvalue=soft}). The mode is chosen once at startup, + * so the semantics are asserted in a child JVM running + * {@link ClassInfoSoftModeProbe}. + */ +final class ClassInfoSoftModeTest { + + @Test + void softModeSemantics_inChildProcess() { + def javaBin = System.getProperty('java.home') + '/bin/java' + def cp = System.getProperty('java.class.path') + def pb = new ProcessBuilder( + javaBin, + '-Dgroovy.use.classvalue=soft', + '-cp', cp, + 'org.codehaus.groovy.reflection.ClassInfoSoftModeProbe') + pb.redirectErrorStream(true) + def proc = pb.start() + def out = proc.inputStream.text + assertEquals(0, proc.waitFor(), "soft-mode probe failed: $out") + assertTrue(out.contains('OK'), out) + } +} diff --git a/src/test/groovy/org/codehaus/groovy/reflection/GroovyClassValueSoftTest.groovy b/src/test/groovy/org/codehaus/groovy/reflection/GroovyClassValueSoftTest.groovy new file mode 100644 index 00000000000..eccf864ed1c --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/reflection/GroovyClassValueSoftTest.groovy @@ -0,0 +1,135 @@ +/* + * 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.groovy.reflection + +import org.junit.jupiter.api.Test + +import java.lang.ref.SoftReference +import java.lang.ref.WeakReference +import java.util.concurrent.atomic.AtomicInteger + +import static org.junit.jupiter.api.Assertions.assertEquals +import static org.junit.jupiter.api.Assertions.assertNotSame +import static org.junit.jupiter.api.Assertions.assertNull +import static org.junit.jupiter.api.Assertions.assertSame +import static org.junit.jupiter.api.Assertions.assertTrue + +/** + * GROOVY-12281: soft-value {@code GroovyClassValue} with resurrection. + * The forced-clear tests simulate GC clearing of the memoized SoftReference; + * a manual {@code clear()} is a conservative superset of what a collector can + * do (a collector never clears a reference to a strongly reachable object), + * so correctness under these tests implies correctness under any GC schedule. + */ +final class GroovyClassValueSoftTest { + + private static final class Holder { + final Class type + Holder(Class type) { this.type = type } + } + + private static GroovyClassValueSoft newSubject(AtomicInteger counter) { + new GroovyClassValueSoft({ Class type -> + counter.incrementAndGet() + new Holder(type) + } as GroovyClassValue.ComputeValue) + } + + /** Clears the memoized SoftReference for {@code type}, as a GC would. */ + private static void forceClear(GroovyClassValueSoft subject, Class type) { + def storeField = GroovyClassValueSoft.getDeclaredField('store') + storeField.accessible = true + ClassValue store = storeField.get(subject) + ((SoftReference) store.get(type)).clear() + } + + private static boolean awaitCollected(WeakReference ref) { + for (int i = 0; i < 100 && ref.get() != null; i++) { + System.gc() + byte[][] pressure = new byte[64][] + for (int j = 0; j < pressure.length; j++) { + pressure[j] = new byte[1 << 16] + } + Thread.sleep(10) + } + return ref.get() == null + } + + @Test + void memoizesLikeAnyClassValue() { + def counter = new AtomicInteger() + def subject = newSubject(counter) + def first = subject.get(String) + assertSame(first, subject.get(String)) + assertEquals(1, counter.get()) + subject.get(Integer) + assertEquals(2, counter.get()) + } + + @Test + void clearedValueIsResurrectedWhileAlive_sameIdentity_noRecompute() { + def counter = new AtomicInteger() + def subject = newSubject(counter) + def survivor = subject.get(String) // strong local ref: "captured by a call site" + assertEquals(1, counter.get()) + + forceClear(subject, String) + def resurrected = subject.get(String) + + assertSame(survivor, resurrected, 'a live value must be resurrected, never replaced') + assertEquals(1, counter.get(), 'resurrection must not invoke computeValue') + } + + @Test + void deadValueIsRecreatedFresh() { + def counter = new AtomicInteger() + def subject = newSubject(counter) + def weak = new WeakReference(subject.get(String)) + assertEquals(1, counter.get()) + + forceClear(subject, String) + assertTrue(awaitCollected(weak), 'unreferenced value should be collectable once cleared') + + def fresh = subject.get(String) + assertEquals(2, counter.get(), 'a truly dead value is recomputed') + assertSame(fresh, subject.get(String)) + } + + @Test + void removeIsAHardDetach_evenWhileOldValueAlive() { + def counter = new AtomicInteger() + def subject = newSubject(counter) + def old = subject.get(String) + subject.remove(String) + def fresh = subject.get(String) + assertNotSame(old, fresh, 'remove() must forget identity (undeploy semantics)') + assertEquals(2, counter.get()) + } + + @Test + void getIfPresentNeverCreates() { + def counter = new AtomicInteger() + def subject = newSubject(counter) + assertNull(subject.getIfPresent(String)) + assertEquals(0, counter.get()) + def value = subject.get(String) + assertSame(value, subject.getIfPresent(String)) + assertEquals(1, counter.get()) + } +} From f242c84c238ebbe8fba98fbf9490c771e1a820e4 Mon Sep 17 00:00:00 2001 From: Paul King Date: Fri, 21 Aug 2026 15:30:56 +1000 Subject: [PATCH 2/2] =?UTF-8?q?GROOVY-12281:=20review=20response=20?= =?UTF-8?q?=E2=80=94=20pin=20non-reconstructible=20state=20inside=20its=20?= =?UTF-8?q?association;=20expose=20value=20reclaimability=20as=20a=20store?= =?UTF-8?q?=20capability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Jochen's reverse memory-pressure question on PR #2820 found a real leak: the global strong root set extended EMC-dirty script-created classes (and their loaders) to the runtime's lifetime, where the default mode releases them with their class (probe: 0/20 loaders collected vs 20/20). The root set's justifying comment was true for immortal platform keys but false for collectible ones. - GroovyClassValue gains valuesReclaimable() plus default-no-op pin/unpin: callers never consult the mode; they ask the store for its capabilities. - GroovyClassValueSoft stores a per-Class slot (bootstrap AtomicReference) holding either a bootstrap SoftReference (reclaimable) or the value itself (pinned). The strong hold lives inside the association — an ephemeron — so a pinned ClassInfo lives exactly as long as its class: an immortal platform key retains it, a dropped script class releases it together with its loader. pin() retries across a concurrent remove-and-recompute; unpin() is a CAS so a foreign value cannot downgrade the slot. - ClassInfo drops nonReclaimableRoots and the isSoftMode() coupling; updateReclaimability() pins/unpins unconditionally (no-ops outside soft mode) and remove() needs no store-specific unroot: the pin travels with the association. - GroovyClassValueFactory stores the parsed mode as booleans; the mode string never leaves the factory. - IndyInvalidation / SwitchPointInvalidator document their layering (mechanism vs policy, with ClassInfo the only other supported consumer) and are annotated @Internal. Tests: GroovyClassValueSoftTest adds the capability, pin-until-unpin, remove-releases-pin and foreign-unpin-noop cases; ClassInfoSoftModeProbe gains the reverse scenario (an EMC-dirty script class dies with its loader under memory pressure) and asserts pinned slots directly — the stronger claim, since a pinned slot has no clearable reference. The reverse scenario is @CompileStatic and nulls its loop locals on purpose: indy call-site guards in the long-lived probe class and stale frame slots retain script classes in every mode — receiver-side effects distinct from the association lifetime under test. The classic call-site scenario moves to a clean POJO receiver, since a pinned receiver like String no longer has a clearable reference. Co-Authored-By: Claude Fable 5 --- .../groovy/runtime/indy/IndyInvalidation.java | 15 ++ .../runtime/indy/SwitchPointInvalidator.java | 15 ++ .../codehaus/groovy/reflection/ClassInfo.java | 91 ++++------ .../groovy/reflection/GroovyClassValue.java | 43 ++++- .../reflection/GroovyClassValueFactory.java | 18 +- .../reflection/GroovyClassValueSoft.java | 98 +++++++--- .../reflection/ClassInfoSoftModeProbe.groovy | 168 ++++++++++++++---- .../reflection/ClassInfoSoftModeTest.groovy | 1 + .../GroovyClassValueSoftTest.groovy | 73 +++++++- 9 files changed, 397 insertions(+), 125 deletions(-) diff --git a/src/main/java/org/apache/groovy/runtime/indy/IndyInvalidation.java b/src/main/java/org/apache/groovy/runtime/indy/IndyInvalidation.java index d77c7332ee8..39b8059d395 100644 --- a/src/main/java/org/apache/groovy/runtime/indy/IndyInvalidation.java +++ b/src/main/java/org/apache/groovy/runtime/indy/IndyInvalidation.java @@ -23,6 +23,7 @@ import groovy.lang.MetaClass; import groovy.lang.MetaClassImpl; import groovy.lang.MetaClassRegistryChangeEvent; +import groovy.transform.Internal; import org.apache.groovy.util.SystemUtil; import org.apache.groovy.util.concurrent.ManagedIdentityConcurrentMap; import org.codehaus.groovy.reflection.ClassInfo; @@ -73,8 +74,22 @@ * Production guards: {@code IndyInterface.applyMopSwitchPoints}; tests may use * {@link #guardWithMopSwitchPoints}. * + *

Layering

+ * This class is the policy half of a two-level subsystem: + * {@link SwitchPointInvalidator} is the mechanism (one domain's SwitchPoint + * lifecycle plus the process-wide live registry) and makes no policy + * decisions; this class decides invalidation width, labels the reasons, owns + * reclaim anchoring and — when the global ClassValue store reclaims values + * (GROOVY-12281 soft mode) — per-Class domain continuity. The only other + * supported consumer of the mechanism is {@link ClassInfo}, which owns domain + * instances and performs the local operations (allocate, invalidate, detach) + * directly. No other code should construct or invalidate + * {@link SwitchPointInvalidator} instances; both classes are internal and may + * change incompatibly. + * * @since 6.0.0 */ +@Internal public final class IndyInvalidation { private static final Logger LOG = Logger.getLogger(IndyInvalidation.class.getName()); diff --git a/src/main/java/org/apache/groovy/runtime/indy/SwitchPointInvalidator.java b/src/main/java/org/apache/groovy/runtime/indy/SwitchPointInvalidator.java index 9639aafbc04..912c477acd7 100644 --- a/src/main/java/org/apache/groovy/runtime/indy/SwitchPointInvalidator.java +++ b/src/main/java/org/apache/groovy/runtime/indy/SwitchPointInvalidator.java @@ -18,6 +18,8 @@ */ package org.apache.groovy.runtime.indy; +import groovy.transform.Internal; + import java.lang.invoke.SwitchPoint; import java.util.List; import java.util.concurrent.ConcurrentHashMap; @@ -53,8 +55,21 @@ * not the SwitchPoint object, so the registry entry is what keeps a * still-installed guard retirable until its domain is explicitly retired. * + *

Layering

+ * This is the mechanism half of the MOP invalidation subsystem and + * makes no policy decisions. It has exactly two supported consumers: + * {@link org.codehaus.groovy.reflection.ClassInfo}, which owns domain + * instances (allocation, local invalidate, detach), and + * {@link IndyInvalidation}, which owns invalidation width policy, reclaim + * anchoring and per-Class domain continuity. On its own this class guarantees + * only the single-domain lifecycle documented above (single-use SwitchPoints; + * registration precedes publication); guarantees about domain identity across + * ClassInfo recreation exist only under {@link IndyInvalidation}'s + * management. It is not an extension point and may change incompatibly. + * * @since 6.0.0 */ +@Internal public final class SwitchPointInvalidator { /** diff --git a/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java b/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java index 0d40ff51b5b..c2923aceb3c 100644 --- a/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java +++ b/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java @@ -61,8 +61,6 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; /** @@ -112,29 +110,6 @@ public class ClassInfo implements Finalizable { private static final ManagedConcurrentLinkedQueue modifiedExpandos = new ManagedConcurrentLinkedQueue(weakBundle); - /** - * Whether {@link #globalClassValue} stores its values softly with - * resurrection ({@code -Dgroovy.use.classvalue=soft}, GROOVY-12281 - * investigation prototype). Soft mode needs two cooperating pieces here: - * strong roots for non-reconstructible state ({@link #nonReclaimableRoots}) - * and per-Class indy domain continuity (see {@link #indyDomain()}). - */ - private static final boolean SOFT_CLASS_VALUES = GroovyClassValueFactory.isSoftMode(); - - /** - * Soft-mode strong roots for ClassInfos whose state could not be - * reconstructed if the instance were soft-collected and later recreated: - * an installed class-level MetaClass ({@link #setStrongMetaClass}), - * per-instance MetaClasses, or registry-written DGM/extension method - * arrays ({@link CachedClass#setNewMopMethods}/{@code addNewMopMethods}). - * The set itself is Groovy-loaded, so it contributes no foreign-loader - * pinning: everything in it dies with Groovy's own loader, which is the - * lifetime every ClassInfo has today under the default strong ClassValue. - * {@code null} unless soft mode is active. - */ - private static final Set nonReclaimableRoots = - SOFT_CLASS_VALUES ? ConcurrentHashMap.newKeySet() : null; - private static final GroovyClassValue globalClassValue = GroovyClassValueFactory.createGroovyClassValue(new ComputeValue(){ /** * Creates a new {@code ClassInfo} for the given class type. @@ -151,6 +126,16 @@ public ClassInfo computeValue(Class type) { } }); + /** + * Whether {@link #globalClassValue} can collect a ClassInfo while its + * class is still alive ({@code -Dgroovy.use.classvalue=soft}, GROOVY-12281 + * investigation prototype). Reclaimable values need two cooperating pieces + * here: ephemeron pinning of instances carrying non-reconstructible state + * ({@link #updateReclaimability()}) and per-Class indy domain continuity + * (see {@link #indyDomain()}). + */ + private static final boolean RECLAIMABLE_CLASS_VALUES = globalClassValue.valuesReclaimable(); + private static final GlobalClassSet globalClassSet = new GlobalClassSet(); ClassInfo(Class klazz) { @@ -221,7 +206,7 @@ private void bumpGenerationLocal() { * lifetime equals this ClassInfo's — sound because the strong ClassValue * keeps the instance alive as long as its class. *

- * Soft mode (GROOVY-12281): the per-Class domain from + * Reclaimable values (GROOVY-12281, soft mode): the per-Class domain from * {@link IndyInvalidation#classDomainFor}, seeded with the local domain on * first touch. Domain identity then survives ClassInfo recreation: a * successor instance adopts its predecessor's domain, so a MetaClass @@ -233,7 +218,7 @@ private void bumpGenerationLocal() { * no receiver can reach any guard, so the local domain suffices. */ private SwitchPointInvalidator indyDomain() { - if (SOFT_CLASS_VALUES) { + if (RECLAIMABLE_CLASS_VALUES) { Class type = getTheClass(); if (type != null) { return IndyInvalidation.classDomainFor(type, indySwitchPointDomain); @@ -254,10 +239,10 @@ private SwitchPointInvalidator indyDomain() { public SwitchPoint getIndySwitchPoint() { SwitchPointInvalidator domain = indyDomain(); if (!indyDomainAnchored) { - if (SOFT_CLASS_VALUES) { - // Soft mode retires domains when the Class dies, not when a - // ClassInfo instance does: instances are replaceable (their - // successor adopts the same domain), classes are not. + if (RECLAIMABLE_CLASS_VALUES) { + // Reclaimable values retire domains when the Class dies, not + // when a ClassInfo instance does: instances are replaceable + // (their successor adopts the same domain), classes are not. Class type = getTheClass(); if (type != null) { IndyInvalidation.anchorClassDomainToClass(type, domain); @@ -390,15 +375,9 @@ public static ClassInfo getClassInfo (Class cls) { * from cache */ public static void remove(Class cls) { - if (SOFT_CLASS_VALUES && globalClassValue instanceof GroovyClassValueSoft) { - // A hard detach must also unroot the detached instance, or an - // undeployed class's ClassInfo would stay pinned by its own - // non-reclaimable root — the exact leak remove() exists to break. - ClassInfo current = ((GroovyClassValueSoft) globalClassValue).getIfPresent(cls); - if (current != null) { - nonReclaimableRoots.remove(current); - } - } + // A hard detach drops the whole association — including any pin the + // detached instance held there — so undeploy semantics need no + // store-specific handling. globalClassValue.remove(cls); } @@ -477,26 +456,34 @@ public void setStrongMetaClass(MetaClass answer) { } /** - * Soft-mode bookkeeping (GROOVY-12281): keeps this ClassInfo strongly - * rooted while it carries state that could not be reconstructed after - * soft collection — an installed class-level MetaClass, per-instance - * MetaClasses, or registry-written DGM/extension method arrays. The DGM - * condition also enforces the registry-rooting invariant by construction - * rather than by audit: any instance holding non-empty MOP arrays is - * non-collectible, so a recreated instance never needs to rebuild them. + * Reclaimability bookkeeping (GROOVY-12281): while this ClassInfo carries + * state that could not be reconstructed after collection — an installed + * class-level MetaClass, per-instance MetaClasses, or registry-written + * DGM/extension method arrays — it is pinned inside its own + * association ({@link GroovyClassValue#pin}), so it lives exactly as + * long as its class: an immortal platform key retains it (it must — the + * state is not reconstructible), while a dropped script class releases it + * together with its loader. A global strong root would get the second half + * wrong, extending dirty script classes (and their loaders) to the + * runtime's lifetime — the "reverse" leak raised in review of PR #2820. + * The DGM condition also enforces the registry-rooting invariant by + * construction rather than by audit: any instance holding non-empty MOP + * arrays is pinned, so a recreated instance never needs to rebuild them. * Removal is conservative: lingering weak entries in the per-instance map - * merely delay unrooting, which is the safe direction (today's default - * roots every ClassInfo forever). No-op unless soft mode is active. + * merely delay unpinning, which is the safe direction (today's default + * retains every ClassInfo for its class's lifetime). No-op unless the + * value store reclaims values ({@link GroovyClassValue#valuesReclaimable}). */ void updateReclaimability() { - if (!SOFT_CLASS_VALUES) return; + Class type = getTheClass(); + if (type == null) return; if (strongMetaClass != null || (perInstanceMetaClassMap != null && !perInstanceMetaClassMap.isEmpty()) || dgmMetaMethods.length != 0 || newMetaMethods.length != 0) { - nonReclaimableRoots.add(this); + globalClassValue.pin(type, this); } else { - nonReclaimableRoots.remove(this); + globalClassValue.unpin(type, this); } } diff --git a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValue.java b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValue.java index 909f9b189e6..cab34765bbd 100644 --- a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValue.java +++ b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValue.java @@ -30,7 +30,46 @@ interface ComputeValue{ } T get(Class type); - + void remove(Class type); - + + /** + * Whether an association's value can be collected while its key class is + * still alive (GROOVY-12281, {@code -Dgroovy.use.classvalue=soft}). The + * default — a value lives exactly as long as its key class — answers + * {@code false}. Implementations answering {@code true} must honor + * {@link #pin} so callers can exempt values whose state cannot be rebuilt + * by recomputation. + * + * @return {@code true} if values may be collected before their key class + */ + default boolean valuesReclaimable() { + return false; + } + + /** + * Keeps {@code value} strongly reachable from its own key's + * association until {@link #unpin} or {@link #remove}. The value then + * lives exactly as long as {@code type} — like a plain + * {@code java.lang.ClassValue} association: an immortal key retains it, + * a collectible key releases it together with its loader. Implementations + * must not root the value globally, which would extend a collectible key's + * lifetime to the runtime's. No-op unless {@link #valuesReclaimable()}. + * + * @param type the key class + * @param value the current value for {@code type} + */ + default void pin(Class type, T value) { + } + + /** + * Reverts {@link #pin}: the association holds {@code value} reclaimably + * again. No-op when {@code value} is not the currently pinned value, and + * unless {@link #valuesReclaimable()}. + * + * @param type the key class + * @param value the value to release + */ + default void unpin(Class type, T value) { + } } diff --git a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java index b12a725376a..f32336d0da5 100644 --- a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java +++ b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java @@ -34,21 +34,17 @@ class GroovyClassValueFactory { private static final String CLASSVALUE_MODE = SystemUtil.getSystemPropertySafe("groovy.use.classvalue", "true"); /** - * GROOVY-12281: whether values are stored behind - * {@link java.lang.ref.SoftReference}s with resurrection semantics - * ({@code -Dgroovy.use.classvalue=soft}). Soft mode needs cooperation from - * {@link ClassInfo} (strong roots for non-reconstructible state, per-Class - * indy domain continuity), which is why it is exposed package-wide rather - * than kept local to {@link #createGroovyClassValue}. + * GROOVY-12281; see {@link #createGroovyClassValue}. Callers needing + * behavior differences never consult the mode: they ask the created store + * for its capabilities ({@link GroovyClassValue#valuesReclaimable}). */ - static boolean isSoftMode() { - return "soft".equalsIgnoreCase(CLASSVALUE_MODE); - } + private static final boolean SOFT_MODE = "soft".equalsIgnoreCase(CLASSVALUE_MODE); public static GroovyClassValue createGroovyClassValue(ComputeValue computeValue) { // GROOVY-12281: "soft" keeps ClassValue for all keys but holds values softly with - // resurrection, so immortal keys hold no strong chain to the value's loader. - if (isSoftMode()) { + // resurrection and ephemeron pinning, so immortal keys hold no strong chain to the + // value's loader. + if (SOFT_MODE) { return new GroovyClassValueSoft<>(computeValue); } return Boolean.parseBoolean(CLASSVALUE_MODE) diff --git a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueSoft.java b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueSoft.java index a179924ec20..7a821ca3d6a 100644 --- a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueSoft.java +++ b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueSoft.java @@ -24,31 +24,43 @@ import java.lang.ref.SoftReference; import java.util.EnumSet; +import java.util.concurrent.atomic.AtomicReference; /** * GROOVY-12281 investigation prototype ({@code -Dgroovy.use.classvalue=soft}): * keeps {@code java.lang.ClassValue}'s per-{@code Class} fast path but stores - * each value behind a {@link SoftReference}, so an association on an immortal + * each value behind a mutable slot holding either a + * {@link SoftReference} to the value (the reclaimable state) or the value + * itself ({@linkplain #pin pinned}), so an association on an immortal * platform class (for example {@code String}) no longer holds a strong chain - * to the value's class loader (JDK-8136353 / GROOVY-12142). The stored wrapper - * is a bootstrap-loaded {@code java.lang.ref.SoftReference}, and the per-Class - * entry references the defining {@code ClassValue} only weakly (via its - * {@code Version}; the map key is a bootstrap {@code Identity} object), so the - * only path from an immortal key to the Groovy-loaded value is softly - * reachable and is cleared under memory pressure once nothing else keeps the - * Groovy island alive. + * to the value's class loader unless the value was deliberately pinned + * (JDK-8136353 / GROOVY-12142). The slot is a bootstrap-loaded + * {@link AtomicReference} and the wrapper a bootstrap-loaded + * {@code java.lang.ref.SoftReference}; the per-Class entry references the + * defining {@code ClassValue} only weakly (via its {@code Version}; the map + * key is a bootstrap {@code Identity} object). So the only path from an + * immortal key to the Groovy-loaded value is softly reachable — cleared under + * memory pressure once nothing else keeps the Groovy island alive. *

* Correctness relies on resurrection: a weak-key/weak-value side map * is the identity authority. {@code computeValue} consults it first, so when - * the {@code ClassValue}'s soft reference has been cleared but the value is - * still reachable anywhere — for example captured by a linked call site — the - * same instance is re-associated rather than a fresh one created. A fresh - * instance can only be created once the old one is weakly unreachable, at - * which point no guard, cache or call site can still observe the old - * instance, so "two live generations of one association" cannot arise. + * the slot's soft reference has been cleared but the value is still reachable + * anywhere — for example captured by a linked call site — the same instance + * is re-associated rather than a fresh one created. A fresh instance can only + * be created once the old one is weakly unreachable, at which point no guard, + * cache or call site can still observe the old instance, so "two live + * generations of one association" cannot arise. *

- * {@link #remove(Class)} stays a hard detach (the identity entry is - * purged too), preserving the documented undeploy semantics of + * {@linkplain #pin Pinning} flips a slot to hold its value strongly, giving + * the value exactly a plain {@code ClassValue} association's lifetime: as + * long as its key class, and no longer. Because the strong hold lives inside + * the association (an ephemeron), a pinned value on a collectible key — for + * example an EMC-carrying script class — is released together with its class + * and loader; a global strong root would instead extend it to the runtime's + * lifetime (the "reverse" leak raised in review of PR #2820). + *

+ * {@link #remove(Class)} stays a hard detach (the identity entry and + * any pin go too), preserving the documented undeploy semantics of * {@link ClassInfo#remove(Class)}: the next {@code get} creates a fresh value * even if the old one is still reachable somewhere. * @@ -78,10 +90,16 @@ class GroovyClassValueSoft implements GroovyClassValue { */ private final Object[] creationLocks = new Object[64]; - private final ClassValue> store = new ClassValue>() { + /** + * Per-Class slot: contains either a {@code SoftReference} (reclaimable) + * or the value itself (pinned). Both the slot and the wrapper are + * bootstrap-loaded, so an unpinned association on an immortal key keeps + * nothing Groovy-loaded strongly reachable. + */ + private final ClassValue> store = new ClassValue>() { @Override - protected SoftReference computeValue(final Class type) { - return new SoftReference<>(canonical(type)); + protected AtomicReference computeValue(final Class type) { + return new AtomicReference<>(new SoftReference<>(canonical(type))); } }; @@ -94,13 +112,13 @@ protected SoftReference computeValue(final Class type) { @Override public T get(final Class type) { - T value = store.get(type).get(); + T value = deref(store.get(type).get()); if (value != null) return value; - // The soft reference was cleared: drop the memoized wrapper and + // The soft reference was cleared: drop the memoized slot and // recompute once — resurrection returns the canonical instance when // it is still alive (GROOVY-12280 remove-and-recompute pattern). store.remove(type); - value = store.get(type).get(); + value = deref(store.get(type).get()); if (value != null) return value; // Pathological memory pressure cleared the fresh wrapper before we // could dereference it: serve the canonical instance uncached; a @@ -109,12 +127,48 @@ public T get(final Class type) { return canonical(type); } + /** The value a slot's content designates: through the soft wrapper, or the pinned value itself. */ + @SuppressWarnings("unchecked") + private T deref(final Object slotContent) { + return slotContent instanceof SoftReference ? ((SoftReference) slotContent).get() : (T) slotContent; + } + @Override public void remove(final Class type) { canonical.remove(type); store.remove(type); } + @Override + public boolean valuesReclaimable() { + return true; + } + + /** + * {@inheritDoc} + *

+ * The retry handles the race with {@link #get}'s remove-and-recompute: if + * the slot we wrote was concurrently discarded (its soft reference had + * been cleared), the write is repeated on the successor slot — whose + * recompute returned the same canonical instance, because {@code value} + * is strongly reachable in our hands throughout. + */ + @Override + public void pin(final Class type, final T value) { + while (true) { + AtomicReference slot = store.get(type); + slot.set(value); + if (store.get(type) == slot) return; // still current: the strong hold is visible + } + } + + @Override + public void unpin(final Class type, final T value) { + // CAS: only downgrade the exact pinned value; a slot already holding a + // soft wrapper (or a successor value) is left untouched. + store.get(type).compareAndSet(value, new SoftReference<>(value)); + } + /** * Returns the current canonical value without creating one — used by * detach paths that must act on the existing instance (for example diff --git a/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeProbe.groovy b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeProbe.groovy index 7b9b41abe06..744858ecb5a 100644 --- a/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeProbe.groovy +++ b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeProbe.groovy @@ -19,12 +19,15 @@ package org.codehaus.groovy.reflection import groovy.lang.GroovySystem +import groovy.transform.CompileStatic +import org.codehaus.groovy.runtime.InvokerHelper import java.lang.invoke.SwitchPoint import java.lang.ref.SoftReference import java.lang.ref.WeakReference import java.util.zip.Adler32 import java.util.zip.CRC32 +import java.util.zip.Deflater import java.util.zip.Inflater /** @@ -43,8 +46,11 @@ final class ClassInfoSoftModeProbe { def ping() { 'pong' } } - /** Clears the global store's memoized SoftReference for {@code type}. */ - private static void clearSoft(Class type) { + /** + * The global store's slot content for {@code type}: a SoftReference for a + * reclaimable value, or the ClassInfo itself when pinned (dirty state). + */ + private static Object slotContent(Class type) { def gcvField = ClassInfo.getDeclaredField('globalClassValue') gcvField.accessible = true def gcv = gcvField.get(null) @@ -52,7 +58,23 @@ final class ClassInfoSoftModeProbe { def storeField = GroovyClassValueSoft.getDeclaredField('store') storeField.accessible = true ClassValue store = storeField.get(gcv) - ((SoftReference) store.get(type)).clear() + store.get(type).get() + } + + /** Clears the global store's memoized SoftReference for {@code type}. A pinned slot has none. */ + private static void clearSoft(Class type) { + def content = slotContent(type) + assert content instanceof SoftReference : "a pinned slot cannot be cleared by GC (${type.name})" + ((SoftReference) content).clear() + } + + /** + * Asserts the strongest statement available about non-reclaimable state: + * the slot holds the ClassInfo itself, so no GC schedule can clear it — + * while the class lives, exactly like the default strong ClassValue. + */ + private static void assertPinned(Class type) { + assert slotContent(type) instanceof ClassInfo : "ClassInfo for ${type.name} should be pinned in its association" } private static boolean awaitCollected(WeakReference ref) { @@ -67,69 +89,93 @@ final class ClassInfoSoftModeProbe { return ref.get() == null } + /** + * Applies real memory pressure: the collector clears soft references + * before throwing {@link OutOfMemoryError}, so generic soft caches + * (for example {@code ClassInfo}'s lazy CachedClass/loader references) + * release their referents — the "on memory pressure" premise of the + * reverse scenario. Mild GC alone retains them in every mode. + */ + private static void applySevereMemoryPressure() { + def hold = [] + try { + while (true) { hold << new byte[1 << 20] } + } catch (OutOfMemoryError expected) { + hold = null + } + } + static void main(String[] args) { resurrectionPreservesIdentityAndVersion() - dgmTargetClassInfoIsRooted() - strongMetaClassSurvivesClear() - perInstanceMetaClassSurvivesClear() + dgmTargetClassInfoIsPinned() + strongMetaClassPinsAndUnpinsWithItsState() + perInstanceMetaClassPinsClassInfo() pristineClassInfoIsCollectedAndRecreatedWorking() classicCallSiteStaysSoundAcrossClearAndRelinksOnChange() indyDomainContinuityAcrossRecreation() + dirtyScriptClassDiesWithItsLoader() println 'OK' } /** * The split-brain check: a ClassInfo captured by any holder must be - * returned as-is after the ClassValue's soft reference clears, with its - * version untouched, so captured version guards stay sound. + * returned as-is after the slot's soft reference clears, with its + * version untouched, so captured version guards stay sound. Uses a class + * with no registry-written DGM arrays, whose slot is therefore soft. */ private static void resurrectionPreservesIdentityAndVersion() { - ClassInfo before = ClassInfo.getClassInfo(String) // strong local ref: "captured by a call site" + ClassInfo before = ClassInfo.getClassInfo(Deflater) // strong local ref: "captured by a call site" int version = before.version - clearSoft(String) - ClassInfo after = ClassInfo.getClassInfo(String) + clearSoft(Deflater) + ClassInfo after = ClassInfo.getClassInfo(Deflater) assert after.is(before) : 'live ClassInfo must be resurrected, not replaced' assert after.version == version : 'resurrection must not disturb the version guard stamp' } /** * The enforced E3 invariant: a ClassInfo holding registry-written DGM/extension - * method arrays is strongly rooted, so it can never be soft-collected and a - * recreated instance never needs to rebuild those arrays. + * method arrays is pinned inside its own association — the slot holds the + * instance itself, so no GC schedule can clear it while the class lives and + * a recreated instance never needs to rebuild those arrays. */ - private static void dgmTargetClassInfoIsRooted() { + private static void dgmTargetClassInfoIsPinned() { def weak = new WeakReference(ClassInfo.getClassInfo(String)) - clearSoft(String) + assertPinned(String) System.gc() - assert weak.get() != null : 'DGM-target ClassInfo must be rooted (non-reclaimable)' + assert weak.get() != null : 'DGM-target ClassInfo must be pinned (non-reclaimable)' assert ClassInfo.getClassInfo(String).is(weak.get()) assert 'abc'.reverse() == 'cba' : 'String DGM dispatch intact' } - /** User metaclass customizations must survive value clearing (dirty root). */ - private static void strongMetaClassSurvivesClear() { + /** + * User metaclass customizations are non-reconstructible: installing one + * pins the ClassInfo in its association; removing it unpins, restoring + * reclaimability — the pin follows the state, not the class. + */ + private static void strongMetaClassPinsAndUnpinsWithItsState() { CRC32.metaClass.twiddle = { -> 42 } try { def weak = new WeakReference(ClassInfo.getClassInfo(CRC32)) - clearSoft(CRC32) + assertPinned(CRC32) System.gc() - assert weak.get() != null : 'ClassInfo with installed MetaClass must be rooted' - assert new CRC32().twiddle() == 42 : 'EMC customization must survive the clear' + assert weak.get() != null : 'ClassInfo with installed MetaClass must be pinned' + assert new CRC32().twiddle() == 42 : 'EMC customization must survive GC' } finally { GroovySystem.metaClassRegistry.removeMetaClass(CRC32) } + assert slotContent(CRC32) instanceof SoftReference : 'removing the MetaClass must unpin the ClassInfo' } /** Per-instance metaclasses are equally non-reconstructible state. */ - private static void perInstanceMetaClassSurvivesClear() { + private static void perInstanceMetaClassPinsClassInfo() { def receiver = new Adler32() receiver.metaClass.spin = { -> 7 } try { def weak = new WeakReference(ClassInfo.getClassInfo(Adler32)) - clearSoft(Adler32) + assertPinned(Adler32) System.gc() - assert weak.get() != null : 'ClassInfo with per-instance MetaClass must be rooted' - assert receiver.spin() == 7 : 'per-instance customization must survive the clear' + assert weak.get() != null : 'ClassInfo with per-instance MetaClass must be pinned' + assert receiver.spin() == 7 : 'per-instance customization must survive GC' } finally { receiver.metaClass = null } @@ -158,26 +204,29 @@ final class ClassInfoSoftModeProbe { * version at link time. Resurrection keeps that capture sound across a * clear, and a later MetaClass change must still be observed via the * version guard on the same instance. Without resurrection the stale site - * would answer 'cba' forever. + * would answer the original result forever. The receiver is a POJO with no + * registry-written DGM arrays, so its slot is soft (clearable); a pinned + * receiver like {@code String} can never be cleared in the first place. */ private static void classicCallSiteStaysSoundAcrossClearAndRelinksOnChange() { // groovy-callsite is runtime-only for core (as for legacy-compiled jars // in the wild), so drive it reflectively def csaClass = Class.forName('org.codehaus.groovy.runtime.callsite.CallSiteArray') - def csa = csaClass.getConstructor(Class, String[]).newInstance(ClassInfoSoftModeProbe, ['reverse'] as String[]) + def csa = csaClass.getConstructor(Class, String[]).newInstance(ClassInfoSoftModeProbe, ['toString'] as String[]) def noparam = csaClass.NOPARAM - assert csa.array[0].call('abc', noparam) == 'cba' - // the linked site (csa.array[0] after the first call) now captures ClassInfo(String)+version - clearSoft(String) + def joiner = new StringJoiner('-') + assert csa.array[0].call(joiner, noparam) == '' + // the linked site (csa.array[0] after the first call) now captures ClassInfo(StringJoiner)+version + clearSoft(StringJoiner) System.gc() - assert csa.array[0].call('abc', noparam) == 'cba' : 'linked site stays correct across the clear' - String.metaClass.reverse = { -> 'emc' } + assert csa.array[0].call(joiner, noparam) == '' : 'linked site stays correct across the clear' + StringJoiner.metaClass.toString = { -> 'emc' } try { - assert csa.array[0].call('abc', noparam) == 'emc' : 'version guard on the resurrected instance must observe the change' + assert csa.array[0].call(joiner, noparam) == 'emc' : 'version guard on the resurrected instance must observe the change' } finally { - GroovySystem.metaClassRegistry.removeMetaClass(String) + GroovySystem.metaClassRegistry.removeMetaClass(StringJoiner) } - assert csa.array[0].call('abc', noparam) == 'cba' + assert csa.array[0].call(joiner, noparam) == '' } /** @@ -200,4 +249,53 @@ final class ClassInfoSoftModeProbe { successor.incVersion() assert sp.hasBeenInvalidated() : "predecessor's SwitchPoint must be retired by a mutation through the successor" } + + /** Installs an EMC method statically, replicating {@code cls.metaClass.extra = { -> 42 }}. */ + @CompileStatic + private static void installExtra(Class cls) { + def emc = new ExpandoMetaClass(cls, true, true) + emc.initialize() + emc.setProperty('extra', { -> 42 }) + GroovySystem.metaClassRegistry.setMetaClass(cls, emc) + } + + /** + * The "reverse" scenario (Jochen, PR #2820 review): the Groovy runtime + * stays alive while script loaders come and go, and a script installs an + * EMC on a class it created. The pin lives inside the class's own + * association, so on memory pressure dropping the loader must release the + * class, its ClassInfo, the EMC and the loader itself — exactly as the + * default strong ClassValue does. A global strong root would fail this: + * it would extend every EMC-dirty script class to the runtime's lifetime. + *

+ * Compiled statically on purpose: a dynamic call in this long-lived probe + * class would link its invokedynamic call-site guards against the script + * classes, retaining them from the call site — a receiver-side inline-cache + * effect present in every mode, not the association lifetime this scenario + * isolates. Locals are nulled for the same reason: the last iteration's + * frame slots stay reachable through the collection loop below. + */ + @CompileStatic + private static void dirtyScriptClassDiesWithItsLoader() { + List> loaderRefs = [] + List> classRefs = [] + for (int i = 0; i < 3; i++) { + def gcl = new GroovyClassLoader() + Class cls = gcl.parseClass("class ReverseScripted${i} { def hi() { 'hi' } }") + installExtra(cls) + def obj = cls.getDeclaredConstructor().newInstance() + assert InvokerHelper.invokeMethod(obj, 'extra', null) == 42 : 'EMC on the script-created class must dispatch' + assertPinned(cls) + loaderRefs << new WeakReference(gcl) + classRefs << new WeakReference(cls) + obj = null; cls = null; gcl = null + } + applySevereMemoryPressure() + loaderRefs.each { WeakReference ref -> + assert awaitCollected(ref) : 'dropped script loader with an EMC-dirty class must be collectable (reverse scenario)' + } + classRefs.each { WeakReference ref -> + assert ref.get() == null : 'the script class must go with its loader' + } + } } diff --git a/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeTest.groovy b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeTest.groovy index f8ca44177bc..e981d2b1858 100644 --- a/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/reflection/ClassInfoSoftModeTest.groovy @@ -37,6 +37,7 @@ final class ClassInfoSoftModeTest { def cp = System.getProperty('java.class.path') def pb = new ProcessBuilder( javaBin, + '-Xmx256m', // bounded heap: the reverse scenario fills it to apply real memory pressure '-Dgroovy.use.classvalue=soft', '-cp', cp, 'org.codehaus.groovy.reflection.ClassInfoSoftModeProbe') diff --git a/src/test/groovy/org/codehaus/groovy/reflection/GroovyClassValueSoftTest.groovy b/src/test/groovy/org/codehaus/groovy/reflection/GroovyClassValueSoftTest.groovy index eccf864ed1c..16738b2e62f 100644 --- a/src/test/groovy/org/codehaus/groovy/reflection/GroovyClassValueSoftTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/reflection/GroovyClassValueSoftTest.groovy @@ -51,12 +51,27 @@ final class GroovyClassValueSoftTest { } as GroovyClassValue.ComputeValue) } - /** Clears the memoized SoftReference for {@code type}, as a GC would. */ - private static void forceClear(GroovyClassValueSoft subject, Class type) { + /** The current slot content for {@code type}: a SoftReference (reclaimable) or the value itself (pinned). */ + private static Object slotContent(GroovyClassValueSoft subject, Class type) { def storeField = GroovyClassValueSoft.getDeclaredField('store') storeField.accessible = true ClassValue store = storeField.get(subject) - ((SoftReference) store.get(type)).clear() + store.get(type).get() + } + + /** Clears the memoized SoftReference for {@code type}, as a GC would. A pinned slot has none. */ + private static void forceClear(GroovyClassValueSoft subject, Class type) { + def content = slotContent(subject, type) + assert content instanceof SoftReference : 'a pinned (strong) slot cannot be cleared by GC' + ((SoftReference) content).clear() + } + + private static boolean survivesGc(WeakReference ref) { + for (int i = 0; i < 5 && ref.get() != null; i++) { + System.gc() + Thread.sleep(10) + } + return ref.get() != null } private static boolean awaitCollected(WeakReference ref) { @@ -122,6 +137,58 @@ final class GroovyClassValueSoftTest { assertEquals(2, counter.get()) } + @Test + void valuesAreReclaimableByContract() { + assertTrue(newSubject(new AtomicInteger()).valuesReclaimable()) + } + + @Test + void pinHoldsTheValueInItsOwnSlotUntilUnpin() { + def counter = new AtomicInteger() + def subject = newSubject(counter) + def value = subject.get(String) + subject.pin(String, value) + assertSame(value, slotContent(subject, String), 'a pinned slot holds the value itself, not a wrapper') + + def weak = new WeakReference(value) + value = null + assertTrue(survivesGc(weak), 'a pinned value must not be collected') + def pinned = weak.get() + assertSame(pinned, subject.get(String)) + assertEquals(1, counter.get(), 'pin must not disturb identity or recompute') + + subject.unpin(String, pinned) + assertTrue(slotContent(subject, String) instanceof SoftReference, 'unpin restores the reclaimable wrapper') + assertSame(pinned, subject.get(String), 'unpin keeps the same live instance') + pinned = null + forceClear(subject, String) + assertTrue(awaitCollected(weak), 'an unpinned value is reclaimable again') + } + + @Test + void removeReleasesThePin() { + def counter = new AtomicInteger() + def subject = newSubject(counter) + def value = subject.get(String) + subject.pin(String, value) + def weak = new WeakReference(value) + value = null + subject.remove(String) + assertTrue(awaitCollected(weak), 'remove() must drop the pin with the association') + subject.get(String) + assertEquals(2, counter.get()) + } + + @Test + void unpinOfAForeignValueIsANoop() { + def counter = new AtomicInteger() + def subject = newSubject(counter) + def value = subject.get(String) + subject.pin(String, value) + subject.unpin(String, new Holder(String)) // not the pinned value + assertSame(value, slotContent(subject, String), 'foreign unpin must not downgrade the slot') + } + @Test void getIfPresentNeverCreates() { def counter = new AtomicInteger()