From 64a46b7cebcecb0d084a6d257040659e826c3725 Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Fri, 15 May 2026 11:58:16 +0200 Subject: [PATCH 1/4] fix(runtime): harden PPI-related DESTROY refcount wiring; extend PPI module plan - InheritanceResolver: do not cache failed DESTROY lookups as null (compile order can define DESTROY after an early miss). - DestroyDispatch: set destroyFired only once Perl DESTROY will run, so a transient resolution miss does not skip PPI::Element::DESTROY. - RuntimeScalar: incrementRefCountForContainerStore activates refCount=-1 nested referents and skips MIN_VALUE / WEAKLY_TRACKED. - dev/modules/ppi.md: future work for t/04_element.t %_PARENT teardown; README index row for PPI. PPI t/04_element.t tests 202/206 remain failing; the doc lists next steps. Generated with [Cursor](https://cursor.com/docs) Co-Authored-By: Cursor Co-authored-by: Cursor --- dev/modules/README.md | 1 + dev/modules/ppi.md | 74 +++++++++++++++++++ .../runtime/mro/InheritanceResolver.java | 11 ++- .../runtime/runtimetypes/DestroyDispatch.java | 14 ++-- .../runtime/runtimetypes/RuntimeScalar.java | 37 +++++++--- 5 files changed, 116 insertions(+), 21 deletions(-) diff --git a/dev/modules/README.md b/dev/modules/README.md index 6982e0e8f..45a3fb6be 100644 --- a/dev/modules/README.md +++ b/dev/modules/README.md @@ -20,6 +20,7 @@ This directory contains design documents and guides related to porting CPAN modu | [math_bigint_bignum.md](math_bigint_bignum.md) | Math::BigInt / BigFloat / BigRat / bignum support (in progress) | | [storable_binary_format.md](storable_binary_format.md) | Storable native Perl binary format — read + write paths landed; jperl ↔ system-perl files interoperate in both directions | | [unicode_collate.md](unicode_collate.md) | Unicode::Collate — plan: file-backed DUCET + Java XS surface (default); optional ICU path tradeoffs | +| [ppi.md](ppi.md) | **PPI** — CPAN test status, RC1–RC4, refcount/`DESTROY` follow-ups (`t/04_element.t` `%_PARENT`) | ## Module Status Overview diff --git a/dev/modules/ppi.md b/dev/modules/ppi.md index 4240b4f49..a6f03bdcb 100644 --- a/dev/modules/ppi.md +++ b/dev/modules/ppi.md @@ -258,3 +258,77 @@ file before closing the phase. - Is the `return $r` bug a separate issue (return copy discipline) or a symptom of the same container-store miscount? Needs a separate minimal repro without containers. + +--- + +## Future work — `t/04_element.t` and `%PPI::Singletons::_PARENT` (2026-05) + +Upstream PPI keeps a weak parent table: `weaken($_PARENT{refaddr $child} = $parent)`. +`PPI::Element::DESTROY` deletes `$_PARENT{refaddr $_[0]}`. +`PPI/tests/t/04_element.t` asserts that after parsing a large document and +destroying the root `PPI::Document`, `scalar keys %_PARENT` returns to the +baseline count (tests **202** explicit `DESTROY`, **206** implicit scope exit). + +### Symptom on PerlOnJava (as of 2026-05) + +Those two subtests still fail: thousands of `_PARENT` keys remain after +document teardown, i.e. many `PPI::Element` objects never run `DESTROY` (or +equivalent cleanup). Other failures in `./jcpan -t PPI` (e.g. `t/07_token.t`, +`t/08_regression.t`) may share refcount/DESTROY plumbing; treat them after +`04_element` is green. + +### Fixes landed incrementally (see linked PR) + +1. **`InheritanceResolver`**: do **not** negative-cache a failed `DESTROY` + method lookup. During `require`/compile order, resolution can run before + a base class installs `DESTROY`; caching `null` would skip Perl destructors + for the rest of the process. +2. **`DestroyDispatch.doCallDestroy`**: set `destroyFired` only **after** it + is known that a Perl `DESTROY` (or the no-DESTROY cleanup path) will run. + Setting it before the null-check could skip `PPI::Element::DESTROY` when + resolution transiently failed. +3. **`RuntimeScalar.incrementRefCountForContainerStore`**: activate nested + referents at `refCount == -1` before incrementing (and refuse `MIN_VALUE` / + `WEAKLY_TRACKED`), so hash/array slot stores and `createReferenceWithTrackedElements` + paths can count anon containers consistently. + +These improve destructor reliability but **do not** yet pass tests 202/206 alone. + +### Next investigation steps (ordered) + +1. **Reproduce at scale** + `./jperl -Ilib t/04_element.t` (or only the failing scope) with `PERL5LIB` + pointing at bundled `IO::File` etc. Use `timeout` on full `./jcpan -t PPI` + (see `AGENTS.md`). + +2. **Correlate stuck keys with JVM state** + For one leaked element after `$doc->DESTROY`, check + `Internals::jperl_refstate`-style introspection (if available): `refCount`, + `refCountOwned`, whether the object is still reachable from a live + `RuntimeArray` slot (`{children}`) or from deferred mortal queues. + +3. **`PPI::Node::DESTROY` pattern** + BFS: `unshift @queue, @{delete $_->{children}}` then `%$_ = ()`. Ensure + `RuntimeHash.delete` and bulk clear both drop nested refcount for **non-`refCountOwned`** + slot scalars where Perl still drops SV refcount (prior art: a narrowly scoped + `deferDecrementRecursive` or hash-delete helper broke unrelated tests when + too broad). + +4. **Hash / anon literal construction** + JVM path uses `RuntimeHash.createHashRef` → `createReferenceWithTrackedElements`. + A mid-construction `MortalList.flush` could in theory destroy nested values + before container-store pins them; `suppressFlush` around **only** the hash-ref + construction sequence was attempted but caused regressions elsewhere — any + retry must be narrower than wrapping all `createHashRef` callers. + +5. **After `04_element` passes** + Re-run full `./jcpan -t PPI`, refresh the failure table at the top of this + doc, then continue Phase 2/RC2 refcount parity work from "Next Steps" above + for lexer/`Structure::Condition` issues. + +### References + +- `lib/PPI/Node.pm` (`DESTROY`), `lib/PPI/Element.pm` (`DESTROY`), + `lib/PPI/Singletons.pm` (`%_PARENT`) +- Runtime: `DestroyDispatch.java`, `MortalList.java`, `RuntimeHash.delete`, + `RuntimeScalar.incrementRefCountForContainerStore` diff --git a/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java b/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java index 4eb4f9fa6..2ab558f51 100644 --- a/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java +++ b/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java @@ -429,8 +429,15 @@ public static RuntimeScalar findMethodInHierarchy(String methodName, String perl } } - // Cache the fact that method was not found (using null) - methodCache.put(cacheKey, null); + // Normally cache "method not found" as null. Do NOT do this for DESTROY: + // during require/use, resolution can run while a package is still compiling + // (e.g. overload/bootstrap) before sub DESTROY is installed in a base class. + // A cached null then pins "no destructor" for that cache key until the next + // full invalidate — PPI's Element tree would skip Perl DESTROY and leak + // %PPI::Singletons::_PARENT keys (t/04_element.t). + if (!"DESTROY".equals(methodName)) { + methodCache.put(cacheKey, null); + } return null; } diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/DestroyDispatch.java b/src/main/java/org/perlonjava/runtime/runtimetypes/DestroyDispatch.java index e6296ca12..e9ed3fa97 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/DestroyDispatch.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/DestroyDispatch.java @@ -250,13 +250,6 @@ public static void callDestroy(RuntimeBase referent) { * Perform the actual DESTROY method call. */ private static void doCallDestroy(RuntimeBase referent, String className) { - // Mark as destroyed before running DESTROY — one-shot guard. - // Prevents re-entrant DESTROY if cascading cleanup brings this - // object's refCount to 0 again within the same call stack. - // Also prevents infinite DESTROY loops for rescued objects - // (destroyFired stays true after rescue — see note in rescue path). - referent.destroyFired = true; - // Use cached method if available RuntimeScalar destroyMethod = destroyMethodCache.get(referent.blessId); if (destroyMethod == null) { @@ -283,6 +276,13 @@ private static void doCallDestroy(RuntimeBase referent, String className) { return; } + // Mark as destroyed only once we will run Perl DESTROY. Setting destroyFired + // before the null check above incorrectly skips PPI::Element::DESTROY (and + // any other Perl destructor) when method resolution transiently returned null + // or fell through without a CODE ref — leaving PPI::_PARENT keys orphaned. + // Prevents re-entrant Perl DESTROY for the same object; see callDestroy. + referent.destroyFired = true; + // If findMethodInHierarchy returned an AUTOLOAD sub (because no explicit DESTROY // exists), we need to set $AUTOLOAD before calling it. The method resolver sets // autoloadVariableName on the RuntimeCode when it falls through to the AUTOLOAD pass. diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java index 770bc5186..d6b40eaf5 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java @@ -942,19 +942,32 @@ public void vivifyLvalue() { * constructor, and their refCount was already incremented at creation time. */ public static void incrementRefCountForContainerStore(RuntimeScalar scalar) { - if (scalar != null && !scalar.refCountOwned - && (scalar.type & REFERENCE_BIT) != 0 && scalar.value instanceof RuntimeBase base - && base.refCount >= 0) { - base.refCount++; - base.hadCountedReference = true; - base.recordOwner(scalar, "incrementRefCountForContainerStore"); - base.recordActiveOwner(scalar); - scalar.refCountOwned = true; - // Phase B1 (refcount_alignment_52leaks_plan.md): track the - // container element so ReachabilityWalker can see it via - // ScalarRefRegistry. - ScalarRefRegistry.registerRef(scalar); + if (scalar == null || scalar.refCountOwned + || (scalar.type & REFERENCE_BIT) == 0 + || !(scalar.value instanceof RuntimeBase base)) { + return; + } + // Destroyed / weaken sentinel states must not be reactivated here. + if (base.refCount == Integer.MIN_VALUE + || base.refCount == WeakRefRegistry.WEAKLY_TRACKED) { + return; } + // Anonymous nested containers start at refCount=-1 (untracked). + // bless {}'s retroactive pass and normal container stores must still count + // the hash slot as a strong ref, otherwise delete $h{k} / hash clear never + // drops the child (PPI::Node->{children}; cpan PPI t/04_element.t). + if (base.refCount < 0) { + base.refCount = 0; + } + base.refCount++; + base.hadCountedReference = true; + base.recordOwner(scalar, "incrementRefCountForContainerStore"); + base.recordActiveOwner(scalar); + scalar.refCountOwned = true; + // Phase B1 (refcount_alignment_52leaks_plan.md): track the + // container element so ReachabilityWalker can see it via + // ScalarRefRegistry. + ScalarRefRegistry.registerRef(scalar); } private static boolean scalarReferenceContentsNeedRetain(RuntimeScalar value) { From 8b04389bbb8f8ee9dce94c74dbb7a71c439a6fec Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Fri, 15 May 2026 12:52:14 +0200 Subject: [PATCH 2/4] fix(runtime): invalidate DESTROY method cache on CV install instead of skipping misses Cache failed DESTROY lookups as null for performance, but clear DESTROY-related methodCache entries and DestroyDispatch caches whenever *::DESTROY map slots change or a stash CV is defined in-place (globalCodeRefFqn + setLargeRefCounted). Generated with [Cursor](https://cursor.com/docs) Co-Authored-By: Cursor Co-authored-by: Cursor --- .../runtime/mro/InheritanceResolver.java | 26 ++++--- .../runtime/runtimetypes/GlobalVariable.java | 67 ++++++++++++++++++- .../runtime/runtimetypes/RuntimeScalar.java | 12 ++++ 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java b/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java index 2ab558f51..857af6813 100644 --- a/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java +++ b/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java @@ -184,6 +184,19 @@ public static void invalidateCache() { DestroyDispatch.invalidateCache(); } + /** + * Invalidates negative and positive DESTROY entries in the method-resolution cache and + * DESTROY dispatch bookkeeping. Call when a {@code *Pkg::DESTROY} slot is installed, + * redefined, or removed so a prior "no DESTROY" cache entry cannot outlive a late CV. + */ + public static void invalidateDestroyMethodLookupCaches() { + methodCache.entrySet().removeIf(e -> { + String k = e.getKey(); + return k.endsWith("::DESTROY") || k.contains("::DESTROY\0"); + }); + DestroyDispatch.invalidateCache(); + } + /** * Retrieves a cached OverloadContext for the given blessing ID. * @@ -429,15 +442,10 @@ public static RuntimeScalar findMethodInHierarchy(String methodName, String perl } } - // Normally cache "method not found" as null. Do NOT do this for DESTROY: - // during require/use, resolution can run while a package is still compiling - // (e.g. overload/bootstrap) before sub DESTROY is installed in a base class. - // A cached null then pins "no destructor" for that cache key until the next - // full invalidate — PPI's Element tree would skip Perl DESTROY and leak - // %PPI::Singletons::_PARENT keys (t/04_element.t). - if (!"DESTROY".equals(methodName)) { - methodCache.put(cacheKey, null); - } + // Cache "method not found" as null, including for DESTROY. Late-installed + // DESTROY subs invalidate these entries via GlobalVariable.globalCodeRefs hooks + // and RuntimeScalar assignments to the *::DESTROY CV slot. + methodCache.put(cacheKey, null); return null; } diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java index 6f0c6ec34..99aef27cd 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java @@ -32,7 +32,7 @@ public class GlobalVariable { // Maps fully-qualified names (package::subname) to indicate they should be called // as user-defined subroutines instead of built-in operators public static final Map isSubs = new HashMap<>(); - public static final Map globalCodeRefs = new PackageRootMap<>(); + public static final Map globalCodeRefs = new GlobalCodeRefMap(); static final Map globalIORefs = new HashMap<>(); static final Map globalFormatRefs = new HashMap<>(); @@ -113,6 +113,71 @@ public void clear() { } } + /** + * Tracks FQN on code-slot scalars and invalidates DESTROY method-resolution caches + * when {@code *Pkg::DESTROY} map entries change (see {@link + * InheritanceResolver#invalidateDestroyMethodLookupCaches}). + */ + private static final class GlobalCodeRefMap extends HashMap { + @Override + public RuntimeScalar put(String key, RuntimeScalar value) { + markPackageGlobalRoot(value); + RuntimeScalar prev = super.put(key, value); + invalidatePackageRootSnapshot(); + if (prev != null && prev != value) { + prev.globalCodeRefFqn = null; + } + if (value != null) { + value.globalCodeRefFqn = key; + } + if (isDestroyCodeSlotKey(key)) { + InheritanceResolver.invalidateDestroyMethodLookupCaches(); + } + return prev; + } + + @Override + public RuntimeScalar putIfAbsent(String key, RuntimeScalar value) { + RuntimeScalar existing = get(key); + if (existing != null) { + markPackageGlobalRoot(existing); + return existing; + } + return put(key, value); + } + + @Override + public RuntimeScalar remove(Object key) { + RuntimeScalar prev = super.remove(key); + if (prev != null) { + invalidatePackageRootSnapshot(); + prev.globalCodeRefFqn = null; + } + if (key instanceof String s && isDestroyCodeSlotKey(s)) { + InheritanceResolver.invalidateDestroyMethodLookupCaches(); + } + return prev; + } + + @Override + public void clear() { + if (!isEmpty()) { + for (RuntimeScalar s : values()) { + if (s != null) { + s.globalCodeRefFqn = null; + } + } + invalidatePackageRootSnapshot(); + } + super.clear(); + } + } + + /** True for global code-ref keys that install Perl's destructor CV. */ + static boolean isDestroyCodeSlotKey(String key) { + return key != null && (key.endsWith("::DESTROY") || key.equals("DESTROY")); + } + static T markPackageGlobalRoot(T root) { if (root == null) return null; root.isPackageGlobalRoot = true; diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java index d6b40eaf5..d0b587300 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java @@ -78,6 +78,13 @@ private static boolean mightBeInteger(String s) { */ public boolean ioOwner; + /** + * When this scalar is installed in {@link GlobalVariable#globalCodeRefs}, the map key + * (fully-qualified name such as {@code My::Pkg::foo}). Used to invalidate DESTROY + * method-resolution caches after in-place CV installation via {@link #set(RuntimeScalar)}. + */ + public String globalCodeRefFqn; + /** * Number of closures that have captured this RuntimeScalar variable. * When {@code captureCount > 0}, {@link #scopeExitCleanup} skips the @@ -1398,6 +1405,11 @@ private RuntimeScalar setLargeRefCounted(RuntimeScalar value) { DestroyDispatch.clearRescuedWeakRefsTo(oldBase); } + if (isPackageGlobalRoot && type == CODE && globalCodeRefFqn != null + && GlobalVariable.isDestroyCodeSlotKey(globalCodeRefFqn)) { + InheritanceResolver.invalidateDestroyMethodLookupCaches(); + } + return this; } From f2795116c8c3f26f84be438b4cfb7bee03ed013c Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Fri, 15 May 2026 13:00:05 +0200 Subject: [PATCH 3/4] fix(runtime): generalize stash CV invalidation to all subs by leaf name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace DESTROY-only methodCache flush with invalidateMethodLookupCachesForStashSubKey: drop cached lookups matching the sub's final segment (e.g. My::Pkg::foo → foo, including \0noautoload keys). DestroyDispatch still clears only when the leaf is DESTROY. GlobalCodeRefMap skips invalidation on first empty stub put; put/remove and setLargeRefCounted cover defined installs and in-place CV updates. Generated with [Cursor](https://cursor.com/docs) Co-Authored-By: Cursor Co-authored-by: Cursor --- .../runtime/mro/InheritanceResolver.java | 34 ++++++++++++----- .../runtime/runtimetypes/GlobalVariable.java | 38 ++++++++++--------- .../runtime/runtimetypes/RuntimeScalar.java | 20 +++++++--- 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java b/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java index 857af6813..e9d2bb47c 100644 --- a/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java +++ b/src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java @@ -185,16 +185,32 @@ public static void invalidateCache() { } /** - * Invalidates negative and positive DESTROY entries in the method-resolution cache and - * DESTROY dispatch bookkeeping. Call when a {@code *Pkg::DESTROY} slot is installed, - * redefined, or removed so a prior "no DESTROY" cache entry cannot outlive a late CV. + * Drop method-resolution cache entries that depend on a given package sub's + * leaf name (stash key {@code My::Pkg::foo} → {@code foo}), including + * {@code \0noautoload} variants. Also clears {@link DestroyDispatch} bookkeeping + * when the leaf is {@code DESTROY}. */ - public static void invalidateDestroyMethodLookupCaches() { + public static void invalidateMethodLookupCachesForStashSubKey(String stashFqn) { + if (stashFqn == null || stashFqn.isEmpty()) { + return; + } + int chop = stashFqn.lastIndexOf("::"); + String leaf = chop < 0 ? stashFqn : stashFqn.substring(chop + 2); + if (leaf.isEmpty()) { + return; + } + String suffix = "::" + leaf; + String suffixNoAutoload = suffix + "\0noautoload"; methodCache.entrySet().removeIf(e -> { String k = e.getKey(); - return k.endsWith("::DESTROY") || k.contains("::DESTROY\0"); + return k.endsWith(suffixNoAutoload) + || k.endsWith(suffix) + || k.equals(leaf) + || k.equals(leaf + "\0noautoload"); }); - DestroyDispatch.invalidateCache(); + if ("DESTROY".equals(leaf)) { + DestroyDispatch.invalidateCache(); + } } /** @@ -442,9 +458,9 @@ public static RuntimeScalar findMethodInHierarchy(String methodName, String perl } } - // Cache "method not found" as null, including for DESTROY. Late-installed - // DESTROY subs invalidate these entries via GlobalVariable.globalCodeRefs hooks - // and RuntimeScalar assignments to the *::DESTROY CV slot. + // Cache "method not found" as null. Late-installed or redefined package subs + // invalidate matching entries via GlobalVariable.globalCodeRefs and in-place CV + // updates on stash-backed RuntimeScalars (see globalCodeRefFqn). methodCache.put(cacheKey, null); return null; } diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java index 99aef27cd..bec936770 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java @@ -114,26 +114,35 @@ public void clear() { } /** - * Tracks FQN on code-slot scalars and invalidates DESTROY method-resolution caches - * when {@code *Pkg::DESTROY} map entries change (see {@link - * InheritanceResolver#invalidateDestroyMethodLookupCaches}). + * Tracks FQN on stash-backed code scalars and invalidates method-resolution cache + * lines that depend on the sub's leaf name when the map entry is meaningfully + * created or replaced (see {@link InheritanceResolver#invalidateMethodLookupCachesForStashSubKey}). */ private static final class GlobalCodeRefMap extends HashMap { + private static void maybeInvalidateMethodCacheForCodeRefPut(String key, RuntimeScalar previous, RuntimeScalar value) { + if (key == null || value == null) { + return; + } + boolean reseat = previous != null && previous != value; + boolean firstDefinedInstall = previous == null && RuntimeCode.isCodeDefined(value); + if (reseat || firstDefinedInstall) { + InheritanceResolver.invalidateMethodLookupCachesForStashSubKey(key); + } + } + @Override public RuntimeScalar put(String key, RuntimeScalar value) { markPackageGlobalRoot(value); - RuntimeScalar prev = super.put(key, value); + RuntimeScalar old = super.put(key, value); invalidatePackageRootSnapshot(); - if (prev != null && prev != value) { - prev.globalCodeRefFqn = null; + if (old != null && old != value) { + old.globalCodeRefFqn = null; } if (value != null) { value.globalCodeRefFqn = key; } - if (isDestroyCodeSlotKey(key)) { - InheritanceResolver.invalidateDestroyMethodLookupCaches(); - } - return prev; + maybeInvalidateMethodCacheForCodeRefPut(key, old, value); + return old; } @Override @@ -153,8 +162,8 @@ public RuntimeScalar remove(Object key) { invalidatePackageRootSnapshot(); prev.globalCodeRefFqn = null; } - if (key instanceof String s && isDestroyCodeSlotKey(s)) { - InheritanceResolver.invalidateDestroyMethodLookupCaches(); + if (key instanceof String s) { + InheritanceResolver.invalidateMethodLookupCachesForStashSubKey(s); } return prev; } @@ -173,11 +182,6 @@ public void clear() { } } - /** True for global code-ref keys that install Perl's destructor CV. */ - static boolean isDestroyCodeSlotKey(String key) { - return key != null && (key.endsWith("::DESTROY") || key.equals("DESTROY")); - } - static T markPackageGlobalRoot(T root) { if (root == null) return null; root.isPackageGlobalRoot = true; diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java index d0b587300..aa42a0472 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java @@ -80,8 +80,9 @@ private static boolean mightBeInteger(String s) { /** * When this scalar is installed in {@link GlobalVariable#globalCodeRefs}, the map key - * (fully-qualified name such as {@code My::Pkg::foo}). Used to invalidate DESTROY - * method-resolution caches after in-place CV installation via {@link #set(RuntimeScalar)}. + * (fully-qualified name such as {@code My::Pkg::foo}). Used to invalidate + * method-resolution cache lines for that sub's leaf name after in-place CV updates + * via {@link #set(RuntimeScalar)}. */ public String globalCodeRefFqn; @@ -1252,6 +1253,9 @@ private RuntimeScalar setLargeRefCounted(RuntimeScalar value) { ScalarRefRegistry.registerRef(this); } + int preAssignType = this.type; + Object preAssignValue = this.value; + // Do the assignment this.type = value.type; this.value = value.value; @@ -1405,9 +1409,15 @@ private RuntimeScalar setLargeRefCounted(RuntimeScalar value) { DestroyDispatch.clearRescuedWeakRefsTo(oldBase); } - if (isPackageGlobalRoot && type == CODE && globalCodeRefFqn != null - && GlobalVariable.isDestroyCodeSlotKey(globalCodeRefFqn)) { - InheritanceResolver.invalidateDestroyMethodLookupCaches(); + if (isPackageGlobalRoot && globalCodeRefFqn != null) { + boolean invalidate = preAssignType != this.type || preAssignValue != this.value; + if (!invalidate && this.type == CODE && this.value instanceof RuntimeCode nc + && preAssignType == CODE && preAssignValue instanceof RuntimeCode oc) { + invalidate = oc.defined() != nc.defined(); + } + if (invalidate) { + InheritanceResolver.invalidateMethodLookupCachesForStashSubKey(globalCodeRefFqn); + } } return this; From 7bf55a1724526ffaea71784a0e7d1eec692c85c7 Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Fri, 15 May 2026 13:38:16 +0200 Subject: [PATCH 4/4] fix(runtime): narrow refCount -1 promotion in container stores to hashes/arrays incrementRefCountForContainerStore must only bootstrap RuntimeHash/RuntimeArray slots from refCount=-1; promoting other referents (e.g. weak_ref targets that are refs to readonly scalars) regressed Moo's t/accessor-weaken.t. PPI nested {children} containers remain covered. Generated with [Cursor](https://cursor.com/docs) Co-Authored-By: Cursor Co-authored-by: Cursor --- .../org/perlonjava/runtime/runtimetypes/RuntimeScalar.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java index aa42a0472..f407b2087 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java @@ -964,7 +964,13 @@ public static void incrementRefCountForContainerStore(RuntimeScalar scalar) { // bless {}'s retroactive pass and normal container stores must still count // the hash slot as a strong ref, otherwise delete $h{k} / hash clear never // drops the child (PPI::Node->{children}; cpan PPI t/04_element.t). + // Restrict promotion to real containers: doing this for arbitrary -1 + // referents (e.g. refs to readonly scalars) breaks Moo weak_ref accessors + // (cpan Moo t/accessor-weaken.t). if (base.refCount < 0) { + if (!(base instanceof RuntimeHash) && !(base instanceof RuntimeArray)) { + return; + } base.refCount = 0; } base.refCount++;