fix(codegen,runtime): release a frame's variable-box cells at scope exit

changelog.d/10613-box-cell-release.md

@@ -0,0 +1,52 @@
+### Fixed: an ordinary frame now releases the box cells it minted (#10464)
+
+A `let`/`var` that a closure captures and something reassigns is stored in a
+malloc-side **box cell**, and every registered cell is a strong GC root
+(`scan_box_roots_mut`). The only release Perry emitted was the async-to-generator
+transform's terminal `Stmt::ReleaseBoxes` (#7933/#8208/#8303), so a synchronous
+function, method, arrow, generator — or an `async` function with no `await` —
+leaked one registered root per boxed binding per call, plus everything that
+binding last pointed at. `PERRY_GC_DIAG=1` reported `releases=0` for every
+non-async workload; the issue's repro reached 601 MB RSS at 100k calls where the
+equal-allocation control stayed at 69 MB, and real packages accumulated cells by
+the hundred thousand (qs 792k, dayjs 434k).
+
+**Root cause.** `js_box_alloc_bits` registers each cell for the life of the
+thread, and only `perry-transform`'s async step lowering produced the
+`Stmt::ReleaseBoxes` that `emit_release_boxes` lowers. Nothing named the cells of
+an ordinary frame, so nothing could ever reclaim them.
+
+**Fix.** Codegen registers every entry slot that holds a cell *this* frame minted
+(`stmt/boxed_frame_release.rs`); the return-site rewrite that already injects
+`js_shadow_frame_pop` now also emits `js_box_scope_release` for each of them
+before every `ret`, and a declaration inside a loop releases the previous
+iteration's cell before minting the next. The runtime publishes a cell no closure
+captured (de-register, cache-evict, clear, free-list push) and marks a captured
+one frame-released in its capture-edge record, so the last capture edge's GC
+death publishes it — the same escape contract #8303 built for async activations,
+including the full-trace ephemeron rule that keeps `box -> closure -> same box`
+collectable. Two holders the runtime cannot count keep their cells: a sloppy-mode
+mapped `arguments` object, and a plain-async step closure's own activation cells.
+A step closure's capture of an *enclosing* frame's cell is now counted, because
+the activation token never covered it and that frame does release its cells.
+
+Fixing this exposed a latent GC hole shared with #8303: a full trace that stops
+rooting a released cell must still keep it in the box young log, because a minor
+walks only that log. Without it the next minor had no root for a payload a live
+closure still reads — `PERRY_GC_SCHEDULE_SEED=1 PERRY_GC_SCHEDULE_RATE=1
+PERRY_GC_PROTECT_FROMSPACE=1` faults on it immediately, and a unit test now
+re-derives the rule.
+
+**Validation.** New gap test `test_gap_10464_box_cell_release.ts` (the repro's
+memory shape compared against its own control in-process, plus escaping-closure,
+per-iteration-binding, generator, class, async and self-cycle cases) differs from
+Node on the parent commit and matches it here. GC stress over seeds 1-7 at
+`PERRY_GC_SCHEDULE_RATE=1` with the from-space quarantine confirmed armed
+(`[gc-fromspace-protect] retired_set=#0`, 141,635 hits over the run) held
+byte-identical to Node on every deterministic output line, and
+`PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1` (panics on any stale
+forwarded-pointer read) passed clean. Instructions *improve* slightly on a hot
+captured-variable loop (20M calls, -3.6%) and on 1M closure creations (-2.6%),
+from reduced GC root-scanning pressure; the issue's `payload` repro reproduces
+the RSS claim independently: 608 MB -> 152 MB peak RSS here (originally recorded
+616 MB -> 163 MB on the destroyed build host).

crates/perry-codegen/src/codegen/arguments.rs

@@ -50,6 +50,34 @@ pub(crate) fn store_param_slot(
     slot
 }
 
+/// #10464: a boxed parameter's cell is minted by this frame's entry block
+/// (`store_param_slot`), so the frame releases it before every `ret`.
+/// `materialize_arguments_object` withdraws a slot it maps into a sloppy-mode
+/// `arguments` object, which holds the raw cell without a counted edge.
+pub(crate) fn release_boxed_param_slots_at_exit(
+    lf: &mut crate::function::LlFunction,
+    params: &[Param],
+    boxed_vars: &HashSet<u32>,
+    slots: &std::collections::HashMap<u32, String>,
+) {
+    for p in params {
+        if !boxed_vars.contains(&p.id) || p.arguments_object.is_some() {
+            continue;
+        }
+        if let Some(slot) = slots.get(&p.id) {
+            lf.add_pre_return_box_release(slot, "js_box_scope_release");
+        }
+    }
+}
+
+/// The parameter ids a synthesized `arguments` object aliases.
+pub(crate) fn mapped_parameter_ids(params: &[Param]) -> HashSet<u32> {
+    mapped_arguments_params(params)
+        .into_iter()
+        .map(|(_, id)| id)
+        .collect()
+}
+
 pub(crate) fn materialize_arguments_object(
     ctx: &mut FnCtx<'_>,
     params: &[Param],
@@ -110,6 +138,8 @@ pub(crate) fn materialize_arguments_object(
     );
     for (arg_index, param_id) in mapped_arguments_params(params) {
         if let Some(param_slot) = ctx.locals.get(&param_id).cloned() {
+            // #10464: the object aliases the cell for its own lifetime.
+            ctx.func.forget_pre_return_box_release(&param_slot);
             let box_ptr = ctx.block().load(I64, &param_slot);
             ctx.block().call_void(
                 "js_arguments_object_map_index",

crates/perry-codegen/src/codegen/closure.rs

@@ -639,6 +639,7 @@ pub(super) fn compile_closure(
         }
         map
     };
+    super::arguments::release_boxed_param_slots_at_exit(lf, params, &closure_boxed_vars, &locals);
 
     // Start with the closure's own params as local_types, then
     // merge in the module-wide map so captured-from-outer ids have

crates/perry-codegen/src/codegen/function.rs

@@ -863,6 +863,7 @@ pub(super) fn compile_function(
         }
         map
     };
+    super::arguments::release_boxed_param_slots_at_exit(lf, &f.params, &boxed_vars, &locals);
 
     // Param types feed local_types so type-aware dispatch (e.g. string
     // concat detection on a `: string` parameter) works inside the body.

crates/perry-codegen/src/codegen/method.rs

@@ -384,6 +384,12 @@ pub(super) fn compile_method(
         }
         (this_slot, map)
     };
+    super::arguments::release_boxed_param_slots_at_exit(
+        lf,
+        &method.params,
+        &method_boxed_vars,
+        &locals,
+    );
 
     let mut local_types: HashMap<u32, perry_hir::types::Type> = module_global_types
         .iter()

crates/perry-codegen/src/codegen/method_static.rs

@@ -122,6 +122,12 @@ pub(in crate::codegen) fn compile_static_method(
         }
         (this_slot, map)
     };
+    crate::codegen::arguments::release_boxed_param_slots_at_exit(
+        lf,
+        &f.params,
+        &static_boxed_vars,
+        &locals,
+    );
 
     // Seed with module-global declared types (mirrors compile_method /
     // compile_function): static-method bodies read module globals through

crates/perry-codegen/src/expr/closure.rs

@@ -12,57 +12,76 @@ use crate::types::{DOUBLE, I32, I64, PTR};
 
 use super::{lower_expr, nanbox_pointer_inline, FnCtx};
 
-/// Whether this is the compiler-private step closure for a lowered plain
-/// async activation. `ReleaseBoxes` is emitted only in that closure's
-/// terminal arms; user-authored closures can never contain it.
+/// The activation cells of the compiler-private step closure for a lowered
+/// plain async activation: every id its terminal `ReleaseBoxes` arms name, or
+/// `None` for any other closure. `ReleaseBoxes` is emitted only in that
+/// closure's terminal arms; user-authored closures can never contain it.
 ///
 /// Queued and running instances of this closure are already covered by the
-/// activation token's refcount. Counting its boxed capture slots as escaping
+/// activation token's refcount. Counting those boxed capture slots as escaping
 /// GC-closure edges would make every cell in the complete activation frame
-/// wait for a full collection, even when no user closure can observe it.
-fn is_plain_async_step_body(stmts: &[Stmt]) -> bool {
-    stmts.iter().any(|stmt| match stmt {
-        Stmt::ReleaseBoxes(_) => true,
-        Stmt::If {
-            then_branch,
-            else_branch,
-            ..
-        } => {
-            is_plain_async_step_body(then_branch)
-                || else_branch.as_deref().is_some_and(is_plain_async_step_body)
-        }
-        Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => is_plain_async_step_body(body),
-        Stmt::For { init, body, .. } => {
-            init.as_deref()
-                .is_some_and(|stmt| is_plain_async_step_body(std::slice::from_ref(stmt)))
-                || is_plain_async_step_body(body)
-        }
-        Stmt::Try {
-            body,
-            catch,
-            finally,
-        } => {
-            is_plain_async_step_body(body)
-                || catch
-                    .as_ref()
-                    .is_some_and(|catch| is_plain_async_step_body(&catch.body))
-                || finally.as_deref().is_some_and(is_plain_async_step_body)
+/// wait for a full collection, even when no user closure can observe it. A
+/// cell from an ENCLOSING scope is not covered by that token (#10464: its
+/// owner frame now releases it at scope exit), so only these ids go uncounted.
+fn plain_async_step_release_ids(stmts: &[Stmt]) -> Option<std::collections::HashSet<u32>> {
+    fn walk(stmts: &[Stmt], out: &mut Option<std::collections::HashSet<u32>>) {
+        for stmt in stmts {
+            match stmt {
+                Stmt::ReleaseBoxes(ids) => out
+                    .get_or_insert_with(Default::default)
+                    .extend(ids.iter().copied()),
+                Stmt::If {
+                    then_branch,
+                    else_branch,
+                    ..
+                } => {
+                    walk(then_branch, out);
+                    if let Some(else_branch) = else_branch {
+                        walk(else_branch, out);
+                    }
+                }
+                Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => walk(body, out),
+                Stmt::For { init, body, .. } => {
+                    if let Some(init) = init {
+                        walk(std::slice::from_ref(init.as_ref()), out);
+                    }
+                    walk(body, out);
+                }
+                Stmt::Try {
+                    body,
+                    catch,
+                    finally,
+                } => {
+                    walk(body, out);
+                    if let Some(catch) = catch {
+                        walk(&catch.body, out);
+                    }
+                    if let Some(finally) = finally {
+                        walk(finally, out);
+                    }
+                }
+                Stmt::Switch { cases, .. } => {
+                    for case in cases {
+                        walk(&case.body, out);
+                    }
+                }
+                Stmt::Labeled { body, .. } => walk(std::slice::from_ref(body.as_ref()), out),
+                Stmt::Let { .. }
+                | Stmt::Expr(_)
+                | Stmt::Return(_)
+                | Stmt::Break
+                | Stmt::Continue
+                | Stmt::LabeledBreak(_)
+                | Stmt::LabeledContinue(_)
+                | Stmt::Throw(_)
+                | Stmt::PreallocateBoxes(_)
+                | Stmt::PreallocateTdzBoxes(_) => {}
+            }
         }
-        Stmt::Switch { cases, .. } => cases
-            .iter()
-            .any(|case| is_plain_async_step_body(&case.body)),
-        Stmt::Labeled { body, .. } => is_plain_async_step_body(std::slice::from_ref(body.as_ref())),
-        Stmt::Let { .. }
-        | Stmt::Expr(_)
-        | Stmt::Return(_)
-        | Stmt::Break
-        | Stmt::Continue
-        | Stmt::LabeledBreak(_)
-        | Stmt::LabeledContinue(_)
-        | Stmt::Throw(_)
-        | Stmt::PreallocateBoxes(_)
-        | Stmt::PreallocateTdzBoxes(_) => false,
-    })
+    }
+    let mut out = None;
+    walk(stmts, &mut out);
+    out
 }
 
 pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
@@ -130,6 +149,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
             // closure body can deref it via js_box_get/set. Without
             // this, each closure would get a snapshot of the box's
             // current value.
+            let plain_async_step_cells = plain_async_step_release_ids(body);
+            let uncounted_box_capture = |cap_id: &u32| {
+                plain_async_step_cells
+                    .as_ref()
+                    .is_some_and(|cells| cells.contains(cap_id))
+            };
             let mut captured_value_bits: Vec<String> = Vec::with_capacity(auto_captures.len());
             for cap_id in &auto_captures {
                 if ctx.boxed_vars.contains(cap_id) {
@@ -156,6 +181,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                     } else if let Some(slot) = ctx.locals.get(cap_id).cloned() {
                         // Enclosing function owns the box: slot holds
                         // the raw box pointer as i64.
+                        if uncounted_box_capture(cap_id) {
+                            // #10464: the activation, not this frame, owns
+                            // the cell's lifetime from here on.
+                            ctx.func.forget_pre_return_box_release(&slot);
+                        }
                         let box_ptr = ctx.block().load(I64, &slot);
                         captured_value_bits.push(box_ptr);
                     } else if let Some(global_name) = ctx.module_globals.get(cap_id).cloned() {
@@ -297,13 +327,13 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
             //
             // The singleton caches therefore only serve closures the
             // COMPILER synthesized: the async-activation step closures
-            // recognized by `is_plain_async_step_body` (their terminal
+            // recognized by `plain_async_step_release_ids` (their terminal
             // `ReleaseBoxes` arms cannot appear in user code, and their
             // identity never escapes the runtime's promise machinery).
             // Those are the closures the caches were built for — re-created
             // per resume with the same per-activation box captures. User
             // arrows and function expressions always mint fresh objects.
-            let is_plain_async_step = is_plain_async_step_body(body);
+            let is_plain_async_step = plain_async_step_cells.is_some();
             let singleton_identity_safe = is_plain_async_step && (*is_arrow || captures_all_boxed);
             let no_capture_singleton = is_plain_async_step && *is_arrow && total_caps == 0;
             let captured_singleton =
@@ -347,9 +377,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 && !captured_singleton
                 && total_caps > 0
                 && !captured_value_bits.is_empty()
-                && auto_captures
-                    .iter()
-                    .all(|cap_id| is_plain_async_step || !ctx.boxed_vars.contains(cap_id));
+                && auto_captures.iter().all(|cap_id| {
+                    !ctx.boxed_vars.contains(cap_id) || uncounted_box_capture(cap_id)
+                });
             let closure_handle = if no_capture_singleton {
                 let blk = ctx.block();
                 blk.call(I64, "js_closure_alloc_singleton", &[(PTR, &func_ref)])
@@ -435,17 +465,19 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
             // their lifetime edges are declared; fresh closures need every
             // slot initialized here. The compiler-private plain-async step
             // closure is different: its activation refcount already covers
-            // every queued/running instance, so declaring its whole boxed
-            // frame as escaped would delay every terminal cell until a full
-            // GC. User closures nested inside it still take the dedicated
-            // setter and therefore preserve #8213's escaped-cell lifetime.
-            let boxed_capture_slots = auto_captures
+            // every queued/running instance of its OWN cells, so declaring its
+            // whole boxed frame as escaped would delay every terminal cell
+            // until a full GC. User closures nested inside it still take the
+            // dedicated setter and therefore preserve #8213's escaped-cell
+            // lifetime, and so does a step closure's capture of an enclosing
+            // scope's cell (#10464).
+            let tracked_box_capture_slots = auto_captures
                 .iter()
-                .map(|cap_id| ctx.boxed_vars.contains(cap_id))
+                .map(|cap_id| ctx.boxed_vars.contains(cap_id) && !uncounted_box_capture(cap_id))
                 .collect::<Vec<_>>();
             let blk = ctx.block();
             for (idx, val_bits) in captured_value_bits.iter().enumerate() {
-                let track_box_capture = boxed_capture_slots[idx] && !is_plain_async_step;
+                let track_box_capture = tracked_box_capture_slots[idx];
                 if bulk_fresh_init {
                     // Every slot was written by `js_closure_alloc_init`.
                     continue;