fix(runtime): PPI DESTROY/refcount wiring + PPI future-work doc

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
 

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`

src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java

@@ -184,6 +184,35 @@ public static void invalidateCache() {
         DestroyDispatch.invalidateCache();
     }
 
+    /**
+     * Drop method-resolution cache entries that depend on a given package sub's
+     * <em>leaf</em> name (stash key {@code My::Pkg::foo} &rarr; {@code foo}), including
+     * {@code \0noautoload} variants. Also clears {@link DestroyDispatch} bookkeeping
+     * when the leaf is {@code DESTROY}.
+     */
+    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(suffixNoAutoload)
+                    || k.endsWith(suffix)
+                    || k.equals(leaf)
+                    || k.equals(leaf + "\0noautoload");
+        });
+        if ("DESTROY".equals(leaf)) {
+            DestroyDispatch.invalidateCache();
+        }
+    }
+
     /**
      * Retrieves a cached OverloadContext for the given blessing ID.
      *
@@ -429,7 +458,9 @@ public static RuntimeScalar findMethodInHierarchy(String methodName, String perl
             }
         }
 
-        // Cache the fact that method was not found (using null)
+        // 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;
     }

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.

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<String, Boolean> isSubs = new HashMap<>();
-    public static final Map<String, RuntimeScalar> globalCodeRefs = new PackageRootMap<>();
+    public static final Map<String, RuntimeScalar> globalCodeRefs = new GlobalCodeRefMap();
     static final Map<String, RuntimeGlob> globalIORefs = new HashMap<>();
     static final Map<String, RuntimeFormat> globalFormatRefs = new HashMap<>();
 
@@ -113,6 +113,75 @@ public void clear() {
         }
     }
 
+    /**
+     * 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<String, RuntimeScalar> {
+        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 old = super.put(key, value);
+            invalidatePackageRootSnapshot();
+            if (old != null && old != value) {
+                old.globalCodeRefFqn = null;
+            }
+            if (value != null) {
+                value.globalCodeRefFqn = key;
+            }
+            maybeInvalidateMethodCacheForCodeRefPut(key, old, value);
+            return old;
+        }
+
+        @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) {
+                InheritanceResolver.invalidateMethodLookupCachesForStashSubKey(s);
+            }
+            return prev;
+        }
+
+        @Override
+        public void clear() {
+            if (!isEmpty()) {
+                for (RuntimeScalar s : values()) {
+                    if (s != null) {
+                        s.globalCodeRefFqn = null;
+                    }
+                }
+                invalidatePackageRootSnapshot();
+            }
+            super.clear();
+        }
+    }
+
     static <T extends RuntimeBase> T markPackageGlobalRoot(T root) {
         if (root == null) return null;
         root.isPackageGlobalRoot = true;

src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java

@@ -78,6 +78,14 @@ 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
+     * method-resolution cache lines for that sub's leaf name after in-place CV updates
+     * via {@link #set(RuntimeScalar)}.
+     */
+    public String globalCodeRefFqn;
+
     /**
      * Number of closures that have captured this RuntimeScalar variable.
      * When {@code captureCount > 0}, {@link #scopeExitCleanup} skips the
@@ -942,19 +950,38 @@ 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).
+        // 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++;
+        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) {
@@ -1232,6 +1259,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;
@@ -1385,6 +1415,17 @@ private RuntimeScalar setLargeRefCounted(RuntimeScalar value) {
             DestroyDispatch.clearRescuedWeakRefsTo(oldBase);
         }
 
+        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;
     }