From 84e756fd94d35ded5dcb89c562605d091d3b8589 Mon Sep 17 00:00:00 2001
From: mirchaemanuel <mirchaemanuel@gmail.com>
Date: Sat, 11 Jul 2026 09:12:23 +0200
Subject: [PATCH] fix(ir_lower): release nullable-access hidden temp after its
 consuming chained read

A chained subscript read on a ?array receiver materializes the
intermediate container through the hidden owned temp created by
lower_nullable_array_access. take_owned_temp clears the backing slot
without releasing, so when the loaded temp value became the receiver of
the consuming read nothing ever dropped its reference, leaking the
returned containers on every non-null call (issue #525).

Release the receiver with an IR-level Op::Release immediately after the
consuming read when it is a load of a one-shot owned hidden temp and the
read went through the boxed Mixed runtime path, whose result is an
owned, independent cell (string payloads persisted, child pointers
retained), so the release cannot invalidate the result. Typed reads
(ArrayGet, StrCharAt, ...) can return payloads borrowing the receiver's
storage and keep it alive, and direct array_get/hash_get receivers are
intentionally untouched.

Covers both merge results exactly once: the populated container from the
non-null path and the boxed Mixed null from the null path.

Fixes #525

Claude-Session: https://claude.ai/code/session_01Jfn7uqEH5Dog1nBzTM5DCW
---
 src/ir_lower/context.rs                 |  2 +-
 src/ir_lower/expr/mod.rs                | 22 +++++++-
 tests/codegen/runtime_gc/regressions.rs | 69 +++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/src/ir_lower/context.rs b/src/ir_lower/context.rs
index 6bce5860a6..51361c667f 100644
--- a/src/ir_lower/context.rs
+++ b/src/ir_lower/context.rs
@@ -1118,7 +1118,7 @@ impl<'m, 'f> LoweringContext<'m, 'f> {
     }
 
     /// Returns whether the value is a read from a one-shot hidden expression temp.
-    fn value_is_owned_temp_load(&self, value: ValueId) -> bool {
+    pub(crate) fn value_is_owned_temp_load(&self, value: ValueId) -> bool {
         let Some(inst) = self.builder.value_defining_instruction(value) else {
             return false;
         };
diff --git a/src/ir_lower/expr/mod.rs b/src/ir_lower/expr/mod.rs
index 71a2f70ede..2e12408005 100644
--- a/src/ir_lower/expr/mod.rs
+++ b/src/ir_lower/expr/mod.rs
@@ -7268,14 +7268,32 @@ fn lower_array_access_from_value(
         _ => Op::RuntimeCall,
     };
     let result_type = array_access_result_type(ctx, array_value.value, op, expr);
-    ctx.emit_value(
+    let result = ctx.emit_value(
         op,
         vec![array_value.value, index_value.value],
         None,
         result_type,
         op.default_effects(),
         Some(expr.span),
-    )
+    );
+    // A receiver materialized through a one-shot hidden owned temp (e.g. the
+    // merge temp `lower_nullable_array_access` uses for a chained `?array`
+    // read) carries an owned reference that nothing else releases anymore:
+    // `take_owned_temp` already cleared the backing slot, so this read is the
+    // temp value's last consuming use. Drop that reference here, exactly once,
+    // on the loaded SSA value itself so both merge results are freed — the
+    // populated container from the non-null path and the boxed Mixed null from
+    // the null path (Release is a no-op on genuinely empty values). This is
+    // restricted to the boxed Mixed runtime read because
+    // `__rt_mixed_array_get` returns an owned, independent cell (string
+    // payloads are persisted and child heap pointers retained), so releasing
+    // the receiver cannot invalidate the result; typed reads (`ArrayGet`,
+    // `StrCharAt`, ...) can return payloads that still borrow from the
+    // receiver's storage and must keep it alive.
+    if op == Op::RuntimeCall && ctx.value_is_owned_temp_load(array_value.value) {
+        crate::ir_lower::ownership::release_if_owned(ctx, array_value, Some(expr.span));
+    }
+    result
 }
 
 /// Lowers nullable receiver indexing without evaluating the index on a null receiver.
diff --git a/tests/codegen/runtime_gc/regressions.rs b/tests/codegen/runtime_gc/regressions.rs
index bdf32578f7..a56556ecc5 100644
--- a/tests/codegen/runtime_gc/regressions.rs
+++ b/tests/codegen/runtime_gc/regressions.rs
@@ -1230,3 +1230,72 @@ echo "done";
         "promoting an indexed array literal to hash storage must free the source array (issue #408)"
     );
 }
+
+/// Regression test for issue #525: a chained subscript read on a `?array`
+/// receiver materializes the intermediate container through the nullable-access
+/// hidden owned temp (`lower_nullable_array_access`), and the consuming read
+/// must release that temp's reference after its last use. Exercises both the
+/// non-null receiver (populated temp) and the null receiver (boxed-null temp)
+/// on every iteration and asserts the heap is clean at exit.
+#[test]
+fn test_regression_525_nullable_chained_read_releases_hidden_temp() {
+    let out = compile_and_run_with_heap_debug(
+        r#"<?php
+function probe(?array $r): int {
+    $x = $r[0][1];
+    if ($x !== null) {
+        return 1;
+    }
+    return 0;
+}
+$hits = 0;
+for ($k = 0; $k < 50; $k++) {
+    $a = [['a', 'b' . $k]];
+    $hits = $hits + probe($a);
+    $hits = $hits + probe(null);
+}
+echo $hits;
+"#,
+    );
+    assert!(out.success, "program failed: {}", out.stderr);
+    assert_eq!(out.stdout, "50");
+    assert!(
+        out.stderr.contains("HEAP DEBUG: leak summary: clean"),
+        "expected a clean heap, got: {}",
+        out.stderr
+    );
+}
+
+/// Regression test for issue #525 (multiple reads): each chained read on the
+/// same `?array` receiver creates its own nullable-access hidden temp, and every
+/// temp's reference must be released independently — using the receiver twice
+/// must not leak either intermediate container or double-release one of them.
+#[test]
+fn test_regression_525_nullable_receiver_read_twice_heap_clean() {
+    let out = compile_and_run_with_heap_debug(
+        r#"<?php
+function probe2(?array $r): int {
+    $a = $r[0][0];
+    $b = $r[0][1];
+    if ($a !== null && $b !== null) {
+        return 1;
+    }
+    return 0;
+}
+$hits = 0;
+for ($k = 0; $k < 50; $k++) {
+    $pair = [['a' . $k, 'b' . $k]];
+    $hits = $hits + probe2($pair);
+    $hits = $hits + probe2(null);
+}
+echo $hits;
+"#,
+    );
+    assert!(out.success, "program failed: {}", out.stderr);
+    assert_eq!(out.stdout, "50");
+    assert!(
+        out.stderr.contains("HEAP DEBUG: leak summary: clean"),
+        "expected a clean heap, got: {}",
+        out.stderr
+    );
+}