From b821a2e695935d6896cbc7b1da4ad0ac2b095b28 Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Thu, 28 May 2026 13:06:12 +0200 Subject: [PATCH 1/3] fix: preserve weak future lifetimes Fix reference ownership and closure capture cases exposed by Test::Future::Deferred. Generated with [Codex](https://openai.com/codex) Co-Authored-By: Codex --- .../bytecode/OpcodeHandlerExtended.java | 11 +- .../bytecode/VariableCollectorVisitor.java | 117 +++++++++++++-- .../backend/jvm/EmitterMethodCreator.java | 5 +- .../perlonjava/backend/jvm/JavaClassInfo.java | 7 + .../frontend/astnode/SubroutineNode.java | 4 +- .../frontend/parser/SubroutineParser.java | 15 +- .../runtime/perlmodule/ListUtil.java | 30 ++-- .../runtimetypes/AutovivificationArray.java | 3 +- .../runtimetypes/AutovivificationHash.java | 1 + .../runtime/runtimetypes/MortalList.java | 50 +++++- .../runtimetypes/ReachabilityWalker.java | 20 ++- .../runtime/runtimetypes/RuntimeBase.java | 27 ++++ .../runtime/runtimetypes/RuntimeCode.java | 142 ++++++++++++++---- .../runtime/runtimetypes/RuntimeHash.java | 19 ++- .../runtime/runtimetypes/RuntimeList.java | 8 +- .../runtime/runtimetypes/RuntimeScalar.java | 105 ++++++++++++- src/test/resources/unit/overload/stringify.t | 9 ++ .../captured_scalar_return_and_weak_slot.t | 41 +++++ ...closure_callback_unblessed_guard_release.t | 33 ++++ .../closure_decl_shadow_weak_capture.t | 25 +++ .../list_util_pair_helpers_refcount.t | 42 ++++++ .../returned_owned_scalar_arg_chain_weak.t | 60 ++++++++ .../sub_quote_overloaded_code_return.t | 27 ++++ 23 files changed, 729 insertions(+), 72 deletions(-) create mode 100644 src/test/resources/unit/refcount/captured_scalar_return_and_weak_slot.t create mode 100644 src/test/resources/unit/refcount/closure_callback_unblessed_guard_release.t create mode 100644 src/test/resources/unit/refcount/closure_decl_shadow_weak_capture.t create mode 100644 src/test/resources/unit/refcount/list_util_pair_helpers_refcount.t create mode 100644 src/test/resources/unit/refcount/returned_owned_scalar_arg_chain_weak.t create mode 100644 src/test/resources/unit/refcount/sub_quote_overloaded_code_return.t diff --git a/src/main/java/org/perlonjava/backend/bytecode/OpcodeHandlerExtended.java b/src/main/java/org/perlonjava/backend/bytecode/OpcodeHandlerExtended.java index 39d7dc1ca..92901b5b1 100644 --- a/src/main/java/org/perlonjava/backend/bytecode/OpcodeHandlerExtended.java +++ b/src/main/java/org/perlonjava/backend/bytecode/OpcodeHandlerExtended.java @@ -946,14 +946,23 @@ public static int executeCreateClosure(int[] bytecode, int pc, RuntimeBase[] reg // Without this, scopeExitCleanup() doesn't know the variable is still alive // via this closure, and may prematurely clear weak references to its value. java.util.List capturedScalars = new java.util.ArrayList<>(); + java.util.List capturedAggregates = new java.util.ArrayList<>(); for (RuntimeBase captured : capturedVars) { if (captured instanceof RuntimeScalar s) { capturedScalars.add(s); - s.captureCount++; + s.retainClosureCapture(); + } else if (captured instanceof RuntimeArray || captured instanceof RuntimeHash) { + capturedAggregates.add(captured); + captured.retainClosureCapture(); } } if (!capturedScalars.isEmpty()) { closureCode.capturedScalars = capturedScalars.toArray(new RuntimeScalar[0]); + } + if (!capturedAggregates.isEmpty()) { + closureCode.capturedAggregates = capturedAggregates.toArray(new RuntimeBase[0]); + } + if (!capturedScalars.isEmpty() || !capturedAggregates.isEmpty()) { closureCode.refCount = 0; } diff --git a/src/main/java/org/perlonjava/backend/bytecode/VariableCollectorVisitor.java b/src/main/java/org/perlonjava/backend/bytecode/VariableCollectorVisitor.java index e560f9de8..d69cc0fe6 100644 --- a/src/main/java/org/perlonjava/backend/bytecode/VariableCollectorVisitor.java +++ b/src/main/java/org/perlonjava/backend/bytecode/VariableCollectorVisitor.java @@ -3,6 +3,9 @@ import org.perlonjava.frontend.analysis.Visitor; import org.perlonjava.frontend.astnode.*; +import java.util.ArrayDeque; +import java.util.HashSet; +import java.util.Deque; import java.util.Set; /** @@ -18,6 +21,7 @@ public class VariableCollectorVisitor implements Visitor { private final Set variables; private boolean hasEvalString = false; + private final Deque> localScopes = new ArrayDeque<>(); /** * Create a new VariableCollectorVisitor. @@ -26,6 +30,7 @@ public class VariableCollectorVisitor implements Visitor { */ public VariableCollectorVisitor(Set variables) { this.variables = variables; + this.localScopes.push(new HashSet<>()); } /** @@ -37,6 +42,71 @@ public boolean hasEvalString() { return hasEvalString; } + private boolean isDeclarationOperator(String op) { + return op.equals("my") || op.equals("state") || op.equals("our"); + } + + private boolean isVariableOperator(String op) { + return op.equals("$") || op.equals("@") || op.equals("%") || op.equals("&"); + } + + private boolean isDeclaredLocal(String varName) { + for (Set scope : localScopes) { + if (scope.contains(varName)) return true; + } + return false; + } + + private void declare(String varName) { + localScopes.peek().add(varName); + } + + private void declareFrom(Node node) { + if (node == null) return; + if (node instanceof OperatorNode opNode) { + if (isDeclarationOperator(opNode.operator)) { + declareFrom(opNode.operand); + } else if (isVariableOperator(opNode.operator) + && opNode.operand instanceof IdentifierNode idNode) { + declare(opNode.operator + idNode.name); + } else { + declareFrom(opNode.operand); + } + } else if (node instanceof ListNode listNode && listNode.elements != null) { + for (Node element : listNode.elements) { + declareFrom(element); + } + } + } + + private boolean containsDeclaration(Node node) { + if (node == null) return false; + if (node instanceof OperatorNode opNode) { + return isDeclarationOperator(opNode.operator) || containsDeclaration(opNode.operand); + } + if (node instanceof ListNode listNode && listNode.elements != null) { + for (Node element : listNode.elements) { + if (containsDeclaration(element)) return true; + } + } + return false; + } + + private void visitAssignmentTarget(Node node) { + if (node == null) return; + if (node instanceof OperatorNode opNode && isDeclarationOperator(opNode.operator)) { + declareFrom(opNode.operand); + return; + } + if (node instanceof ListNode listNode && listNode.elements != null) { + for (Node element : listNode.elements) { + visitAssignmentTarget(element); + } + return; + } + node.accept(this); + } + @Override public void visit(IdentifierNode node) { // Leaf node - nothing to traverse @@ -54,16 +124,25 @@ public void visit(OperatorNode node) { } // Check if this is a variable reference (sigil + identifier) - if ((op.equals("$") || op.equals("@") || op.equals("%") || op.equals("&")) - && node.operand instanceof IdentifierNode idNode) { + if (isDeclarationOperator(op)) { + declareFrom(node.operand); + return; + } + + if (isVariableOperator(op) && node.operand instanceof IdentifierNode idNode) { // This is a variable reference String varName = op + idNode.name; - variables.add(varName); + if (!isDeclaredLocal(varName)) { + variables.add(varName); + } } // $#arr references @arr (array last index) if (op.equals("$#") && node.operand instanceof IdentifierNode idNode) { - variables.add("@" + idNode.name); + String varName = "@" + idNode.name; + if (!isDeclaredLocal(varName)) { + variables.add(varName); + } } // Visit operand if it exists @@ -74,17 +153,31 @@ public void visit(OperatorNode node) { @Override public void visit(BinaryOperatorNode node) { + if ("=".equals(node.operator) && containsDeclaration(node.left)) { + if (node.right != null) { + node.right.accept(this); + } + visitAssignmentTarget(node.left); + return; + } + // $a{key}, @a{keys}, %a{keys} all access hash %a if ("{".equals(node.operator) && node.left instanceof OperatorNode leftOp && ("$".equals(leftOp.operator) || "@".equals(leftOp.operator) || "%".equals(leftOp.operator)) && leftOp.operand instanceof IdentifierNode idNode) { - variables.add("%" + idNode.name); + String varName = "%" + idNode.name; + if (!isDeclaredLocal(varName)) { + variables.add(varName); + } } // $a[idx], @a[indices], %a[indices] all access array @a if ("[".equals(node.operator) && node.left instanceof OperatorNode leftOp && ("$".equals(leftOp.operator) || "@".equals(leftOp.operator) || "%".equals(leftOp.operator)) && leftOp.operand instanceof IdentifierNode idNode) { - variables.add("@" + idNode.name); + String varName = "@" + idNode.name; + if (!isDeclaredLocal(varName)) { + variables.add(varName); + } } if (node.left != null) { node.left.accept(this); @@ -96,6 +189,7 @@ public void visit(BinaryOperatorNode node) { @Override public void visit(BlockNode node) { + localScopes.push(new HashSet<>()); if (node.elements != null) { for (Node element : node.elements) { if (element != null) { @@ -103,6 +197,7 @@ public void visit(BlockNode node) { } } } + localScopes.pop(); } @Override @@ -153,22 +248,25 @@ public void visit(StringNode node) { @Override public void visit(For1Node node) { - if (node.variable != null) { - node.variable.accept(this); - } + localScopes.push(new HashSet<>()); if (node.list != null) { node.list.accept(this); } + if (node.variable != null) { + visitAssignmentTarget(node.variable); + } if (node.body != null) { node.body.accept(this); } if (node.continueBlock != null) { node.continueBlock.accept(this); } + localScopes.pop(); } @Override public void visit(For3Node node) { + localScopes.push(new HashSet<>()); if (node.initialization != null) { node.initialization.accept(this); } @@ -189,6 +287,7 @@ public void visit(For3Node node) { if (node.continueBlock != null) { node.continueBlock.accept(this); } + localScopes.pop(); } @Override diff --git a/src/main/java/org/perlonjava/backend/jvm/EmitterMethodCreator.java b/src/main/java/org/perlonjava/backend/jvm/EmitterMethodCreator.java index fedae29dc..c0f6e53fd 100644 --- a/src/main/java/org/perlonjava/backend/jvm/EmitterMethodCreator.java +++ b/src/main/java/org/perlonjava/backend/jvm/EmitterMethodCreator.java @@ -607,6 +607,8 @@ private static byte[] getBytecodeInternal(EmitterContext ctx, Node ast, boolean // or can use a minimal null-store fast path. See CleanupNeededVisitor // and JavaClassInfo.cleanupNeeded. JPERL_FORCE_CLEANUP=1 bypasses the // analysis (forces cleanupNeeded=true) as an escape hatch. + ctx.javaClassInfo.isLvalueSubroutine = + Boolean.TRUE.equals(ast.getAnnotation("subroutineIsLvalue")); if (FORCE_CLEANUP) { ctx.javaClassInfo.cleanupNeeded = true; } else { @@ -781,10 +783,11 @@ private static byte[] getBytecodeInternal(EmitterContext ctx, Node ast, boolean // Materialize it into a local slot immediately so all subsequent control-flow // checks operate from locals and join points don't depend on operand stack shape. mv.visitVarInsn(Opcodes.ILOAD, 2); + mv.visitInsn(ctx.javaClassInfo.isLvalueSubroutine ? Opcodes.ICONST_0 : Opcodes.ICONST_1); mv.visitMethodInsn(Opcodes.INVOKESTATIC, "org/perlonjava/runtime/runtimetypes/RuntimeCode", "returnList", - "(Lorg/perlonjava/runtime/runtimetypes/RuntimeBase;I)Lorg/perlonjava/runtime/runtimetypes/RuntimeList;", + "(Lorg/perlonjava/runtime/runtimetypes/RuntimeBase;IZ)Lorg/perlonjava/runtime/runtimetypes/RuntimeList;", false); mv.visitVarInsn(Opcodes.ASTORE, returnListSlot); diff --git a/src/main/java/org/perlonjava/backend/jvm/JavaClassInfo.java b/src/main/java/org/perlonjava/backend/jvm/JavaClassInfo.java index ac4f5d374..55d275440 100644 --- a/src/main/java/org/perlonjava/backend/jvm/JavaClassInfo.java +++ b/src/main/java/org/perlonjava/backend/jvm/JavaClassInfo.java @@ -90,6 +90,13 @@ public class JavaClassInfo { */ public boolean isMapGrepBlock; + /** + * True when this generated method belongs to a subroutine with the + * {@code :lvalue} attribute. Non-lvalue return copying must be disabled + * for these subs so callers can assign through returned aliases. + */ + public boolean isLvalueSubroutine; + /** * Counter tracking nesting depth inside finally blocks. * Control flow statements (last, next, redo, return, goto) are prohibited in finally blocks. diff --git a/src/main/java/org/perlonjava/frontend/astnode/SubroutineNode.java b/src/main/java/org/perlonjava/frontend/astnode/SubroutineNode.java index 777df127d..e37225dc3 100644 --- a/src/main/java/org/perlonjava/frontend/astnode/SubroutineNode.java +++ b/src/main/java/org/perlonjava/frontend/astnode/SubroutineNode.java @@ -44,6 +44,9 @@ public SubroutineNode(String name, String prototype, List attributes, No if (block instanceof AbstractNode abstractNode) { abstractNode.setAnnotation("blockIsSubroutine", true); + if (attributes != null && attributes.contains("lvalue")) { + abstractNode.setAnnotation("subroutineIsLvalue", true); + } } } @@ -60,4 +63,3 @@ public void accept(Visitor visitor) { visitor.visit(this); } } - diff --git a/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java b/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java index a2b1060d8..ab255fb6a 100644 --- a/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java +++ b/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java @@ -1469,6 +1469,9 @@ public static ListNode handleNamedSubWithFilter(Parser parser, String subName, S Supplier subroutineCreationTaskSupplier = () -> { // Try unified API (returns RuntimeCode - either CompiledCode or InterpretedCode) + if (placeholder.attributes != null && placeholder.attributes.contains("lvalue")) { + block.setAnnotation("subroutineIsLvalue", true); + } RuntimeCode runtimeCode = EmitterMethodCreator.createRuntimeCode(newCtx, block, false); @@ -1599,14 +1602,24 @@ private static void installClosureCaptureMetadata(RuntimeCode code, List } ArrayList capturedScalars = new ArrayList<>(); + ArrayList capturedAggregates = new ArrayList<>(); for (Object value : capturedValues) { if (value instanceof RuntimeScalar scalar) { capturedScalars.add(scalar); - scalar.captureCount++; + scalar.retainClosureCapture(); + } else if (value instanceof RuntimeArray || value instanceof RuntimeHash) { + RuntimeBase aggregate = (RuntimeBase) value; + capturedAggregates.add(aggregate); + aggregate.retainClosureCapture(); } } if (!capturedScalars.isEmpty()) { code.capturedScalars = capturedScalars.toArray(new RuntimeScalar[0]); + } + if (!capturedAggregates.isEmpty()) { + code.capturedAggregates = capturedAggregates.toArray(new RuntimeBase[0]); + } + if (!capturedScalars.isEmpty() || !capturedAggregates.isEmpty()) { code.refCount = 0; } } diff --git a/src/main/java/org/perlonjava/runtime/perlmodule/ListUtil.java b/src/main/java/org/perlonjava/runtime/perlmodule/ListUtil.java index 669c69c8f..aed551ebc 100644 --- a/src/main/java/org/perlonjava/runtime/perlmodule/ListUtil.java +++ b/src/main/java/org/perlonjava/runtime/perlmodule/ListUtil.java @@ -708,7 +708,7 @@ public static RuntimeList tail(RuntimeArray args, int ctx) { * they provide ->key / ->value / ->TO_JSON methods. */ public static RuntimeList pairs(RuntimeArray args, int ctx) { - RuntimeArray result = new RuntimeArray(); + RuntimeList result = new RuntimeList(); RuntimeScalar pairClass = new RuntimeScalar("List::Util::_Pair"); for (int i = 0; i < args.size(); i += 2) { @@ -719,57 +719,57 @@ public static RuntimeList pairs(RuntimeArray args, int ctx) { } else { pair.push(scalarUndef); } - RuntimeScalar pairRef = pair.createReference(); + RuntimeScalar pairRef = pair.createAnonymousReference(); org.perlonjava.runtime.operators.ReferenceOperators.bless(pairRef, pairClass); - result.push(pairRef); + result.add(pairRef); } - return result.getList(); + return result; } /** * Flattens pairs back to a list. */ public static RuntimeList unpairs(RuntimeArray args, int ctx) { - RuntimeArray result = new RuntimeArray(); + RuntimeList result = new RuntimeList(); for (RuntimeScalar pairRef : args.elements) { if (pairRef.type == RuntimeScalarType.ARRAYREFERENCE) { RuntimeArray pair = (RuntimeArray) pairRef.value; // Always emit exactly two values (key, value), padding with undef // when the input arrayref has fewer than two elements. - result.push(pair.size() >= 1 ? pair.get(0) : scalarUndef); - result.push(pair.size() >= 2 ? pair.get(1) : scalarUndef); + result.add(pair.size() >= 1 ? pair.get(0) : scalarUndef); + result.add(pair.size() >= 2 ? pair.get(1) : scalarUndef); } } - return result.getList(); + return result; } /** * Returns the first values of each pair (keys). */ public static RuntimeList pairkeys(RuntimeArray args, int ctx) { - RuntimeArray result = new RuntimeArray(); + RuntimeList result = new RuntimeList(); for (int i = 0; i < args.size(); i += 2) { - result.push(args.get(i)); + result.add(args.get(i)); } - return result.getList(); + return result; } /** * Returns the second values of each pair (values). */ public static RuntimeList pairvalues(RuntimeArray args, int ctx) { - RuntimeArray result = new RuntimeArray(); + RuntimeList result = new RuntimeList(); for (int i = 1; i < args.size(); i += 2) { - result.push(args.get(i)); + result.add(args.get(i)); } - return result.getList(); + return result; } /** @@ -975,7 +975,7 @@ private static RuntimeList zipImpl(RuntimeArray args, int ctx, boolean shortest) tuple.push(RuntimeScalarCache.scalarUndef); } } - result.push(tuple.createReference()); + result.push(tuple.createAnonymousReference()); } return result.getList(); diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/AutovivificationArray.java b/src/main/java/org/perlonjava/runtime/runtimetypes/AutovivificationArray.java index 1a1af4d45..eaf1f9884 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/AutovivificationArray.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/AutovivificationArray.java @@ -43,6 +43,7 @@ public static void vivify(RuntimeArray array) { arrayProxy.scalarToAutovivify.value = array; arrayProxy.scalarToAutovivify.type = RuntimeScalarType.ARRAYREFERENCE; + RuntimeScalar.incrementRefCountForContainerStore(arrayProxy.scalarToAutovivify); } } @@ -79,4 +80,4 @@ static RuntimeArray createAutovivifiedArray(RuntimeScalar runtimeScalar) { // but will be autovivified to an array reference on first write operation. return newArray; } -} \ No newline at end of file +} diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/AutovivificationHash.java b/src/main/java/org/perlonjava/runtime/runtimetypes/AutovivificationHash.java index c99d86eb3..478d71742 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/AutovivificationHash.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/AutovivificationHash.java @@ -46,6 +46,7 @@ public static void vivify(RuntimeHash hash) { hashProxy.scalarToAutovivify.value = hash; hashProxy.scalarToAutovivify.type = HASHREFERENCE; + RuntimeScalar.incrementRefCountForContainerStore(hashProxy.scalarToAutovivify); } } diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/MortalList.java b/src/main/java/org/perlonjava/runtime/runtimetypes/MortalList.java index 172c6dadc..f58b27624 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/MortalList.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/MortalList.java @@ -57,8 +57,10 @@ public static void pushTemporaryRoot(RuntimeBase root) { public static void popTemporaryRoot(RuntimeBase root) { if (root != null) { ArrayDeque roots = temporaryRoots.get(); - if (!roots.isEmpty()) { + if (!roots.isEmpty() && roots.peek() == root) { roots.pop(); + } else { + roots.removeFirstOccurrence(root); } } } @@ -345,6 +347,10 @@ public static void scopeExitCleanupHash(RuntimeHash hash) { // variable no longer holds a strong reference. boolean hadLocalBinding = hash.localBindingExists; hash.localBindingExists = false; + if (hash.captureCount > 0) { + hash.scopeExited = true; + return; + } // Skip container walks only when there are NO blessed objects AND NO // weak refs anywhere in the JVM. If weak refs exist (even to unblessed // data), we must still cascade decrements so their weak-ref entries @@ -408,6 +414,10 @@ public static void scopeExitCleanupArray(RuntimeArray arr) { // or flush) to correctly trigger callDestroy, since the local // variable no longer holds a strong reference. arr.localBindingExists = false; + if (arr.captureCount > 0) { + arr.scopeExited = true; + return; + } // Skip container walks only when there are NO blessed objects AND NO // weak refs anywhere in the JVM (see scopeExitCleanupHash for details). if (!RuntimeBase.blessedObjectExists && !WeakRefRegistry.weakRefsExist) return; @@ -544,6 +554,20 @@ private static void deferDecrementRecursive(RuntimeScalar scalar) { } base.releaseActiveOwner(s); pending.add(base); + if (base.activeOwnerCount() > 0 + || (base.refCount > 1 + && !containerHasWeakElementRefs(base) + && isReachableFromExternalRootCached(base))) { + continue; + } + } else if (base.refCount > 0 + && (base.activeOwnerCount() > 0 + || (!containerHasWeakElementRefs(base) + && isReachableFromExternalRootCached(base)))) { + // This scalar is a non-owning copy of a still-live container. + // Cleaning it up must not walk into the shared container and + // release the original owner's children. + continue; } // Walk into unblessed containers to find nested blessed refs if (base instanceof RuntimeArray arr) { @@ -559,6 +583,23 @@ private static void deferDecrementRecursive(RuntimeScalar scalar) { } } + private static boolean containerHasWeakElementRefs(RuntimeBase base) { + if (base instanceof RuntimeArray arr) { + for (RuntimeScalar elem : arr.elements) { + if (elem != null && WeakRefRegistry.hasWeakRefsTo(elem)) { + return true; + } + } + } else if (base instanceof RuntimeHash hash) { + for (RuntimeScalar value : hash.elements.values()) { + if (value != null && WeakRefRegistry.hasWeakRefsTo(value)) { + return true; + } + } + } + return false; + } + /** * Mortal-ize blessed refs with refCount==0 in a RuntimeList that will be * discarded (void-context call result). Without this, objects that were @@ -695,6 +736,9 @@ private static boolean isReachableCached(RuntimeBase base) { } private static boolean isReachableFromExternalRootCached(RuntimeBase base) { + if (ReachabilityWalker.isReachableFromTemporaryRoots(base)) { + return true; + } if (externalRootSnapshot == null) { externalRootSnapshot = new ReachabilityWalker.ExternalRootSnapshot(); } @@ -786,7 +830,9 @@ private static void processDeferredBase(RuntimeBase base, boolean clearWeakRefsF } else if (base.blessId != 0 && hasWeakRefs && !blessedClassHasDestroy(base) - && isReachableFromExternalRootCached(base)) { + && (RuntimeCode.argsStackDepth() > 1 + || isReachableFromExternalRootCached(base) + || ReachabilityWalker.isReachableFromRoots(base))) { // A weakened probe copy can make the selective count reach // zero while an ordinary blessed object is still held by a // live lexical. Test::Refcount exercises this shape; clearing diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/ReachabilityWalker.java b/src/main/java/org/perlonjava/runtime/runtimetypes/ReachabilityWalker.java index 095ad9cfc..27d07cb88 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/ReachabilityWalker.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/ReachabilityWalker.java @@ -176,10 +176,12 @@ private void bfs(java.util.ArrayDeque todo, boolean walkCaptures) { if (cur instanceof RuntimeHash h) { if (h.elements instanceof HashSpecialVariable) continue; for (RuntimeScalar v : h.elements.values()) { + addReachable(v, todo); visitScalar(v, todo); } } else if (cur instanceof RuntimeArray a) { for (RuntimeScalar v : a.elements) { + addReachable(v, todo); visitScalar(v, todo); } } else if (cur instanceof RuntimeCode code) { @@ -212,14 +214,19 @@ private void visitCodeCaptures(RuntimeCode code, java.util.ArrayDeque todo) { if (code.capturedScalars != null) { for (RuntimeScalar cap : code.capturedScalars) { + addReachable(cap, todo); visitScalar(cap, todo); } } - visitReflectiveCodeScalars(code, cap -> visitScalar(cap, todo)); + visitReflectiveCodeScalars(code, cap -> { + addReachable(cap, todo); + visitScalar(cap, todo); + }); if (code instanceof org.perlonjava.backend.bytecode.InterpretedCode interpreted && interpreted.capturedVars != null) { for (RuntimeBase cap : interpreted.capturedVars) { if (cap instanceof RuntimeScalar scalar) { + addReachable(scalar, todo); visitScalar(scalar, todo); } else if (cap != null) { addReachable(cap, todo); @@ -850,6 +857,17 @@ public static boolean isReachableFromExternalRoot(RuntimeBase target) { return new ExternalRootSnapshot().isReachable(target); } + public static boolean isReachableFromTemporaryRoots(RuntimeBase target) { + if (target == null) return false; + ReachabilityWalker walker = new ReachabilityWalker(); + java.util.ArrayDeque todo = new java.util.ArrayDeque<>(); + for (RuntimeBase tempRoot : MortalList.snapshotTemporaryRoots()) { + walker.addReachable(tempRoot, todo); + } + walker.bfs(todo, true); + return walker.reachable.contains(target); + } + /** * Target-specific reachability query for scope-exit cleanup of a named * lexical container. The container's own my-variable slot is still present diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeBase.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeBase.java index c620d3724..ddbba6b81 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeBase.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeBase.java @@ -63,6 +63,33 @@ public abstract class RuntimeBase implements DynamicState, Iterable 0) { + captureCount--; + } + if (captureCount == 0 && scopeExited) { + scopeExited = false; + if (this instanceof RuntimeHash hash) { + MortalList.scopeExitCleanupHash(hash); + } else if (this instanceof RuntimeArray array) { + MortalList.scopeExitCleanupArray(array); + } + } + } + // ───────────────────────────────────────────────────────────────────── // D-W6.13: production-grade ownership tracking. // Active for blessed objects that have at least one weak reference diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java index bd448fefd..089bea0c2 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java @@ -392,6 +392,10 @@ public static int effectiveCallContext(int callContext) { } public static RuntimeList returnList(RuntimeBase retVal, int callContext) { + return returnList(retVal, callContext, true); + } + + public static RuntimeList returnList(RuntimeBase retVal, int callContext, boolean copyReferenceScalars) { if (retVal == null) { return new RuntimeList(); } @@ -401,7 +405,15 @@ public static RuntimeList returnList(RuntimeBase retVal, int callContext) { result.add(retVal); return result; } - return retVal.getList(); + RuntimeList result = retVal.getList(); + if (copyReferenceScalars && callContext == RuntimeContextType.LIST) { + RuntimeList copied = copyReturnedReferenceScalars(result, callContext, true); + if (copied != result) { + MortalList.pushTemporaryRoot(copied); + } + return copied; + } + return result; } /** @@ -413,22 +425,63 @@ public static RuntimeList returnList(RuntimeBase retVal, int callContext) { * tear down shared JDBC state before the outer method ran. */ public static RuntimeList coerceScalarCallResult(RuntimeList result, int effectiveContext) { - if (result == null) { - return null; + return coerceScalarCallResult(result, effectiveContext, effectiveContext, true); + } + + public static RuntimeList coerceScalarCallResult(RuntimeList result, int effectiveContext, int originalContext) { + return coerceScalarCallResult(result, effectiveContext, originalContext, true); + } + + public static RuntimeList coerceScalarCallResult(RuntimeList result, int effectiveContext, + int originalContext, boolean copyCapturedScalars) { + try { + if (result == null) { + return null; + } + if (result instanceof RuntimeControlFlowList) { + return result; + } + if (effectiveContext == RuntimeContextType.SCALAR && result.elements.size() > 1) { + return copyReturnedReferenceScalars(new RuntimeList(result.scalar()), originalContext, copyCapturedScalars); + } + if (effectiveContext == RuntimeContextType.SCALAR && result.elements.size() == 1) { + RuntimeBase value = result.elements.getFirst(); + if (value instanceof RuntimeScalar scalar && scalar.type == RuntimeScalarType.TIED_SCALAR) { + return copyReturnedReferenceScalars(new RuntimeList(scalar.tiedFetch()), originalContext, copyCapturedScalars); + } + } + return copyReturnedReferenceScalars(copyReadonlyListReturns(result, effectiveContext), + originalContext, copyCapturedScalars); + } finally { + MortalList.popTemporaryRoot(result); } - if (result instanceof RuntimeControlFlowList) { + } + + private static RuntimeList copyReturnedReferenceScalars(RuntimeList result, int originalContext, + boolean copyCapturedScalars) { + if (result == null + || result instanceof RuntimeControlFlowList + || !copyCapturedScalars + || originalContext == RuntimeContextType.LVALUE + || originalContext == RuntimeContextType.LVALUE_LIST) { return result; } - if (effectiveContext == RuntimeContextType.SCALAR && result.elements.size() > 1) { - return new RuntimeList(result.scalar()); - } - if (effectiveContext == RuntimeContextType.SCALAR && result.elements.size() == 1) { - RuntimeBase value = result.elements.getFirst(); - if (value instanceof RuntimeScalar scalar && scalar.type == RuntimeScalarType.TIED_SCALAR) { - return new RuntimeList(scalar.tiedFetch()); + for (RuntimeBase value : result.elements) { + if (value instanceof RuntimeScalar scalar + && !isCodeScalar(scalar) + && (RuntimeScalarType.isReference(scalar) || scalar.captureCount > 0)) { + return result.cloneScalars(); } } - return copyReadonlyListReturns(result, effectiveContext); + return result; + } + + private static boolean isCodeScalar(RuntimeScalar scalar) { + RuntimeScalar current = scalar; + while (current != null && current.type == RuntimeScalarType.READONLY_SCALAR) { + current = (RuntimeScalar) current.value; + } + return current != null && current.type == RuntimeScalarType.CODE; } private static RuntimeList copyReadonlyListReturns(RuntimeList result, int effectiveContext) { @@ -695,6 +748,12 @@ private void exitCall() { */ public RuntimeScalar[] capturedScalars; + /** + * Captured aggregate lexicals (arrays/hashes). Their element cleanup is + * deferred while the closure keeps the aggregate alive. + */ + public RuntimeBase[] capturedAggregates; + /** * Tracks the number of stash (glob) entries that reference this CODE object. * Stash entries created via {@code *Foo::bar = $coderef} are invisible to the @@ -750,7 +809,7 @@ public void releaseCaptures() { RuntimeScalar[] scalars = capturedScalars; capturedScalars = null; // null out first to prevent re-entry for (RuntimeScalar s : scalars) { - s.captureCount--; + s.releaseClosureCapture(); if (s.captureCount == 0) { // If the captured scalar itself holds a CODE ref with captures, // release those recursively (handles nested closures). @@ -763,13 +822,10 @@ public void releaseCaptures() { // closure is alive). Now that the last closure is releasing this // capture, decrement refCount to balance the original increment. // - // Only cascade for BLESSED referents. For unblessed containers - // (arrays, hashes), the selective refCount from releaseCaptures - // can falsely reach 0 (because closure captures hold JVM references - // not counted in refCount). Cascading to callDestroy for such - // containers would clear weak references prematurely, breaking - // Sub::Defer/Moo's %DEFERRED and %QUOTED weak ref tables. - // The JVM GC handles truly-dead unblessed containers eventually. + // Balance the captured lexical's owned reference now that the + // last closure is gone. MortalList handles the weak-ref rescue + // cases for unblessed containers whose selective refCount can + // transiently reach zero while still reachable through live CODE. if (s.scopeExited) { if (s.type == RuntimeScalarType.TIED_SCALAR && s.value instanceof TiedVariableBase tiedVariable) { @@ -779,7 +835,7 @@ public void releaseCaptures() { } if ((s.type & RuntimeScalarType.REFERENCE_BIT) != 0 && s.value instanceof RuntimeBase rb - && rb.blessId != 0) { + && (rb.blessId != 0 || !WeakRefRegistry.hasWeakRefsTo(rb))) { MortalList.releaseCapturedDecrement(s); } MortalList.noteDeferredCaptureMayBeReady(); @@ -787,6 +843,15 @@ public void releaseCaptures() { } } } + if (capturedAggregates != null) { + RuntimeBase[] aggregates = capturedAggregates; + capturedAggregates = null; + for (RuntimeBase aggregate : aggregates) { + if (aggregate != null) { + aggregate.releaseClosureCapture(); + } + } + } } /** @@ -2295,17 +2360,29 @@ public static RuntimeScalar makeCodeObject( // can decrement blessed ref refCounts when the closure is discarded. Field[] allFields = clazz.getDeclaredFields(); List captured = new ArrayList<>(); + List capturedAggregates = new ArrayList<>(); for (Field f : allFields) { if (f.getType() == RuntimeScalar.class && !"__SUB__".equals(f.getName())) { RuntimeScalar capturedVar = (RuntimeScalar) f.get(codeObject); if (capturedVar != null) { captured.add(capturedVar); - capturedVar.captureCount++; + capturedVar.retainClosureCapture(); + } + } else if (f.getType() == RuntimeArray.class || f.getType() == RuntimeHash.class) { + RuntimeBase capturedAggregate = (RuntimeBase) f.get(codeObject); + if (capturedAggregate != null) { + capturedAggregates.add(capturedAggregate); + capturedAggregate.retainClosureCapture(); } } } if (!captured.isEmpty()) { code.capturedScalars = captured.toArray(new RuntimeScalar[0]); + } + if (!capturedAggregates.isEmpty()) { + code.capturedAggregates = capturedAggregates.toArray(new RuntimeBase[0]); + } + if (!captured.isEmpty() || !capturedAggregates.isEmpty()) { // Enable refCount tracking for closures with captures. // When the CODE ref's refCount drops to 0, releaseCaptures() // fires (via DestroyDispatch.callDestroy), letting captured @@ -3357,15 +3434,16 @@ public static RuntimeList apply(RuntimeScalar runtimeScalar, RuntimeArray a, int MortalList.flushAboveMark(); return result; } - return coerceScalarCallResult(result, effectiveContext); + return coerceScalarCallResult(result, effectiveContext, callContext, !isLvalueCode(code)); } } catch (PerlNonLocalReturnException e) { // Non-local return from map/grep block if (code.isMapGrepBlock || code.isEvalBlock) { throw e; // Propagate through map/grep blocks and eval blocks } - // Consume at normal subroutine boundary - return e.returnValue != null ? e.returnValue.getList() : new RuntimeList(); + // Consume at normal subroutine boundary. + RuntimeList result = e.returnValue != null ? e.returnValue.getList() : new RuntimeList(); + return coerceScalarCallResult(result, effectiveContext, callContext, !isLvalueCode(code)); } catch (RuntimeException e) { // On die: run scopeExitCleanup for my-variables whose normal // SCOPE_EXIT_CLEANUP bytecodes were skipped by the exception. @@ -3640,8 +3718,9 @@ public static RuntimeList apply(RuntimeScalar runtimeScalar, String subroutineNa if (code.isMapGrepBlock || code.isEvalBlock) { throw e; // Propagate through map/grep blocks and eval blocks } - // Consume at normal subroutine boundary - return e.returnValue != null ? e.returnValue.getList() : new RuntimeList(); + // Consume at normal subroutine boundary. + RuntimeList result = e.returnValue != null ? e.returnValue.getList() : new RuntimeList(); + return coerceScalarCallResult(result, effectiveContext, callContext, !isLvalueCode(code)); } catch (RuntimeException e) { if (!(e instanceof PerlExitException)) { MyVarCleanupStack.unwindTo(cleanupMark); @@ -3888,8 +3967,9 @@ private static RuntimeList applyImpl(RuntimeScalar runtimeScalar, String subrout if (code.isMapGrepBlock || code.isEvalBlock) { throw e; // Propagate through map/grep blocks and eval blocks } - // Consume at normal subroutine boundary - return e.returnValue != null ? e.returnValue.getList() : new RuntimeList(); + // Consume at normal subroutine boundary. + RuntimeList result = e.returnValue != null ? e.returnValue.getList() : new RuntimeList(); + return coerceScalarCallResult(result, effectiveContext, callContext, !isLvalueCode(code)); } catch (RuntimeException e) { if (!(e instanceof PerlExitException)) { MyVarCleanupStack.unwindTo(cleanupMark); @@ -4346,7 +4426,7 @@ public RuntimeList apply(RuntimeArray a, int callContext) { } else { result = (RuntimeList) this.methodHandle.invoke(this.codeObject, a, effectiveContext); } - return coerceScalarCallResult(result, effectiveContext); + return coerceScalarCallResult(result, effectiveContext, callContext, !isLvalueCode(this)); } finally { if (warningBits != null) { WarningBitsRegistry.popCurrent(); @@ -4465,7 +4545,7 @@ public RuntimeList apply(String subroutineName, RuntimeArray a, int callContext) } else { result = (RuntimeList) this.methodHandle.invoke(this.codeObject, a, effectiveContext); } - return coerceScalarCallResult(result, effectiveContext); + return coerceScalarCallResult(result, effectiveContext, callContext, !isLvalueCode(this)); } finally { if (warningBits != null) { WarningBitsRegistry.popCurrent(); diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeHash.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeHash.java index c5c38ce4b..7f88eaffd 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeHash.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeHash.java @@ -203,9 +203,9 @@ public static RuntimeHash createHashForAssignment(RuntimeBase value) { && (firstType == RuntimeScalarType.HASHREFERENCE || firstType == RuntimeScalarType.ARRAYREFERENCE)) ? "Reference found where even-sized list expected" : "Odd number of elements in hash assignment"; - return createHashInternal(value, warning); + return createHashInternal(value, warning, true); } else { - return createHashNoWarn(value); + return createHashNoWarn(value, true); } } @@ -217,6 +217,11 @@ public static RuntimeHash createHashForAssignment(RuntimeBase value) { * @return A new RuntimeHash populated with the elements from the list. */ private static RuntimeHash createHashInternal(RuntimeBase value, String oddWarningMessage) { + return createHashInternal(value, oddWarningMessage, false); + } + + private static RuntimeHash createHashInternal(RuntimeBase value, String oddWarningMessage, + boolean trackValues) { RuntimeHash result = new RuntimeHash(); Map resultHash = result.elements; @@ -240,6 +245,9 @@ private static RuntimeHash createHashInternal(RuntimeBase value, String oddWarni String key = iterator.next().toString(); RuntimeScalar val = iterator.hasNext() ? new RuntimeScalar(iterator.next()) : new RuntimeScalar(); resultHash.put(key, val); + if (trackValues) { + RuntimeScalar.incrementRefCountForContainerStore(val); + } } return result; } @@ -251,6 +259,10 @@ private static RuntimeHash createHashInternal(RuntimeBase value, String oddWarni * @return A new RuntimeHash populated with the elements from the list. */ private static RuntimeHash createHashNoWarn(RuntimeBase value) { + return createHashNoWarn(value, false); + } + + private static RuntimeHash createHashNoWarn(RuntimeBase value, boolean trackValues) { RuntimeHash result = new RuntimeHash(); Map resultHash = result.elements; Iterator iterator = value.iterator(); @@ -258,6 +270,9 @@ private static RuntimeHash createHashNoWarn(RuntimeBase value) { String key = iterator.next().toString(); RuntimeScalar val = iterator.hasNext() ? new RuntimeScalar(iterator.next()) : new RuntimeScalar(); resultHash.put(key, val); + if (trackValues) { + RuntimeScalar.incrementRefCountForContainerStore(val); + } } return result; } diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeList.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeList.java index b3b034027..46d9c0900 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeList.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeList.java @@ -695,10 +695,10 @@ public RuntimeArray setFromList(RuntimeList value) { } // Undo materialized copies' refCount increments. // createHashForAssignment creates new RuntimeScalars for hash values - // (via createHashNoWarn's `new RuntimeScalar(iterator.next())`), which - // do NOT inherit refCountOwned. The original rhs elements' refCount - // increments (from materialization via addToArray → setLarge) are now - // redundant and would leak since nobody decrements them. + // and gives those hash slots their own container-store ownership. + // The original rhs elements' refCount increments (from materialization + // via addToArray -> setLarge) are now redundant and would leak since + // nobody decrements them. for (RuntimeScalar r : remainingArr.elements) { if (r.refCountOwned && (r.type & RuntimeScalarType.REFERENCE_BIT) != 0 && r.value instanceof RuntimeBase base && base.refCount > 0) { diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java index e48b6baad..4ec3ed0f1 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java @@ -103,6 +103,14 @@ private static boolean mightBeInteger(String s) { */ public int captureCount; + /** + * Number of closure-capture owners currently retained on this scalar's + * referent. This is separate from {@link #captureCount}: a weak scalar, an + * untracked referent, or a scalar that already owns a normal refCount slot + * may be captured without adding an extra selective refCount owner. + */ + public int captureRefCountOwned; + /** * True if {@link #scopeExitCleanup} has been called for this variable * (i.e., the variable's declaring scope has exited), but cleanup was @@ -114,6 +122,66 @@ private static boolean mightBeInteger(String s) { */ public boolean scopeExited; + public void retainClosureCapture() { + captureCount++; + } + + public void releaseClosureCapture() { + releaseOneClosureCaptureReferent(); + if (captureCount > 0) { + captureCount--; + } + } + + private void retainClosureCaptureReferent() { + RuntimeBase base = closureCaptureReferent(); + if (base == null) return; + base.traceRefCount(+1, "RuntimeScalar.retainClosureCaptureReferent"); + base.refCount++; + base.hadCountedReference = true; + captureRefCountOwned++; + } + + private void retainMissingClosureCaptureReferents() { + while (captureRefCountOwned < captureCount) { + int before = captureRefCountOwned; + retainClosureCaptureReferent(); + if (captureRefCountOwned == before) break; + } + } + + private RuntimeBase closureCaptureReferent() { + if (WeakRefRegistry.isweak(this)) return null; + if ((type & RuntimeScalarType.REFERENCE_BIT) == 0 || !(value instanceof RuntimeBase base)) { + return null; + } + if (refCountOwned && !(base instanceof RuntimeCode)) return null; + if (base.refCount < 0 || base.refCount == WeakRefRegistry.WEAKLY_TRACKED + || base.refCount == Integer.MIN_VALUE) { + return null; + } + return base; + } + + private void releaseOneClosureCaptureReferent() { + if (captureRefCountOwned <= 0) return; + if ((type & RuntimeScalarType.REFERENCE_BIT) != 0 + && value instanceof RuntimeBase base + && base.refCount > 0) { + MortalList.deferDecrement(base); + } + captureRefCountOwned--; + } + + private void releaseAllClosureCaptureReferents(RuntimeBase oldBase) { + while (captureRefCountOwned > 0) { + if (oldBase != null && oldBase.refCount > 0) { + MortalList.deferDecrement(oldBase); + } + captureRefCountOwned--; + } + } + /** * True if this scalar "owns" a refCount increment on its referent. * Set to true by {@link #setLarge} after incrementing the referent's refCount. @@ -1173,7 +1241,8 @@ private RuntimeScalar setLargeRefCounted(RuntimeScalar value) { boolean scalarRefContentTrackingNeeded = this.ownsScalarReferenceContents || scalarReferenceContentsNeedRetain(value); if (!scalarRefContentTrackingNeeded - && !this.refCountOwned && this.type != GLOBREFERENCE && value.type != GLOBREFERENCE) { + && !this.refCountOwned && this.captureCount == 0 && this.captureRefCountOwned == 0 + && this.type != GLOBREFERENCE && value.type != GLOBREFERENCE) { // Both old and new are non-GLOB references. Check if referents are untracked. boolean oldUntracked = (this.type & REFERENCE_BIT) == 0 || this.value == null @@ -1275,6 +1344,9 @@ private RuntimeScalar setLargeRefCounted(RuntimeScalar value) { && oldBase.blessId != 0 && WeakRefRegistry.weakRefsExist && blessedClassHasDestroy(oldBase); + if (oldBase != null && this.captureRefCountOwned > 0) { + releaseAllClosureCaptureReferents(oldBase); + } // Increment new value's refCount for tracked stores. RuntimeHash/RuntimeArray // at refCount==-1 are promoted to 0 here (aligned with @@ -1441,6 +1513,7 @@ private RuntimeScalar setLargeRefCounted(RuntimeScalar value) { // Update ownership: this scalar now owns a refCount iff we incremented. this.refCountOwned = newOwned; + retainMissingClosureCaptureReferents(); retainScalarReferenceContents(value); // Flush deferred mortal decrements from the current function scope. @@ -1627,12 +1700,33 @@ private String toStringLarge() { // we. Detect by identity first, then by depth via a ThreadLocal // guard inside Overload.stringify (handles the transitive case). RuntimeScalar overloaded = Overload.stringify(this); - if (overloaded == this) yield toStringRef(); + if (isSameOverloadStringificationTarget(overloaded, this)) yield toStringRef(); yield overloaded.toString(); } }; } + private static boolean isSameOverloadStringificationTarget(RuntimeScalar overloaded, + RuntimeScalar original) { + RuntimeScalar resolvedOverloaded = unwrapReadonlyScalar(overloaded); + RuntimeScalar resolvedOriginal = unwrapReadonlyScalar(original); + if (resolvedOverloaded == resolvedOriginal) return true; + if (resolvedOverloaded == null || resolvedOriginal == null) return false; + if ((resolvedOverloaded.type & REFERENCE_BIT) == 0 + || (resolvedOriginal.type & REFERENCE_BIT) == 0) { + return false; + } + return resolvedOverloaded.value != null + && resolvedOverloaded.value == resolvedOriginal.value; + } + + private static RuntimeScalar unwrapReadonlyScalar(RuntimeScalar scalar) { + while (scalar != null && scalar.type == READONLY_SCALAR) { + scalar = (RuntimeScalar) scalar.value; + } + return scalar; + } + /** * Convert to string without triggering overload dispatch. * Used by {@code no overloading} pragma to bypass the {@code ""} overload. @@ -2521,6 +2615,7 @@ public RuntimeScalar undefine() { // truly reach zero. if (type == RuntimeScalarType.CODE && value instanceof RuntimeCode code && globalCodeRefFqn != null) { boolean releasedCode = false; + releaseAllClosureCaptureReferents(code); if (this.refCountOwned && code.refCount > 0) { this.refCountOwned = false; code.releaseActiveOwner(this); @@ -2546,6 +2641,9 @@ public RuntimeScalar undefine() { && base.refCount != -1 && base.refCount != Integer.MIN_VALUE) { oldBase = base; } + if (oldBase != null && this.captureRefCountOwned > 0) { + releaseAllClosureCaptureReferents(oldBase); + } // Close IO handles when dropping a glob reference. // This mimics Perl's internal sv_clear behavior where IO handles are closed @@ -2765,7 +2863,7 @@ public static void scopeExitCleanup(RuntimeScalar scalar) { } if (selfRef) { // Decrement our captureCount (the closure captured us) - scalar.captureCount--; + scalar.releaseClosureCapture(); // Remove self from capturedScalars to prevent double-decrement // when releaseCaptures runs later during CODE ref destruction RuntimeScalar[] old = code.capturedScalars; @@ -2784,6 +2882,7 @@ public static void scopeExitCleanup(RuntimeScalar scalar) { // Mark that this variable's scope has exited. When releaseCaptures // later decrements captureCount to 0, it will know the scope is gone. scalar.scopeExited = true; + scalar.retainMissingClosureCaptureReferents(); // For CODE refs: still decrement the VALUE's refCount so the RuntimeCode // is eventually destroyed and its releaseCaptures fires (decrementing // captureCount on all the variables IT captured). This is critical for diff --git a/src/test/resources/unit/overload/stringify.t b/src/test/resources/unit/overload/stringify.t index 352c764c4..926f0ff76 100644 --- a/src/test/resources/unit/overload/stringify.t +++ b/src/test/resources/unit/overload/stringify.t @@ -68,4 +68,13 @@ my @objects = ($basic, $advanced); my $joined = join ", ", @objects; ok(!($joined ne "Number: 42, Advanced Number: 100"), 'stringification works in join'); +{ + package BrokenStringify; + use overload '""' => sub { $_[0] }; +} + +my $broken = bless {}, 'BrokenStringify'; +like("$broken", qr/^BrokenStringify=HASH\(0x[0-9a-f]+\)$/i, + 'string overload returning same referent falls back to reference string'); + done_testing(); diff --git a/src/test/resources/unit/refcount/captured_scalar_return_and_weak_slot.t b/src/test/resources/unit/refcount/captured_scalar_return_and_weak_slot.t new file mode 100644 index 000000000..3eccf48b3 --- /dev/null +++ b/src/test/resources/unit/refcount/captured_scalar_return_and_weak_slot.t @@ -0,0 +1,41 @@ +use strict; +use warnings; + +use Test::More; +use B qw(svref_2object); +use Scalar::Util qw(weaken); + +{ + package POJCapturedScalarReturn; + sub new { bless {}, shift } +} + +sub retain_like { + my $self = shift; + push @{ $self->{callbacks} }, sub { undef $self }; + return $self; +} + +sub consume_deeply { + my ($got, $expected) = @_; + my %copy = %$expected; + return $got == $expected && exists $copy{callbacks}; +} + +my $object = POJCapturedScalarReturn->new; +$object->{callbacks} = []; + +ok consume_deeply(retain_like($object), $object), 'captured scalar return compares as the same object'; +is svref_2object($object)->REFCNT, 2, 'captured scalar return does not release closure owner'; + +my $slot_object = bless {}, 'POJWeakScalarSlot'; +my $array = [$slot_object]; +my $slot_ref = \$array->[0]; +weaken($slot_ref); + +ok defined($slot_ref), 'weak scalar ref to live array slot remains defined'; +undef $$slot_ref; +ok !defined($array->[0]), 'weak scalar ref can still clear the original array slot'; +is svref_2object($slot_object)->REFCNT, 1, 'clearing through weak scalar ref releases slot owner'; + +done_testing; diff --git a/src/test/resources/unit/refcount/closure_callback_unblessed_guard_release.t b/src/test/resources/unit/refcount/closure_callback_unblessed_guard_release.t new file mode 100644 index 000000000..316189c47 --- /dev/null +++ b/src/test/resources/unit/refcount/closure_callback_unblessed_guard_release.t @@ -0,0 +1,33 @@ +use strict; +use warnings; + +use Test::More; +use Test2::Tools::Refcount qw(is_refcount is_oneref); + +sub add_callback { + my ($callbacks, $guard) = @_; + push @$callbacks, do { + my $ref = $guard; + sub { $ref = $ref }; + }; +} + +sub run_and_clear_callbacks { + my ($callbacks) = @_; + my @todo = @$callbacks; + @$callbacks = (); + $_->() for @todo; + return; +} + +my $guard = {}; +my @callbacks; + +is_oneref($guard, 'guard starts with one lexical owner'); +add_callback(\@callbacks, $guard); +is_refcount($guard, 2, 'stored callback captures unblessed guard'); + +run_and_clear_callbacks(\@callbacks); +is_oneref($guard, 'discarded callback releases unblessed captured guard'); + +done_testing; diff --git a/src/test/resources/unit/refcount/closure_decl_shadow_weak_capture.t b/src/test/resources/unit/refcount/closure_decl_shadow_weak_capture.t new file mode 100644 index 000000000..b5dd4bbfb --- /dev/null +++ b/src/test/resources/unit/refcount/closure_decl_shadow_weak_capture.t @@ -0,0 +1,25 @@ +use strict; +use warnings; + +use Test::More; +use Scalar::Util qw(weaken); + +sub make_callback { + my $self = bless {}, 'POJClosureDeclShadowWeakCapture'; + weaken(my $weakself = $self); + + my $callback = sub { + return unless my $self = $weakself; + return $self; + }; + + return ($self, $callback); +} + +my ($object, $callback) = make_callback(); +ok defined $callback->(), 'weak self is available while strong owner lives'; + +undef $object; +ok !defined $callback->(), 'inner my declaration does not capture outer strong self'; + +done_testing; diff --git a/src/test/resources/unit/refcount/list_util_pair_helpers_refcount.t b/src/test/resources/unit/refcount/list_util_pair_helpers_refcount.t new file mode 100644 index 000000000..4dd81954a --- /dev/null +++ b/src/test/resources/unit/refcount/list_util_pair_helpers_refcount.t @@ -0,0 +1,42 @@ +use strict; +use warnings; + +use Test::More; +use B qw(svref_2object); +use List::Util qw(pairkeys pairvalues pairs unpairs); + +{ + package POJListUtilPairHelpersRefcount; + sub new { bless {}, shift } +} + +my $object = POJListUtilPairHelpersRefcount->new; +is svref_2object($object)->REFCNT, 1, 'object starts with one owner'; + +my @pairs = ($object, 'value'); +is svref_2object($object)->REFCNT, 2, 'source pair list owns one reference'; + +my @keys = pairkeys @pairs; +is svref_2object($object)->REFCNT, 3, 'pairkeys result owns one reference'; +undef @keys; +is svref_2object($object)->REFCNT, 2, 'dropping pairkeys result releases its owner'; + +my @values = pairvalues ('key', $object); +is svref_2object($object)->REFCNT, 3, 'pairvalues result owns one reference'; +undef @values; +is svref_2object($object)->REFCNT, 2, 'dropping pairvalues result releases its owner'; + +my @pair_refs = pairs $object, 'value'; +is svref_2object($object)->REFCNT, 3, 'pairs result owns one nested reference'; +undef @pair_refs; +is svref_2object($object)->REFCNT, 2, 'dropping pairs result releases nested owner'; + +my $pair = [$object, 'value']; +is svref_2object($object)->REFCNT, 3, 'array pair owns one reference'; + +my @flat = unpairs $pair; +is svref_2object($object)->REFCNT, 4, 'unpairs result owns one reference'; +undef @flat; +is svref_2object($object)->REFCNT, 3, 'dropping unpairs result releases its owner'; + +done_testing; diff --git a/src/test/resources/unit/refcount/returned_owned_scalar_arg_chain_weak.t b/src/test/resources/unit/refcount/returned_owned_scalar_arg_chain_weak.t new file mode 100644 index 000000000..380d94d5e --- /dev/null +++ b/src/test/resources/unit/refcount/returned_owned_scalar_arg_chain_weak.t @@ -0,0 +1,60 @@ +use strict; +use warnings; + +use Test::More; +use Scalar::Util qw(weaken); + +{ + package POJReturnedOwnedScalarSequence; + + sub new { + bless { callbacks => [] }, shift; + } + + sub then { + my $self = shift; + my $seq = __PACKAGE__->new; + push @{ $self->{callbacks} }, [ $seq ]; + Scalar::Util::weaken $self->{callbacks}[-1][0]; + return $seq; + } + + sub on_ready { + my $self = shift; + return $self; + } +} + +{ + package POJReturnedOwnedScalarMutex; + + sub new { + bless { queue => [] }, shift; + } + + sub enter { + my $self = shift; + my $down = POJReturnedOwnedScalarSequence->new; + push @{ $self->{queue} }, $down; + my $retf = $down->then->on_ready; + return $retf; + } +} + +sub collect { + my @items = @_; + return \@items; +} + +my $mutex = POJReturnedOwnedScalarMutex->new; +my $held = collect( + $mutex->enter, + $mutex->enter, + $mutex->enter, +); + +ok defined $mutex->{queue}[0]{callbacks}[0][0], + 'weak callback target survives direct chained return into argument list'; +is scalar(@$held), 3, 'collector holds returned sequence objects'; + +done_testing; diff --git a/src/test/resources/unit/refcount/sub_quote_overloaded_code_return.t b/src/test/resources/unit/refcount/sub_quote_overloaded_code_return.t new file mode 100644 index 000000000..1a3873f6d --- /dev/null +++ b/src/test/resources/unit/refcount/sub_quote_overloaded_code_return.t @@ -0,0 +1,27 @@ +use strict; +use warnings; + +use Test::More; +use B::Deparse; +use Sub::Defer qw(defer_sub); +use Sub::Quote qw(quote_sub); + +defer_sub 'QuotedConstructor::new' => sub { + my $constructor = quote_sub( + 'QuotedConstructor::new' => 'my $marker = "quoted constructor survives"; 42', + {}, + { + package => 'QuotedConstructor', + no_defer => 1, + no_install => 1, + }, + ); + $constructor; +}; + +is(QuotedConstructor->new, 42, 'deferred quoted constructor remains callable'); + +my $source = B::Deparse->new->coderef2text(QuotedConstructor->can('new')); +like($source, qr/quoted constructor survives/, 'deferred quoted constructor metadata survives temporary cleanup'); + +done_testing; From 9203c28c69408e303ef04a35974b1421ab9f61cb Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Thu, 28 May 2026 20:28:12 +0200 Subject: [PATCH 2/3] fix: preserve interpreter assignment aliases Preserve captured scalar slots in bytecode-interpreter lexical assignments so closures and lvalue subs continue to alias the live lexical. Route bytecode-interpreter assignments to `our` scalars through the declaring package global, matching interpreter reads. Add interpreter-forced regression coverage for the captured lexical and `our` scalar assignment failures found in PR #833. Generated with [Codex](https://openai.com/codex) Co-Authored-By: Codex --- .../backend/bytecode/BytecodeInterpreter.java | 2 + .../backend/bytecode/CompileAssignment.java | 21 +++++- ...terpreter_capture_assignment_regressions.t | 74 +++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 src/test/resources/unit/interpreter_capture_assignment_regressions.t diff --git a/src/main/java/org/perlonjava/backend/bytecode/BytecodeInterpreter.java b/src/main/java/org/perlonjava/backend/bytecode/BytecodeInterpreter.java index f75266be7..cccaefb2f 100644 --- a/src/main/java/org/perlonjava/backend/bytecode/BytecodeInterpreter.java +++ b/src/main/java/org/perlonjava/backend/bytecode/BytecodeInterpreter.java @@ -55,6 +55,8 @@ static boolean isImmutableProxy(RuntimeBase val) { private static boolean lexicalAssignmentMustPreserveSlot(RuntimeBase val) { if (!(val instanceof RuntimeScalar scalar)) return false; return scalar instanceof ReadOnlyAlias + || scalar.captureCount > 0 + || scalar.captureRefCountOwned > 0 || scalar.type == RuntimeScalarType.TIED_SCALAR || scalar.type == RuntimeScalarType.READONLY_SCALAR; } diff --git a/src/main/java/org/perlonjava/backend/bytecode/CompileAssignment.java b/src/main/java/org/perlonjava/backend/bytecode/CompileAssignment.java index d55e0130c..536381fc0 100644 --- a/src/main/java/org/perlonjava/backend/bytecode/CompileAssignment.java +++ b/src/main/java/org/perlonjava/backend/bytecode/CompileAssignment.java @@ -3,6 +3,7 @@ import org.perlonjava.frontend.analysis.ConstantFoldingVisitor; import org.perlonjava.frontend.analysis.LValueVisitor; import org.perlonjava.frontend.astnode.*; +import org.perlonjava.frontend.semantic.SymbolTable; import org.perlonjava.runtime.runtimetypes.NameNormalizer; import org.perlonjava.runtime.runtimetypes.RuntimeCode; import org.perlonjava.runtime.runtimetypes.RuntimeContextType; @@ -822,7 +823,25 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler, if (leftOp.operator.equals("$") && leftOp.operand instanceof IdentifierNode) { String varName = "$" + ((IdentifierNode) leftOp.operand).name; - if (bytecodeCompiler.hasVariable(varName)) { + if (bytecodeCompiler.hasVariable(varName) && bytecodeCompiler.isOurVariable(varName)) { + SymbolTable.SymbolEntry ourEntry = bytecodeCompiler.symbolTable.getSymbolEntry(varName); + String ourPkg = (ourEntry != null && ourEntry.perlPackage() != null) + ? ourEntry.perlPackage() + : bytecodeCompiler.getCurrentPackage(); + String bareVarName = varName.substring(1); + String normalizedName = NameNormalizer.normalizeVariableName(bareVarName, ourPkg); + int nameIdx = bytecodeCompiler.addToStringPool(normalizedName); + + bytecodeCompiler.emit(Opcodes.STORE_GLOBAL_SCALAR); + bytecodeCompiler.emit(nameIdx); + bytecodeCompiler.emitReg(valueReg); + + int lvalReg = bytecodeCompiler.allocateRegister(); + bytecodeCompiler.emit(Opcodes.LOAD_GLOBAL_SCALAR); + bytecodeCompiler.emitReg(lvalReg); + bytecodeCompiler.emit(nameIdx); + bytecodeCompiler.lastResultReg = lvalReg; + } else if (bytecodeCompiler.hasVariable(varName)) { // Lexical variable - check if it's captured int targetReg = bytecodeCompiler.getVariableRegister(varName); diff --git a/src/test/resources/unit/interpreter_capture_assignment_regressions.t b/src/test/resources/unit/interpreter_capture_assignment_regressions.t new file mode 100644 index 000000000..e3205ace2 --- /dev/null +++ b/src/test/resources/unit/interpreter_capture_assignment_regressions.t @@ -0,0 +1,74 @@ +use strict; +use warnings; +use Test::More tests => 5; + +# These regressions only surface on the bytecode-interpreter path. The +# unreachable dynamic goto is enough to force interpreter fallback for each sub. + +sub captured_lvalue_assignment { + my $in; + sub pr833_get_lex : lvalue { $in } + $in = 5; + pr833_get_lex() = 7; + return $in; + + my $f = sub { 1 }; + goto $f; +} + +is(captured_lvalue_assignment(), 7, + 'interpreter assignment preserves scalar slot captured by lvalue sub'); + +sub captured_closure_assignment { + my $value = 1; + my $get = sub { $value }; + $value = 2; + return $get->(); + + my $f = sub { 1 }; + goto $f; +} + +is(captured_closure_assignment(), 2, + 'interpreter assignment preserves scalar slot captured by anonymous closure'); + +sub our_assignment_chain { + our $PR833_X1 = ''; + our $PR833_X2 = ''; + my ($a, $b) = qw(abcd wxyz); + $PR833_X1 = ($PR833_X2 = sprintf('%s%s', $a, $b)); + return "$PR833_X1|$PR833_X2"; + + my $f = sub { 1 }; + goto $f; +} + +is(our_assignment_chain(), 'abcdwxyz|abcdwxyz', + 'interpreter assignment to our scalars writes package globals'); + +sub local_our_assignment_chain { + local our $PR833_LOCAL_X1 = ''; + local our $PR833_LOCAL_X2 = ''; + my ($a, $b) = qw(abcd wxyz); + $PR833_LOCAL_X1 = ($PR833_LOCAL_X2 = sprintf('%s%s', $a, $b)); + return "$PR833_LOCAL_X1|$PR833_LOCAL_X2"; + + my $f = sub { 1 }; + goto $f; +} + +is(local_our_assignment_chain(), 'abcdwxyz|abcdwxyz', + 'interpreter assignment to localized our scalars writes localized globals'); + +sub our_declared_package_assignment { + package PR833::Other; + our $VALUE = ''; + $VALUE = 'set in declared package'; + return $VALUE; + + my $f = sub { 1 }; + goto $f; +} + +is(our_declared_package_assignment(), 'set in declared package', + 'interpreter assignment honours the declaring package of our scalar'); From e179a16b8b06981406f135ea6498357e5e301f1e Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Thu, 28 May 2026 22:15:33 +0200 Subject: [PATCH 3/3] test: make quoted code refcount regression self-contained Inline the small Sub::Quote/Sub::Defer behavior needed by the regression test so CI unit tests do not depend on locally installed CPAN modules. Generated with [Codex](https://openai.com/codex) Co-Authored-By: Codex --- .../sub_quote_overloaded_code_return.t | 82 ++++++++++++++++--- 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/src/test/resources/unit/refcount/sub_quote_overloaded_code_return.t b/src/test/resources/unit/refcount/sub_quote_overloaded_code_return.t index 1a3873f6d..db0cbbfc7 100644 --- a/src/test/resources/unit/refcount/sub_quote_overloaded_code_return.t +++ b/src/test/resources/unit/refcount/sub_quote_overloaded_code_return.t @@ -3,18 +3,76 @@ use warnings; use Test::More; use B::Deparse; -use Sub::Defer qw(defer_sub); -use Sub::Quote qw(quote_sub); - -defer_sub 'QuotedConstructor::new' => sub { - my $constructor = quote_sub( - 'QuotedConstructor::new' => 'my $marker = "quoted constructor survives"; 42', - {}, - { - package => 'QuotedConstructor', - no_defer => 1, - no_install => 1, - }, +use Scalar::Util qw(weaken); + +{ + package Sub::Quote; + + our %QUOTED; + + sub quote_sub { + my ($name, $source) = @_; + my $quoted_info = { + name => $name, + code => $source, + }; + + my $constructor = sub { + ($quoted_info) if 0; + my $marker = "quoted constructor survives"; + 42; + }; + + Scalar::Util::weaken($QUOTED{$constructor} = $quoted_info); + return $constructor; + } + + sub quoted_from_sub { + my ($sub) = @_; + my $quoted_info = $QUOTED{$sub || ''} or return undef; + return [ $quoted_info->{name}, $quoted_info->{code} ]; + } +} + +{ + package Sub::Defer; + + our %DEFERRED; + + sub defer_sub { + my ($target, $maker) = @_; + my $undeferred; + my $deferred; + + $deferred = sub { + $undeferred ||= undefer_sub($deferred); + goto &$undeferred; + }; + + $DEFERRED{$deferred} = [ $target, $maker, \$undeferred, $deferred ]; + no strict 'refs'; + *{$target} = $deferred; + return $deferred; + } + + sub undefer_sub { + my ($deferred) = @_; + my $info = $DEFERRED{$deferred} or return $deferred; + my ($target, $maker, $undeferred_ref) = @$info; + return $$undeferred_ref if $$undeferred_ref; + + my $made = $$undeferred_ref = $maker->(); + no strict 'refs'; + *{$target} = $made if defined $target; + $DEFERRED{$made} = $info; + return $made; + } +} + +Sub::Defer::defer_sub 'QuotedConstructor::new' => sub { + my $constructor = Sub::Quote::quote_sub( + 'QuotedConstructor::new', + 'my $marker = "quoted constructor survives"; 42', ); $constructor; };