From 1c0206c92b4ea8b861af6a2fbc7aa59c6970274e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 08:20:25 +0000 Subject: [PATCH 1/2] fix(codegen,runtime): release a frame's variable-box cells at scope exit A let/var a closure captures and something reassigns lives in a malloc-side box cell, and every registered cell is a strong GC root (scan_box_roots_mut). Only the async-to-generator transform's terminal Stmt::ReleaseBoxes ever released one (#7933/#8208/#8303); an ordinary 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. Codegen registers every entry slot holding a cell this frame minted (stmt/boxed_frame_release.rs, new) and the existing return-site rewrite that already injects js_shadow_frame_pop now also emits js_box_scope_release before every ret; a declaration inside a loop releases the previous iteration's cell before minting the next. Runtime (box/scope_release.rs, new): a cell no closure captured publishes immediately; a captured cell is marked frame-released in its capture-edge record and published only when its last capture edge dies via the existing dead-owner pruning -- the same escape contract #8303 built for async activations. Two holders the runtime cannot count keep their cells instead of double-releasing: 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, since the activation token never covered it. Fixes a latent GC hole shared with #8303 in the same commit: a full trace that stops rooting a released cell must still keep it in BOX_YOUNG_ROOTS while young, because a minor walks only that log -- dropping it left the next minor with no root for a payload a live closure still reads. --- crates/perry-codegen/src/codegen/arguments.rs | 30 ++ crates/perry-codegen/src/codegen/closure.rs | 1 + crates/perry-codegen/src/codegen/function.rs | 1 + crates/perry-codegen/src/codegen/method.rs | 6 + .../src/codegen/method_static.rs | 6 + crates/perry-codegen/src/expr/closure.rs | 150 +++++--- crates/perry-codegen/src/function.rs | 50 ++- crates/perry-codegen/src/gc_call_effects.rs | 11 +- .../src/lower_call/new_ctor_args.rs | 17 +- .../src/runtime_decls/strings.rs | 4 + .../src/stmt/boxed_frame_release.rs | 65 ++++ .../src/stmt/boxed_frame_release_tests.rs | 293 +++++++++++++++ .../src/stmt/boxed_local_init.rs | 26 +- crates/perry-codegen/src/stmt/let_stmt.rs | 16 +- crates/perry-codegen/src/stmt/mod.rs | 62 ++-- crates/perry-runtime/src/box.rs | 92 +++-- crates/perry-runtime/src/box/scope_release.rs | 346 ++++++++++++++++++ .../perry-runtime/src/closure/box_captures.rs | 182 +++++++-- crates/perry-runtime/src/closure/mod.rs | 3 +- scripts/gc_root_dominance_check.py | 25 ++ test-files/test_gap_10464_box_cell_release.ts | 305 +++++++++++++++ 21 files changed, 1514 insertions(+), 177 deletions(-) create mode 100644 crates/perry-codegen/src/stmt/boxed_frame_release.rs create mode 100644 crates/perry-codegen/src/stmt/boxed_frame_release_tests.rs create mode 100644 crates/perry-runtime/src/box/scope_release.rs create mode 100644 test-files/test_gap_10464_box_cell_release.ts diff --git a/crates/perry-codegen/src/codegen/arguments.rs b/crates/perry-codegen/src/codegen/arguments.rs index 231a58fde4..ba72301e88 100644 --- a/crates/perry-codegen/src/codegen/arguments.rs +++ b/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, + slots: &std::collections::HashMap, +) { + 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 { + 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(¶m_id).cloned() { + // #10464: the object aliases the cell for its own lifetime. + ctx.func.forget_pre_return_box_release(¶m_slot); let box_ptr = ctx.block().load(I64, ¶m_slot); ctx.block().call_void( "js_arguments_object_map_index", diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index 2772017112..8ce9a39ab6 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/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 diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index f4522072b9..cbc2d25c47 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/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. diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 17efc82af9..0e77806ec2 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/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 = module_global_types .iter() diff --git a/crates/perry-codegen/src/codegen/method_static.rs b/crates/perry-codegen/src/codegen/method_static.rs index f33abb28f3..eef81c0da4 100644 --- a/crates/perry-codegen/src/codegen/method_static.rs +++ b/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 diff --git a/crates/perry-codegen/src/expr/closure.rs b/crates/perry-codegen/src/expr/closure.rs index f53c588aef..7abb8bb0a8 100644 --- a/crates/perry-codegen/src/expr/closure.rs +++ b/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> { + fn walk(stmts: &[Stmt], out: &mut Option>) { + 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 { @@ -130,6 +149,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // 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 = 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 { } 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 { // // 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 { && !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 { // 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::>(); 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; diff --git a/crates/perry-codegen/src/function.rs b/crates/perry-codegen/src/function.rs index dff3fb48e2..641036385a 100644 --- a/crates/perry-codegen/src/function.rs +++ b/crates/perry-codegen/src/function.rs @@ -178,6 +178,14 @@ pub struct LlFunction { /// Entry/module-init functions use this for process-level diagnostics /// that must run regardless of which block reaches the normal epilogue. pre_return_void_calls: Vec, + /// #10464: entry-alloca slots holding a variable-box cell this frame + /// minted, paired with the kind's `js_*box_scope_release`. Each `ret` + /// hands the slot's current cell back to the runtime (a no-op for a slot + /// still holding its TAG_UNDEFINED entry sentinel). + pre_return_box_releases: Vec<(String, &'static str)>, + /// Slots withdrawn by [`Self::forget_pre_return_box_release`]; a later + /// registration of the same slot stays withdrawn. + withheld_box_release_slots: Vec, } /// Render the frame-push instruction. Kept in one place so the eager @@ -282,6 +290,8 @@ impl LlFunction { stack_map_slot_count: 0, force_shadow_frame: false, pre_return_void_calls: Vec::new(), + pre_return_box_releases: Vec::new(), + withheld_box_release_slots: Vec::new(), } } @@ -497,6 +507,29 @@ impl LlFunction { self.pre_return_void_calls.push(func_name.into()); } + /// #10464: release the variable-box cell held by `slot` before every + /// `ret`. `slot` must be an entry-block alloca whose value is a box + /// pointer or TAG_UNDEFINED on every path. Idempotent per slot. + pub fn add_pre_return_box_release(&mut self, slot: &str, release_fn: &'static str) { + if !self.pre_return_box_releases.iter().any(|(s, _)| s == slot) + && !self.withheld_box_release_slots.iter().any(|s| s == slot) + { + self.pre_return_box_releases + .push((slot.to_string(), release_fn)); + } + } + + /// Withdraw a slot registered by [`Self::add_pre_return_box_release`] + /// because a holder the runtime does not count (a mapped `arguments` + /// object, a plain-async step closure) received its cell. Sticky: the + /// slot is never released by this frame afterwards. + pub fn forget_pre_return_box_release(&mut self, slot: &str) { + self.pre_return_box_releases.retain(|(s, _)| s != slot); + if !self.withheld_box_release_slots.iter().any(|s| s == slot) { + self.withheld_box_release_slots.push(slot.to_string()); + } + } + /// Invoke-EH (#7302): enter/leave a handler scope. While a scope is /// active, every potentially-throwing call any block of this function /// emits carries an unwind edge to the scope's landing-pad label. @@ -1092,8 +1125,9 @@ impl LlFunction { &self, sink: &mut dyn FnMut(FinalItem<'_>) -> Result<(), E>, ) -> Result<(), E> { - let rewrite_rets = - self.shadow_frame_slot.is_some() || !self.pre_return_void_calls.is_empty(); + let rewrite_rets = self.shadow_frame_slot.is_some() + || !self.pre_return_void_calls.is_empty() + || !self.pre_return_box_releases.is_empty(); let mut seq: u32 = 0; for (i, blk) in self.blocks.iter().enumerate() { if i > 0 { @@ -1203,6 +1237,18 @@ impl LlFunction { for func_name in &self.pre_return_void_calls { sink(FinalItem::Text(&format!(" call void @{}()", func_name)))?; } + for (slot, release_fn) in &self.pre_return_box_releases { + let load_reg = format!("%box_release_l_{}", seq); + *seq += 1; + sink(FinalItem::Text(&format!( + " {} = load i64, ptr {}", + load_reg, slot + )))?; + sink(FinalItem::Text(&format!( + " call void @{}(i64 {})", + release_fn, load_reg + )))?; + } if let Some(handle_slot) = &self.shadow_frame_slot { let load_reg = format!("%shadow_pop_l_{}", seq); *seq += 1; diff --git a/crates/perry-codegen/src/gc_call_effects.rs b/crates/perry-codegen/src/gc_call_effects.rs index 265da914e7..6be9a6379a 100644 --- a/crates/perry-codegen/src/gc_call_effects.rs +++ b/crates/perry-codegen/src/gc_call_effects.rs @@ -258,7 +258,13 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect { // collection trigger — the same audit as the accessors above. | "js_box_release" | "js_i32_box_release" - | "js_bool_box_release" => GcCallEffect::CannotCollect, + | "js_bool_box_release" + // #10464 scope-exit release: a registry probe, a capture-count + // lookup, then either the same publish (registry remove, cache evict, + // raw clear, TLS free-list push) or a TLS pending-map insert. + | "js_box_scope_release" + | "js_i32_box_scope_release" + | "js_bool_box_scope_release" => GcCallEffect::CannotCollect, // Audited allocate-but-never-reenter helpers (2026-07-31): each body // was checked for closure invocation, coercion (valueOf/toString), // and accessor dispatch — none present, and none takes a receiver @@ -835,6 +841,9 @@ mod tests { "js_box_release", "js_i32_box_release", "js_bool_box_release", + "js_box_scope_release", + "js_i32_box_scope_release", + "js_bool_box_scope_release", ] { assert_eq!( classify_direct_callee(name), diff --git a/crates/perry-codegen/src/lower_call/new_ctor_args.rs b/crates/perry-codegen/src/lower_call/new_ctor_args.rs index de9bd00491..b4948fc297 100644 --- a/crates/perry-codegen/src/lower_call/new_ctor_args.rs +++ b/crates/perry-codegen/src/lower_call/new_ctor_args.rs @@ -75,6 +75,7 @@ pub(crate) fn bind_inline_constructor_params( .collect(); crate::codegen::arguments::add_arguments_mapped_boxes(params, &mut ctx.boxed_vars); + let mapped_param_ids = crate::codegen::arguments::mapped_parameter_ids(params); let values = inline_constructor_param_values_with_class(ctx, params, lowered_args, capture_fill); for ((param, arg_val), proof) in params @@ -86,7 +87,21 @@ pub(crate) fn bind_inline_constructor_params( let slot = ctx .func .alloca_entry(if boxed_param { I64 } else { DOUBLE }); - if boxed_param { + if boxed_param && !mapped_param_ids.contains(¶m.id) { + // #10464: this frame mints the cell (again per iteration when the + // `new` sits in a loop), so it also releases it. + let arg_bits = ctx.block().bitcast_double_to_i64(arg_val); + ctx.func + .entry_allocas_push_store(I64, crate::nanbox::TAG_UNDEFINED_I64, &slot); + use crate::stmt::boxed_frame_release as frame_release; + frame_release::mint_frame_cell( + ctx, + &slot, + "js_box_alloc_bits", + &[(I64, &arg_bits)], + frame_release::JS_BOX_SCOPE_RELEASE, + ); + } else if boxed_param { let arg_bits = ctx.block().bitcast_double_to_i64(arg_val); let box_ptr = ctx .block() diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index a73cd55079..3988b944c4 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1117,6 +1117,10 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { module.declare_function("js_box_release", VOID, &[I64]); module.declare_function("js_i32_box_release", VOID, &[I64]); module.declare_function("js_bool_box_release", VOID, &[I64]); + // #10464: frame-exit release of cells an ordinary frame minted. + module.declare_function("js_box_scope_release", VOID, &[I64]); + module.declare_function("js_i32_box_scope_release", VOID, &[I64]); + module.declare_function("js_bool_box_scope_release", VOID, &[I64]); module.declare_function("js_bool_box_alloc", I64, &[I32]); module.declare_function("js_bool_box_get", I32, &[I64]); module.declare_function("js_bool_box_set", VOID, &[I64, I32]); diff --git a/crates/perry-codegen/src/stmt/boxed_frame_release.rs b/crates/perry-codegen/src/stmt/boxed_frame_release.rs new file mode 100644 index 0000000000..f01aef30a0 --- /dev/null +++ b/crates/perry-codegen/src/stmt/boxed_frame_release.rs @@ -0,0 +1,65 @@ +//! #10464: hand a frame's variable-box cells back to the runtime when the +//! frame can no longer name them. +//! +//! A boxed local's entry alloca holds the cell this frame minted (or its +//! TAG_UNDEFINED entry sentinel). Two points end the frame's hold on that +//! cell: every `ret` (registered here, emitted by the `LlFunction` return-site +//! rewrite so no return path can be missed), and a declaration inside a loop +//! minting the next iteration's cell into the same slot. The runtime +//! (`perry_runtime::r#box::scope_release`) publishes a cell nothing else +//! captured and leaves a closure-captured cell to its closures' GC death. +//! +//! Only holders the runtime counts may keep a released cell alive. The two +//! compiler-emitted holders it does not count withdraw the slot instead: +//! a mapped sloppy-mode `arguments` object (`codegen/arguments.rs`) and a +//! plain-async step closure's own activation cells (`expr/closure.rs`). + +use crate::expr::FnCtx; +use crate::types::{LlvmType, I64}; + +pub(crate) const JS_BOX_SCOPE_RELEASE: &str = "js_box_scope_release"; +pub(crate) const I32_BOX_SCOPE_RELEASE: &str = "js_i32_box_scope_release"; +pub(crate) const BOOL_BOX_SCOPE_RELEASE: &str = "js_bool_box_scope_release"; + +/// Release `slot`'s cell before every `ret` of the current function. +pub(crate) fn release_at_frame_exit(ctx: &mut FnCtx<'_>, slot: &str, release_fn: &'static str) { + ctx.func.add_pre_return_box_release(slot, release_fn); +} + +/// A declaration about to mint a fresh cell into `slot` re-executes only in a +/// loop; the previous iteration's cell is then unnameable by this frame. +/// Outside a loop the slot still holds its entry sentinel, so nothing is +/// emitted. `switch` frames push an empty continue label and do not count. +pub(crate) fn release_previous_iteration_cell( + ctx: &mut FnCtx<'_>, + slot: &str, + release_fn: &'static str, +) { + let in_loop = ctx + .loop_targets + .iter() + .any(|(continue_label, _, _)| !continue_label.is_empty()); + if !in_loop { + return; + } + let previous = ctx.block().load(I64, slot); + ctx.block().call_void(release_fn, &[(I64, &previous)]); +} + +/// Store a freshly minted cell (`alloc_fn(args)`) into this frame's entry +/// `slot`: release the previous iteration's cell first (so an uncaptured one +/// is reused by this very allocation), and release the slot at frame exit. +/// Returns the new cell pointer. +pub(crate) fn mint_frame_cell( + ctx: &mut FnCtx<'_>, + slot: &str, + alloc_fn: &str, + args: &[(LlvmType, &str)], + release_fn: &'static str, +) -> String { + release_previous_iteration_cell(ctx, slot, release_fn); + let cell = ctx.block().call(I64, alloc_fn, args); + ctx.block().store(I64, &cell, slot); + release_at_frame_exit(ctx, slot, release_fn); + cell +} diff --git a/crates/perry-codegen/src/stmt/boxed_frame_release_tests.rs b/crates/perry-codegen/src/stmt/boxed_frame_release_tests.rs new file mode 100644 index 0000000000..08f10d66ba --- /dev/null +++ b/crates/perry-codegen/src/stmt/boxed_frame_release_tests.rs @@ -0,0 +1,293 @@ +//! #10464: an ordinary frame releases the box cells it minted. +//! +//! Each fixture pairs the released shape with the holder the runtime cannot +//! count, so the assertions discriminate in both directions: dropping the +//! frame release loses the positive assertions, releasing too much trips the +//! negative ones. + +use perry_hir::types::Type; +use perry_hir::{ArgumentsObjectMeta, Expr, Function, Module, Param, Stmt}; + +const RELEASE: &str = "call void @js_box_scope_release(i64 "; + +fn param(id: u32, name: &str) -> Param { + Param { + id, + name: name.into(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + } +} + +fn closure(func_id: u32, body: Vec, captures: Vec) -> Expr { + Expr::Closure { + func_id, + params: Vec::new(), + return_type: Type::Any, + body, + mutable_captures: captures.clone(), + captures, + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: true, + } +} + +fn let_stmt(id: u32, init: Expr) -> Stmt { + Stmt::Let { + id, + name: format!("v{id}"), + ty: Type::Any, + mutable: true, + init: Some(init), + } +} + +fn function(name: &str, params: Vec, body: Vec) -> Function { + Function { + id: 1, + name: name.into(), + type_params: Vec::new(), + params, + return_type: Type::Any, + body, + is_async: false, + is_generator: false, + is_strict: true, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + } +} + +fn function_ir(f: Function) -> String { + let mut module = Module::new("boxed_frame_release.ts"); + let needle = format!("__{}(", f.name); + module.functions.push(f); + let ir = String::from_utf8( + crate::compile_module(&module, super::prealloc_module_global_tests::ir_opts()).unwrap(), + ) + .unwrap(); + ir.split("\ndefine ") + .find(|block| block.lines().next().is_some_and(|l| l.contains(&needle))) + .unwrap_or_else(|| panic!("no define for {needle}:\n{ir}")) + .to_string() +} + +/// Every `ret` of the frame is preceded by one release per slot it names. +fn releases_before_each_ret(ir: &str) -> Vec { + let lines: Vec<&str> = ir.lines().collect(); + let mut counts = Vec::new(); + for (i, line) in lines.iter().enumerate() { + if line.trim_start().starts_with("ret ") { + let mut n = 0; + for prev in lines[..i].iter().rev() { + let t = prev.trim_start(); + if t.starts_with("call void @js_shadow_frame_pop") || t.contains("= load i64, ptr") + { + continue; + } + if t.starts_with(RELEASE) { + n += 1; + continue; + } + break; + } + counts.push(n); + } + } + counts +} + +#[test] +fn captured_reassigned_let_is_released_at_every_return() { + // function counter(flag) { let n = 0; const inc = () => { n = 1 }; + // if (flag) return inc; return n; } + let body = vec![ + let_stmt(10, Expr::Integer(0)), + let_stmt( + 11, + closure( + 2, + vec![Stmt::Expr(Expr::LocalSet(10, Box::new(Expr::Integer(1))))], + vec![10], + ), + ), + Stmt::If { + condition: Expr::LocalGet(1), + then_branch: vec![Stmt::Return(Some(Expr::LocalGet(11)))], + else_branch: None, + }, + Stmt::Return(Some(Expr::LocalGet(10))), + ]; + let ir = function_ir(function("counter", vec![param(1, "flag")], body)); + assert!( + ir.contains("call i64 @js_box_alloc_bits("), + "premise: n is boxed\n{ir}" + ); + assert!( + ir.contains("js_closure_set_box_capture_ptr"), + "premise: counted edge\n{ir}" + ); + let per_ret = releases_before_each_ret(&ir); + assert!(per_ret.len() >= 2, "both returns must be lowered:\n{ir}"); + assert!( + per_ret.iter().all(|n| *n == 1), + "each return releases the single frame-owned cell ({per_ret:?}):\n{ir}" + ); + // Outside a loop the declaration runs once: no previous-iteration release. + let alloc = ir.find("call i64 @js_box_alloc_bits(").unwrap(); + assert!( + !ir[..alloc].contains(RELEASE), + "no release before a one-shot mint:\n{ir}" + ); +} + +#[test] +fn loop_declaration_releases_the_previous_iterations_cell_before_minting() { + // while (flag) { let x = 0; keep = () => { x = 1 }; } + let body = vec![ + Stmt::While { + condition: Expr::LocalGet(1), + body: vec![ + let_stmt(20, Expr::Integer(0)), + let_stmt( + 21, + closure( + 3, + vec![Stmt::Expr(Expr::LocalSet(20, Box::new(Expr::Integer(1))))], + vec![20], + ), + ), + ], + }, + Stmt::Return(Some(Expr::Undefined)), + ]; + let ir = function_ir(function("looped", vec![param(1, "flag")], body)); + let lines: Vec<&str> = ir.lines().map(str::trim_start).collect(); + let alloc = lines + .iter() + .position(|l| l.contains("call i64 @js_box_alloc_bits(")) + .expect("premise: x is boxed"); + let cell = lines[alloc].split(" = ").next().unwrap(); + let slot = lines[alloc..] + .iter() + .find_map(|l| l.strip_prefix(&format!("store i64 {cell}, ptr "))) + .expect("the minted cell is stored in its slot"); + let previous = lines[..alloc] + .iter() + .rev() + .take(4) + .find_map(|l| l.strip_suffix(&format!(" = load i64, ptr {slot}"))) + .unwrap_or_else(|| panic!("the slot's previous cell is loaded before minting:\n{ir}")); + assert!( + lines[..alloc] + .iter() + .rev() + .take(4) + .any(|l| l.starts_with(&format!("{RELEASE}{previous})"))), + "and released before the next iteration's cell is minted:\n{ir}" + ); + assert!( + releases_before_each_ret(&ir).iter().all(|n| *n == 1), + "{ir}" + ); +} + +#[test] +fn mapped_arguments_parameter_is_never_released_by_its_frame() { + // Sloppy `function f(a, b) { const g = () => { a = 1; b = 2 }; return arguments; }` + // with only `a` mapped: the Arguments object aliases a's cell raw. + let mut arguments = param(3, "arguments"); + arguments.arguments_object = Some(ArgumentsObjectMeta { + strict: false, + simple_parameters: true, + mapped_parameter_ids: vec![(0, 1)], + restricted_callee: false, + }); + let body = vec![ + let_stmt( + 30, + closure( + 4, + vec![ + Stmt::Expr(Expr::LocalSet(1, Box::new(Expr::Integer(1)))), + Stmt::Expr(Expr::LocalSet(2, Box::new(Expr::Integer(2)))), + ], + vec![1, 2], + ), + ), + Stmt::Return(Some(Expr::LocalGet(3))), + ]; + let mut f = function( + "sloppy", + vec![param(1, "a"), param(2, "b"), arguments], + body, + ); + f.is_strict = false; + let ir = function_ir(f); + assert!( + ir.contains("js_arguments_object_map_index"), + "premise: a is mapped\n{ir}" + ); + assert_eq!( + ir.matches("call i64 @js_box_alloc_bits(").count(), + 2, + "premise: both parameters are boxed:\n{ir}" + ); + let per_ret = releases_before_each_ret(&ir); + assert!( + !per_ret.is_empty() && per_ret.iter().all(|n| *n == 1), + "only the unmapped parameter is released ({per_ret:?}):\n{ir}" + ); +} + +#[test] +fn plain_async_step_counts_only_enclosing_cells_and_frame_keeps_its_own() { + // An activation frame: OWN is named by the step closure's terminal + // `ReleaseBoxes`, OUTER is an ordinary captured-and-reassigned local. + const OWN: u32 = 40; + const OUTER: u32 = 41; + let step = closure( + 5, + vec![ + Stmt::Expr(Expr::LocalSet(OUTER, Box::new(Expr::Integer(1)))), + Stmt::Expr(Expr::LocalSet(OWN, Box::new(Expr::Integer(2)))), + Stmt::ReleaseBoxes(vec![OWN]), + Stmt::Return(Some(Expr::Undefined)), + ], + vec![OWN, OUTER], + ); + let body = vec![ + Stmt::PreallocateBoxes(vec![OWN]), + let_stmt(OUTER, Expr::Integer(0)), + let_stmt(42, step), + Stmt::Return(Some(Expr::LocalGet(42))), + ]; + let ir = function_ir(function("activation", Vec::new(), body)); + assert_eq!( + ir.matches("call i64 @js_box_alloc_bits(").count(), + 2, + "premise: both cells are minted by this frame:\n{ir}" + ); + assert_eq!( + ir.matches("call void @js_closure_set_box_capture_ptr(") + .count(), + 1, + "the enclosing cell is a counted edge, the activation's own is not:\n{ir}" + ); + let per_ret = releases_before_each_ret(&ir); + assert!( + !per_ret.is_empty() && per_ret.iter().all(|n| *n == 1), + "the frame releases OUTER only; OWN belongs to the activation ({per_ret:?}):\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/stmt/boxed_local_init.rs b/crates/perry-codegen/src/stmt/boxed_local_init.rs index 418f7f2377..9fd2dc5d8d 100644 --- a/crates/perry-codegen/src/stmt/boxed_local_init.rs +++ b/crates/perry-codegen/src/stmt/boxed_local_init.rs @@ -24,18 +24,30 @@ pub(super) fn ensure_reused_box_is_initialized(ctx: &mut FnCtx<'_>, id: u32) { let ready_label = ctx.block_label(ready); ctx.block().cond_br(&missing, &allocate_label, &ready_label); ctx.current_block = allocate; - let cell = if crate::expr::is_compiler_private_async_i32_control_local(ctx, id) { - ctx.block().call(I64, "js_i32_box_alloc", &[(I32, "0")]) + use super::boxed_frame_release as frame_release; + let (cell, release_fn) = if crate::expr::is_compiler_private_async_i32_control_local(ctx, id) { + ( + ctx.block().call(I64, "js_i32_box_alloc", &[(I32, "0")]), + frame_release::I32_BOX_SCOPE_RELEASE, + ) } else if crate::expr::is_compiler_private_async_i1_control_local(ctx, id) { - ctx.block().call(I64, "js_bool_box_alloc", &[(I32, "0")]) + ( + ctx.block().call(I64, "js_bool_box_alloc", &[(I32, "0")]), + frame_release::BOOL_BOX_SCOPE_RELEASE, + ) } else { - ctx.block().call( - I64, - "js_box_alloc_bits", - &[(I64, crate::nanbox::TAG_UNDEFINED_I64)], + ( + ctx.block().call( + I64, + "js_box_alloc_bits", + &[(I64, crate::nanbox::TAG_UNDEFINED_I64)], + ), + frame_release::JS_BOX_SCOPE_RELEASE, ) }; ctx.block().store(I64, &cell, &slot); + // #10464: the cell is this frame's (a withdrawn slot stays withdrawn). + frame_release::release_at_frame_exit(ctx, &slot, release_fn); super::record_boxed_slot_js_value_bits(ctx, id, &cell, "boxed_let.reused_missing_box"); ctx.block().br(&ready_label); ctx.current_block = ready; diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index 251f4bbd8a..f360f0a353 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -1242,13 +1242,6 @@ pub(crate) fn lower_let( } return Ok(()); } - // Step 1: allocate box with undefined sentinel bits. - let blk = ctx.block(); - let box_ptr = blk.call( - crate::types::I64, - "js_box_alloc_bits", - &[(I64, crate::nanbox::TAG_UNDEFINED_I64)], - ); // Slot must live in the entry block — closures from sibling // branches may capture this id later, and an alloca placed // here would not dominate those branches' loads. @@ -1267,7 +1260,14 @@ pub(crate) fn lower_let( // deterministically. let undef_bits = crate::nanbox::TAG_UNDEFINED_I64.to_string(); ctx.func.entry_allocas_push_store(I64, &undef_bits, &slot); - ctx.block().store(I64, &box_ptr, &slot); + // Step 1: allocate the box (#10464: released by this frame). + let box_ptr = super::boxed_frame_release::mint_frame_cell( + ctx, + &slot, + "js_box_alloc_bits", + &[(I64, crate::nanbox::TAG_UNDEFINED_I64)], + super::boxed_frame_release::JS_BOX_SCOPE_RELEASE, + ); super::record_boxed_slot_js_value_bits(ctx, id, &box_ptr, "boxed_let.box_ptr_slot"); // Step 2: register BEFORE lowering init. ctx.locals.insert(id, slot); diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index a6ca584e6a..4846931d95 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -13,6 +13,9 @@ use crate::types::DOUBLE; #[cfg(test)] mod boxed_continuation_tests; +pub(crate) mod boxed_frame_release; +#[cfg(test)] +mod boxed_frame_release_tests; mod boxed_local_init; #[cfg(test)] mod boxed_slot_no_root_tests; @@ -665,6 +668,14 @@ pub(crate) fn lower_stmt(ctx: &mut FnCtx<'_>, stmt: &Stmt) -> Result<()> { } fn emit_preallocate_boxes(ctx: &mut FnCtx<'_>, ids: &[u32], tdz: bool) -> Result<()> { + // #10464: a generator/async activation frame's list names its + // compiler-private control cells. That list runs once per frame, and a + // plain-async step closure holds its cells without a counted capture edge, + // so it never releases a "previous iteration" cell. + let activation_frame = ids.iter().any(|id| { + ctx.compiler_private_async_i32_control_locals.contains(id) + || ctx.compiler_private_async_i1_control_locals.contains(id) + }); for id in ids { // #7521: a module-level binding promoted to `@perry_global___` // ALREADY has the shared, forward-visible, GC-rooted cell a prealloc box @@ -701,41 +712,35 @@ fn emit_preallocate_boxes(ctx: &mut FnCtx<'_>, ids: &[u32], tdz: bool) -> Result } let is_i32_control = crate::expr::is_compiler_private_async_i32_control_local(ctx, *id); let is_i1_control = crate::expr::is_compiler_private_async_i1_control_local(ctx, *id); - let blk = ctx.block(); - let (box_ptr, cell_note) = if is_i32_control { + // Seed the JSValue box with TAG_TDZ (Temporal Dead Zone) when + // requested -- a read before the declaration runs throws a spec + // ReferenceError via the runtime `js_box_get_bits` choke point. + // Compiler-private i32/i1 control cells are never TDZ. + let seed_bits = if tdz { + crate::nanbox::TAG_TDZ_I64.to_string() + } else { + crate::nanbox::TAG_UNDEFINED_I64.to_string() + }; + use boxed_frame_release as frame_release; + let (alloc_fn, alloc_arg, release_fn, cell_note) = if is_i32_control { ( - blk.call( - crate::types::I64, - "js_i32_box_alloc", - &[(crate::types::I32, "0")], - ), + "js_i32_box_alloc", + (crate::types::I32, "0"), + frame_release::I32_BOX_SCOPE_RELEASE, "primitive_i32_control_cell", ) } else if is_i1_control { ( - blk.call( - crate::types::I64, - "js_bool_box_alloc", - &[(crate::types::I32, "0")], - ), + "js_bool_box_alloc", + (crate::types::I32, "0"), + frame_release::BOOL_BOX_SCOPE_RELEASE, "primitive_i1_control_cell", ) } else { - // Seed the JSValue box with TAG_TDZ (Temporal Dead Zone) when - // requested -- a read before the declaration runs throws a spec - // ReferenceError via the runtime `js_box_get_bits` choke point. - // Compiler-private i32/i1 control cells are never TDZ. - let seed_bits = if tdz { - crate::nanbox::TAG_TDZ_I64.to_string() - } else { - crate::nanbox::TAG_UNDEFINED_I64.to_string() - }; ( - blk.call( - crate::types::I64, - "js_box_alloc_bits", - &[(crate::types::I64, &seed_bits)], - ), + "js_box_alloc_bits", + (crate::types::I64, seed_bits.as_str()), + frame_release::JS_BOX_SCOPE_RELEASE, "jsvalue_box_cell", ) }; @@ -762,7 +767,12 @@ fn emit_preallocate_boxes(ctx: &mut FnCtx<'_>, ids: &[u32], tdz: bool) -> Result .entry_allocas_push_store(crate::types::I64, &undef_bits, &slot); slot }; + if !activation_frame { + frame_release::release_previous_iteration_cell(ctx, &slot, release_fn); + } + let box_ptr = ctx.block().call(crate::types::I64, alloc_fn, &[alloc_arg]); ctx.block().store(crate::types::I64, &box_ptr, &slot); + frame_release::release_at_frame_exit(ctx, &slot, release_fn); record_boxed_slot_js_value_bits(ctx, *id, &box_ptr, "preallocate_boxes.box_ptr_slot"); if cell_note != "jsvalue_box_cell" { let lowered = LoweredValue::js_value_bits(&box_ptr); diff --git a/crates/perry-runtime/src/box.rs b/crates/perry-runtime/src/box.rs index 4eae4b605b..77e7ebf71b 100644 --- a/crates/perry-runtime/src/box.rs +++ b/crates/perry-runtime/src/box.rs @@ -457,27 +457,29 @@ fn push_free_cell(addr: usize, head: &'static crate::tls_hot::HotKey { - BOX_REGISTRY.with(|r| { - r.borrow_mut().remove(&addr); - }); + if !BOX_REGISTRY.with(|r| r.borrow_mut().remove(&addr)) { + return; + } box_ptr_cache_evict(box_ptr_cache(), addr); unsafe { (*(addr as *mut Box)).value = crate::value::TAG_UNDEFINED }; push_free_cell(addr, &BOX_FREE_HEAD); } ASYNC_RELEASE_I32 => { - I32_BOX_REGISTRY.with(|r| { - r.borrow_mut().remove(&addr); - }); + if !I32_BOX_REGISTRY.with(|r| r.borrow_mut().remove(&addr)) { + return; + } box_ptr_cache_evict(i32_box_ptr_cache(), addr); unsafe { (*(addr as *mut I32Box)).value = -1 }; push_free_cell(addr, &I32_BOX_FREE_HEAD); } ASYNC_RELEASE_BOOL => { - BOOL_BOX_REGISTRY.with(|r| { - r.borrow_mut().remove(&addr); - }); + if !BOOL_BOX_REGISTRY.with(|r| r.borrow_mut().remove(&addr)) { + return; + } box_ptr_cache_evict(bool_box_ptr_cache(), addr); unsafe { (*(addr as *mut BoolBox)).value = true }; push_free_cell(addr, &BOOL_BOX_FREE_HEAD); @@ -485,7 +487,10 @@ fn publish_box_cell(addr: usize, tag: usize) { _ => unreachable!("invalid async released-cell tag"), } ASYNC_PENDING_RELEASES.with(|pending| { - pending.borrow_mut().remove(&addr); + let mut pending = pending.borrow_mut(); + if !pending.is_empty() { + pending.remove(&addr); + } }); BOX_FLUSH_PUBLISHED.fetch_add(1, Ordering::Relaxed); } @@ -499,21 +504,22 @@ pub(crate) fn box_capture_count_reached_zero(addr: usize) { } } -/// Expose a drained, closure-owned JS box's payload to the closure tracer. -/// The exact-capture table may also contain i32/bool box addresses; requiring -/// the pending JS tag is the authoritative type discriminator before the -/// pointer is dereferenced as [`Box`]. +/// Expose a released, closure-owned JS box's payload to the closure tracer — +/// a drained async activation cell, or one its ordinary frame released +/// (#10464). The exact-capture table may also contain i32/bool box addresses; +/// requiring the JS tag on the release record is the authoritative type +/// discriminator before the pointer is dereferenced as [`Box`]. pub(crate) fn visit_pending_captured_js_box_payload_slot( addr: usize, visit: &mut dyn FnMut(*mut u64), ) { - let is_pending_js = ASYNC_PENDING_RELEASES.with(|pending| { + let is_released_js = ASYNC_PENDING_RELEASES.with(|pending| { pending .borrow() .get(&addr) .is_some_and(|tag| *tag == (ASYNC_RELEASE_JS | ASYNC_RELEASE_DRAINED)) - }); - if is_pending_js && BOX_REGISTRY.with(|registry| registry.borrow().contains(&addr)) { + }) || crate::closure::frame_released_js_cell(addr, ASYNC_RELEASE_JS); + if is_released_js && BOX_REGISTRY.with(|registry| registry.borrow().contains(&addr)) { let ptr = addr as *mut Box; unsafe { visit(&raw mut (*ptr).value) }; } @@ -965,19 +971,6 @@ pub fn scan_box_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { BOX_REGISTRY.with(|r| { let r = r.borrow(); for &addr in r.iter() { - // A drained box is retained only by exact closure-capture - // metadata. During a full trace its payload is reached from - // each live closure instead. Rooting it here as well would - // make `box -> closure -> same box` an uncollectable native - // cycle. Minors retain the old strong-root rule because they - // cannot adjudicate old-closure liveness. - if full_trace - && pending - .get(&addr) - .is_some_and(|tag| *tag == (ASYNC_RELEASE_JS | ASYNC_RELEASE_DRAINED)) - { - continue; - } let ptr = addr as *mut Box; // Defensive: the registry should only contain valid live // pointers, but if a stale entry slipped through we'd @@ -985,15 +978,40 @@ pub fn scan_box_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { // address (alloc gives 8-aligned pointers in user space) // matches `is_plausible_box_ptr` to keep this a no-op for // any pathological entry. - if addr >= 0x1000 && (addr as u64) < 0x0001_0000_0000_0000 && addr % 8 == 0 { + if !(addr >= 0x1000 && (addr as u64) < 0x0001_0000_0000_0000 && addr % 8 == 0) { + continue; + } + // A released box (a drained async cell, or a cell its + // ordinary frame released — #10464) is retained only by exact + // closure-capture metadata. During a full trace its payload is + // reached from each live closure instead. Rooting it here as + // well would make `box -> closure -> same box` an uncollectable + // native cycle. Minors retain the old strong-root rule because + // they cannot adjudicate old-closure liveness — and a minor + // walks ONLY the log below, so a young payload must stay logged + // even when this trace does not visit it. Dropping it here left + // the next minor with no root for a live payload + // (`gc-fromspace-protect` catches the stale use immediately). + if full_trace + && (pending + .get(&addr) + .is_some_and(|tag| *tag == (ASYNC_RELEASE_JS | ASYNC_RELEASE_DRAINED)) + || crate::closure::frame_released_js_cell(addr, ASYNC_RELEASE_JS)) + { unsafe { - visitor.visit_nanbox_u64_raw_slot(&raw mut (*ptr).value); if crate::gc::young_log::bits_are_minor_relevant((*ptr).value) { kept.push(addr); } } - visited += 1; + continue; } + unsafe { + visitor.visit_nanbox_u64_raw_slot(&raw mut (*ptr).value); + if crate::gc::young_log::bits_are_minor_relevant((*ptr).value) { + kept.push(addr); + } + } + visited += 1; } }); }); @@ -1905,3 +1923,11 @@ mod tests { #[cfg(test)] #[path = "box/release_tests.rs"] mod release_tests; + +// #10464: scope-exit release for frame-owned cells. +#[path = "box/scope_release.rs"] +mod scope_release; +pub(crate) use scope_release::publish_frame_released_cell; +pub use scope_release::{ + js_bool_box_scope_release, js_box_scope_release, js_i32_box_scope_release, +}; diff --git a/crates/perry-runtime/src/box/scope_release.rs b/crates/perry-runtime/src/box/scope_release.rs new file mode 100644 index 0000000000..b9274fc5ef --- /dev/null +++ b/crates/perry-runtime/src/box/scope_release.rs @@ -0,0 +1,346 @@ +//! #10464: release the box cells an ordinary frame minted once that frame can +//! no longer name them. +//! +//! A variable box is minted per execution of the declaration of a captured and +//! reassigned binding, and every registered cell is a strong GC root +//! (`scan_box_roots_mut`). Before #10464 only a lowered plain-async activation +//! ever released its cells (#7933/#8208), so a synchronous function, method, +//! arrow, generator, or an `async` function without `await` kept one registered +//! root per boxed binding per call alive for the life of the thread, together +//! with everything that binding last pointed at. +//! +//! Codegen now names each frame-owned cell before every `ret` of the frame and +//! before a declaration inside a loop mints the next iteration's cell. The +//! frame is the holder that disappears at those points; every other holder is +//! a compiler-declared closure capture edge (`js_closure_set_box_capture_ptr`), +//! which is exactly the edge set #8303 counts. So: +//! +//! - a cell with no capture edge publishes immediately: de-registered, +//! positive-cache-evicted, cleared, and pushed on its kind's free list; +//! - a captured cell stays registered and readable, and its capture-edge record +//! is marked frame-released. It publishes when authoritative GC death +//! pruning removes its last capture edge. Like a drained async terminal cell, +//! a full trace reaches its payload only through live capturing closures (the +//! ephemeron half), so a payload that references its own closure does not +//! keep either alive. +//! +//! A running closure keeps its own edges: every capturing closure body spills +//! `%this_closure` into a frame-long root slot (#7055), so a cell a closure body +//! cached at entry cannot be published while that body is still executing. +//! +//! These entries are deliberately separate from `js_*box_release`, which parks +//! into the AMBIENT async activation: a synchronous callee running inside an +//! async step must not append its own cells to the caller activation's +//! contiguous terminal release range. + +use super::*; + +#[inline] +fn release_scope_cell(addr: usize, tag: usize, registered: bool) { + // Not registered: a TAG_UNDEFINED slot whose declaration never ran, an + // already-published cell, or a foreign address. All are no-ops, which is + // what makes a repeated release unable to push one cell twice. + if !registered { + return; + } + // An async activation's terminal cell: its activation already decided. + let async_pending = ASYNC_PENDING_RELEASES.with(|pending| { + let pending = pending.borrow(); + !pending.is_empty() && pending.contains_key(&addr) + }); + if async_pending { + return; + } + match crate::closure::note_frame_released_cell(addr, tag) { + crate::closure::FrameRelease::Uncaptured => { + BOX_RELEASE_COUNT.fetch_add(1, Ordering::Relaxed); + publish_box_cell(addr, tag); + } + crate::closure::FrameRelease::Deferred => { + BOX_RELEASE_COUNT.fetch_add(1, Ordering::Relaxed); + } + crate::closure::FrameRelease::AlreadyReleased => {} + } +} + +/// The last capture edge of a frame-released cell disappeared (GC death +/// pruning): publish it for reuse. +pub(crate) fn publish_frame_released_cell(addr: usize, tag: usize) { + publish_box_cell(addr, tag); +} + +/// Release a JSValue box cell owned by the exiting (or re-entering) frame. +#[no_mangle] +pub extern "C" fn js_box_scope_release(ptr: *mut Box) { + release_scope_cell(ptr as usize, ASYNC_RELEASE_JS, is_registered_box_ptr(ptr)); +} + +/// [`js_box_scope_release`] for the compiler-private i32 control cells of a +/// generator frame. +#[no_mangle] +pub extern "C" fn js_i32_box_scope_release(ptr: *mut I32Box) { + release_scope_cell( + ptr as usize, + ASYNC_RELEASE_I32, + is_registered_i32_box_ptr(ptr), + ); +} + +/// [`js_box_scope_release`] for the compiler-private boolean control cells of +/// a generator frame. +#[no_mangle] +pub extern "C" fn js_bool_box_scope_release(ptr: *mut BoolBox) { + release_scope_cell( + ptr as usize, + ASYNC_RELEASE_BOOL, + is_registered_bool_box_ptr(ptr), + ); +} + +#[cfg(feature = "keepalive-anchors")] +#[used(compiler)] +static KEEP_JS_BOX_SCOPE_RELEASE: extern "C" fn(*mut Box) = js_box_scope_release; +#[cfg(feature = "keepalive-anchors")] +#[used(compiler)] +static KEEP_JS_I32_BOX_SCOPE_RELEASE: extern "C" fn(*mut I32Box) = js_i32_box_scope_release; +#[cfg(feature = "keepalive-anchors")] +#[used(compiler)] +static KEEP_JS_BOOL_BOX_SCOPE_RELEASE: extern "C" fn(*mut BoolBox) = js_bool_box_scope_release; + +#[cfg(test)] +mod tests { + use super::*; + + fn int_bits(value: i32) -> i64 { + crate::value::JSValue::int32(value).bits() as i64 + } + + fn free_list_contains(addr: usize) -> bool { + let mut cursor = BOX_FREE_HEAD.with(std::cell::Cell::get); + let mut steps = 0; + while cursor != 0 && steps < 1 << 20 { + if cursor == addr { + return true; + } + cursor = unsafe { (cursor as *const usize).read() }; + steps += 1; + } + false + } + + /// The common case: the frame was the cell's only holder. The cell is + /// inert at once and the next allocation reuses it; repeating the release + /// (a second `ret` path, a loop that exits right after re-entering) must + /// not push it onto the free list a second time. + #[test] + fn uncaptured_cell_publishes_at_scope_exit_exactly_once() { + test_clear_box_registry(); + let cell = js_box_alloc_bits(int_bits(7)); + js_box_scope_release(cell); + assert!(!is_registered_box_ptr(cell), "published cell de-registers"); + assert_eq!(js_box_get_bits(cell) as u64, crate::value::TAG_UNDEFINED); + js_box_scope_release(cell); + js_box_scope_release(cell); + + let first = js_box_alloc_bits(int_bits(1)); + let second = js_box_alloc_bits(int_bits(2)); + assert_eq!(first, cell, "the published cell is reused immediately"); + assert_ne!( + first, second, + "a repeated release must not alias two live bindings onto one cell" + ); + assert_eq!(js_box_get_bits(first), int_bits(1)); + assert_eq!(js_box_get_bits(second), int_bits(2)); + } + + /// A slot whose declaration never ran still holds TAG_UNDEFINED; codegen + /// releases it unconditionally at `ret`. Foreign pointers are rejected + /// by the same registry gate as every other box entry point. + #[test] + fn unminted_slot_values_and_foreign_pointers_are_no_ops() { + test_clear_box_registry(); + let live = js_box_alloc_bits(int_bits(3)); + js_box_scope_release(crate::value::TAG_UNDEFINED as usize as *mut Box); + js_box_scope_release(std::ptr::null_mut()); + static RODATA: [u64; 1] = [0xDEAD_BEEF]; + js_box_scope_release((&RODATA[0] as *const u64) as *mut Box); + assert_eq!(RODATA[0], 0xDEAD_BEEF); + assert!(is_registered_box_ptr(live)); + assert_eq!(js_box_get_bits(live), int_bits(3)); + assert!(!free_list_contains(live as usize)); + } + + /// `function counter() { let n = 0; return () => ++n; }`: the returned + /// closure outlives the frame. Its cell stays readable and writable after + /// scope exit, a closure created later from that closure adds its own + /// edge, and only the last closure's death publishes the cell. + #[test] + fn escaped_closure_keeps_its_cell_until_gc_death() { + test_clear_box_registry(); + let cell = js_box_alloc_bits(int_bits(0)); + let counter = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::js_closure_set_box_capture_ptr(counter, 0, cell as i64); + js_box_scope_release(cell); + + assert!(is_registered_box_ptr(cell), "an escaped closure owns it"); + js_box_set_bits(cell, int_bits(1)); + assert_eq!(js_box_get_bits(cell), int_bits(1)); + assert!(!free_list_contains(cell as usize)); + js_box_scope_release(cell); + assert!( + crate::closure::frame_released_js_cell(cell as usize, ASYNC_RELEASE_JS), + "a repeated scope release leaves the released state unchanged" + ); + assert_eq!(crate::closure::box_capture_count(cell as usize), 1); + + let child = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::js_closure_set_box_capture_ptr(child, 0, cell as i64); + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == counter as usize); + assert!(is_registered_box_ptr(cell), "the child closure is live"); + assert_eq!(js_box_get_bits(cell), int_bits(1)); + + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == child as usize); + assert!(!is_registered_box_ptr(cell)); + assert!(free_list_contains(cell as usize)); + let reused = js_box_alloc_bits(int_bits(9)); + assert_eq!(reused, cell, "last closure death publishes the cell"); + } + + /// A scope-released captured cell must follow the #8303 full-trace rule: + /// not a global root (else a payload that references its own closure is + /// immortal), but traced through a closure the mark set proved live. + #[test] + fn scope_released_captured_cell_is_an_ephemeron_edge_in_a_full_trace() { + test_clear_box_registry(); + let cell = js_box_alloc_bits(crate::value::TAG_UNDEFINED as i64); + let closure = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::js_closure_set_box_capture_ptr(closure, 0, cell as i64); + let closure_bits = crate::value::js_nanbox_pointer(closure as i64).to_bits(); + js_box_set_bits(cell, closure_bits as i64); + + let mut minor_roots = Vec::new(); + scan_box_roots(&mut |value| minor_roots.push(value.to_bits())); + assert!( + minor_roots.contains(&closure_bits), + "sabotage check: before release the registry roots the payload" + ); + js_box_scope_release(cell); + + crate::gc::begin_full_trace(); + let mut rooted = Vec::new(); + scan_box_roots(&mut |value| rooted.push(value.to_bits())); + assert!( + !rooted.contains(&closure_bits), + "a scope-released cell must not root its own closure in a full trace" + ); + let header = unsafe { + (closure as *mut u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader + }; + let saved_flags = unsafe { (*header).gc_flags }; + unsafe { (*header).gc_flags |= crate::gc::GC_FLAG_MARKED }; + let slots = crate::gc::test_gc_rewrite_slot_addresses(closure as usize) + .expect("closure rewrite descriptor"); + unsafe { (*header).gc_flags = saved_flags }; + crate::gc::finish_full_trace(); + assert!( + slots.contains(&(cell as usize)), + "a marked closure must trace the scope-released cell's payload" + ); + + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == closure as usize); + assert!(!is_registered_box_ptr(cell)); + } + + /// A minor walks ONLY the box young log. A full trace that stops rooting a + /// released cell must therefore still LOG it while its payload is young, + /// or the next minor has no root for a payload a live closure still reads. + /// `PERRY_GC_PROTECT_FROMSPACE` caught exactly this as a stale from-space + /// closure call; this test is its cheap, deterministic twin. + #[test] + fn a_full_trace_keeps_a_released_cell_in_the_minor_remembered_set() { + test_clear_box_registry(); + let cell = js_box_alloc_bits(crate::value::TAG_UNDEFINED as i64); + let closure = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::js_closure_set_box_capture_ptr(closure, 0, cell as i64); + let payload = crate::value::js_nanbox_pointer(closure as i64).to_bits(); + js_box_set_bits(cell, payload as i64); + assert!( + crate::gc::young_log::bits_are_minor_relevant(payload), + "premise: the payload is a young object a minor must rewrite" + ); + js_box_scope_release(cell); + + crate::gc::begin_full_trace(); + scan_box_roots(&mut |_| {}); + crate::gc::finish_full_trace(); + + BOX_YOUNG_ROOTS.with(|log| { + log.borrow() + .debug_assert_logged(BOX_YOUNG_LOG_NAME, &relevant_box_roots()) + }); + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == closure as usize); + } + + /// A synchronous callee running inside an async step must not park its + /// cells into the caller's activation range (that is what + /// `js_box_release` would do): the scope release is independent of the + /// ambient activation, and the activation's own terminal release still + /// works afterwards. + #[test] + fn scope_release_ignores_the_ambient_async_activation() { + test_clear_box_registry(); + let activation = new_async_box_activation(); + retain_async_box_activation(activation); + let previous = crate::promise::INLINE_TRAP.with(|trap| { + trap.replace(crate::promise::InlineTrap { + trap_next: std::ptr::null_mut(), + current_step: 0, + box_activation: activation, + }) + }); + let callee_cell = js_box_alloc_bits(int_bits(4)); + js_box_scope_release(callee_cell); + assert!(!is_registered_box_ptr(callee_cell)); + assert_eq!( + unsafe { (*activation).release_start.get() }, + NO_RELEASE_RANGE, + "the callee's cell must not enter the caller's release range" + ); + + let frame_cell = js_box_alloc_bits(int_bits(5)); + js_box_release(frame_cell); + assert!(is_registered_box_ptr(frame_cell), "a step still owns it"); + release_async_box_activation(activation); + assert!(!is_registered_box_ptr(frame_cell)); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); + } + + /// Generator frames also own compiler-private i32/bool control cells. + /// Each kind publishes through its own registry, and a cell of one kind + /// is never accepted by another kind's release entry. + #[test] + fn primitive_control_cells_release_through_their_own_registries() { + test_clear_box_registry(); + let state = js_i32_box_alloc(3); + let done = js_bool_box_alloc(1); + let ordinary = js_box_alloc_bits(int_bits(6)); + + js_i32_box_scope_release(ordinary.cast::()); + js_bool_box_scope_release(ordinary.cast::()); + js_box_scope_release(state.cast::()); + assert!(is_registered_box_ptr(ordinary)); + assert!(is_registered_i32_box_ptr(state)); + + let closure = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::js_closure_set_box_capture_ptr(closure, 0, done as i64); + js_i32_box_scope_release(state); + js_bool_box_scope_release(done); + assert!(!is_registered_i32_box_ptr(state)); + assert!(is_registered_bool_box_ptr(done), "a live closure reads it"); + assert_eq!(js_bool_box_get(done), 1); + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == closure as usize); + assert!(!is_registered_bool_box_ptr(done)); + assert_eq!(js_i32_box_alloc(8), state, "i32 free list reuses it"); + assert_eq!(js_bool_box_alloc(0), done, "bool free list reuses it"); + } +} diff --git a/crates/perry-runtime/src/closure/box_captures.rs b/crates/perry-runtime/src/closure/box_captures.rs index 2cc7fd7b4a..bdc7b3fda4 100644 --- a/crates/perry-runtime/src/closure/box_captures.rs +++ b/crates/perry-runtime/src/closure/box_captures.rs @@ -6,56 +6,162 @@ //! pointer-shaped bits. Once its async activation drains, the box runtime //! publishes an unobserved cell immediately and leaves only a captured cell //! pending. Closure moves rekey the per-closure index; authoritative GC death -//! pruning drops the corresponding per-cell counts. +//! pruning drops the corresponding per-cell counts. A cell released by the +//! ordinary frame that minted it (#10464) follows the same edges. use super::ClosureHeader; use std::cell::RefCell; -type BoxCaptureSlots = Vec<(u32, usize)>; +/// One closure's `(capture index, box address)` edges. Nearly every closure +/// declares one or two, and one is stored inline: a closure capturing a +/// reassigned local then costs no heap allocation for its lifetime record +/// (#10464 made those records a per-call cost of ordinary frames). +#[derive(Clone, Default)] +struct BoxCaptureSlots { + first: Option<(u32, usize)>, + rest: Vec<(u32, usize)>, +} + +impl BoxCaptureSlots { + fn take(&mut self, index: u32) -> Option { + if self.first.is_some_and(|(slot, _)| slot == index) { + let cell = self.first.take().map(|(_, cell)| cell); + self.first = self.rest.pop(); + return cell; + } + let pos = self.rest.iter().position(|(slot, _)| *slot == index)?; + Some(self.rest.swap_remove(pos).1) + } + + fn push(&mut self, index: u32, cell: usize) { + if self.first.is_none() { + self.first = Some((index, cell)); + } else { + self.rest.push((index, cell)); + } + } + + /// `rest` is non-empty only while `first` is occupied. + fn is_empty(&self) -> bool { + self.first.is_none() + } + + fn cells(&self) -> impl Iterator + '_ { + self.first + .iter() + .chain(self.rest.iter()) + .map(|(_, cell)| *cell) + } +} crate::perry_thread_local! { /// Closure address -> compiler-declared `(capture index, box address)` edges. static CLOSURE_BOX_CELLS: RefCell> = RefCell::new(crate::fast_hash::new_ptr_hash_map()); - /// Box address -> total number of capture slots naming it. + /// Box address -> packed edge record: the number of capture slots naming + /// it (`EDGE_COUNT_MASK`) plus, once the frame that minted the cell has + /// released it (#10464), `FRAME_RELEASED` and the cell-kind tag. Keeping + /// the release state in the record the capture edges already maintain + /// makes a frame exit one probe, and lets the final edge's removal publish + /// the cell without a second table. static BOX_CAPTURE_COUNTS: RefCell> = RefCell::new(crate::fast_hash::new_ptr_hash_map()); } +const FRAME_RELEASED: usize = 1 << 63; +const FRAME_RELEASE_TAG_SHIFT: u32 = 60; +const FRAME_RELEASE_TAG_MASK: usize = 0b11 << FRAME_RELEASE_TAG_SHIFT; +const EDGE_COUNT_MASK: usize = (1 << FRAME_RELEASE_TAG_SHIFT) - 1; + fn increment_cell_capture_count(cell: usize, amount: usize) { BOX_CAPTURE_COUNTS.with(|counts| { let mut counts = counts.borrow_mut(); - let count = counts.entry(cell).or_default(); - *count = count + let record = counts.entry(cell).or_default(); + let count = (*record & EDGE_COUNT_MASK) .checked_add(amount) + .filter(|count| *count <= EDGE_COUNT_MASK) .expect("box capture count overflow"); + *record = (*record & !EDGE_COUNT_MASK) | count; }); } fn decrement_cell_capture_count(cell: usize, amount: usize) { + // `Some(record)` when the final edge disappeared. let reached_zero = BOX_CAPTURE_COUNTS.with(|counts| { let mut counts = counts.borrow_mut(); - let Some(count) = counts.get_mut(&cell) else { - return false; - }; - debug_assert!(*count >= amount); - *count -= amount; - if *count == 0 { - counts.remove(&cell); - true + let record = counts.get_mut(&cell)?; + debug_assert!(*record & EDGE_COUNT_MASK >= amount); + *record -= amount; + if *record & EDGE_COUNT_MASK == 0 { + counts.remove(&cell) } else { - false + None } }); - if reached_zero { - crate::r#box::box_capture_count_reached_zero(cell); + match reached_zero { + Some(record) if record & FRAME_RELEASED != 0 => crate::r#box::publish_frame_released_cell( + cell, + (record & FRAME_RELEASE_TAG_MASK) >> FRAME_RELEASE_TAG_SHIFT, + ), + Some(_) => crate::r#box::box_capture_count_reached_zero(cell), + None => {} } } pub(crate) fn box_capture_count(cell: usize) -> usize { BOX_CAPTURE_COUNTS - .with(|counts| counts.borrow().get(&cell).copied()) - .unwrap_or(0) + .with(|counts| { + let counts = counts.borrow(); + if counts.is_empty() { + None + } else { + counts.get(&cell).copied() + } + }) + .map_or(0, |record| record & EDGE_COUNT_MASK) +} + +/// Outcome of [`note_frame_released_cell`]. +pub(crate) enum FrameRelease { + /// No closure edge names the cell: the frame was its only holder. + Uncaptured, + /// Captured; the final edge's removal now publishes it. + Deferred, + /// Captured and already released by its frame. + AlreadyReleased, +} + +/// #10464: the frame that minted `cell` (of kind `tag`, 1..=3) can no longer +/// name it. A captured cell is marked so its last capture edge publishes it. +pub(crate) fn note_frame_released_cell(cell: usize, tag: usize) -> FrameRelease { + debug_assert!((1..=3).contains(&tag)); + BOX_CAPTURE_COUNTS.with(|counts| { + let mut counts = counts.borrow_mut(); + if counts.is_empty() { + return FrameRelease::Uncaptured; + } + match counts.get_mut(&cell) { + None => FrameRelease::Uncaptured, + Some(record) if *record & FRAME_RELEASED != 0 => FrameRelease::AlreadyReleased, + Some(record) => { + *record |= FRAME_RELEASED | (tag << FRAME_RELEASE_TAG_SHIFT); + FrameRelease::Deferred + } + } + }) +} + +/// Whether `cell` is a frame-released, still-captured JSValue box: during a +/// full trace such a cell is reached only through its live closures. +pub(crate) fn frame_released_js_cell(cell: usize, js_tag: usize) -> bool { + BOX_CAPTURE_COUNTS.with(|counts| { + let counts = counts.borrow(); + !counts.is_empty() + && counts.get(&cell).is_some_and(|record| { + *record & FRAME_RELEASED != 0 + && (*record & FRAME_RELEASE_TAG_MASK) >> FRAME_RELEASE_TAG_SHIFT == js_tag + }) + }) } /// Visit the JSValue payload slots reached through one live closure's exact @@ -72,7 +178,7 @@ pub(crate) fn visit_closure_box_payload_slots_mut(closure: usize, mut visit: imp let Some(cells) = captures.get(&closure) else { return; }; - for &(_, cell) in cells { + for cell in cells.cells() { crate::r#box::visit_pending_captured_js_box_payload_slot(cell, &mut visit); } }); @@ -91,12 +197,9 @@ pub(super) fn set_closure_box_capture( let previous = CLOSURE_BOX_CELLS.with(|captures| { let mut captures = captures.borrow_mut(); let slots = captures.entry(closure).or_default(); - let previous = slots - .iter() - .position(|(slot, _)| *slot == index) - .map(|pos| slots.swap_remove(pos).1); + let previous = slots.take(index); if let Some(cell) = cell { - slots.push((index, cell)); + slots.push(index, cell); } if slots.is_empty() { captures.remove(&closure); @@ -133,7 +236,7 @@ pub(crate) fn clone_closure_box_captures( } copied }); - for (_, cell) in copied { + for cell in copied.cells() { increment_cell_capture_count(cell, 1); } } @@ -152,21 +255,22 @@ pub(crate) fn closure_box_captures_owner_moved(old_owner: usize, new_owner: usiz } pub(crate) fn prune_dead_closure_box_capture_owners(is_dead_closure: &dyn Fn(usize) -> bool) { - let dead_keys = CLOSURE_BOX_CELLS.with(|captures| { - captures - .borrow() - .keys() - .copied() - .filter(|owner| is_dead_closure(*owner)) - .collect::>() + // One pass: dropping a dead owner and collecting its edges together avoids + // a second probe per dead closure. Counts (and any publication they + // trigger) are settled after the table borrow ends. + let mut dead_cells = Vec::new(); + CLOSURE_BOX_CELLS.with(|captures| { + captures.borrow_mut().retain(|owner, slots| { + if is_dead_closure(*owner) { + dead_cells.extend(slots.cells()); + false + } else { + true + } + }); }); - for closure in dead_keys { - let cells = CLOSURE_BOX_CELLS - .with(|captures| captures.borrow_mut().remove(&closure)) - .unwrap_or_default(); - for (_, cell) in cells { - decrement_cell_capture_count(cell, 1); - } + for cell in dead_cells { + decrement_cell_capture_count(cell, 1); } } diff --git a/crates/perry-runtime/src/closure/mod.rs b/crates/perry-runtime/src/closure/mod.rs index a7e4210220..a7f4cb499f 100644 --- a/crates/perry-runtime/src/closure/mod.rs +++ b/crates/perry-runtime/src/closure/mod.rs @@ -73,7 +73,8 @@ pub use unbox::{js_closure_unbox_callee_checked, js_closure_unbox_callee_checked pub(crate) use box_captures::test_clear_closure_box_capture_indexes; pub(crate) use box_captures::{ box_capture_count, clone_closure_box_captures, closure_box_captures_owner_moved, - prune_dead_closure_box_capture_owners, visit_closure_box_payload_slots_mut, + frame_released_js_cell, note_frame_released_cell, prune_dead_closure_box_capture_owners, + visit_closure_box_payload_slots_mut, FrameRelease, }; #[cfg(feature = "wasm-host")] pub(crate) use dynamic_props::register_wasm_funcref_external; diff --git a/scripts/gc_root_dominance_check.py b/scripts/gc_root_dominance_check.py index 015a5f9590..c7d6e82707 100755 --- a/scripts/gc_root_dominance_check.py +++ b/scripts/gc_root_dominance_check.py @@ -522,6 +522,10 @@ def build_cfg(f): # (#7510: letting these two lists drift one-sided printed 358 spurious # violations once the corpus widened). "js_box_release", "js_i32_box_release", "js_bool_box_release", + # #10464 scope-exit release (box/scope_release.rs): the same publish, or a + # TLS pending-map insert for a closure-captured cell. + "js_box_scope_release", "js_i32_box_scope_release", + "js_bool_box_scope_release", "js_write_barrier", # gc/barrier.rs:930 "js_tdz_suppress_begin", "js_tdz_suppress_end", # box.rs:242/248 counter "js_array_note_numeric_write", # array/header.rs:1443 @@ -2742,6 +2746,11 @@ def _probe_boxes_outside_the_gc_heap(): still resume, which is a use-after-release aliasing hazard that this exemption would otherwise silently suppress. The old global quarantine is retained only as a conservative fallback for untracked callers. + + #10464 adds the scope-exit release of an ordinary frame's cells + (`box/scope_release.rs`). It may publish directly only because it is gated + on the cell's closure capture count; a captured cell must take the same + drained-pending path closure death pruning publishes from. """ try: with open("crates/perry-runtime/src/box.rs", @@ -2785,6 +2794,22 @@ def _probe_boxes_outside_the_gc_heap(): "park until its activation reaches zero references. " "Publishing overwrites the terminal value a stray " "resume still writes through.") + scope_path = "crates/perry-runtime/src/box/scope_release.rs" + scope = rust_fn_body(scope_path, "release_scope_cell") + if scope is None: + return (False, "release_scope_cell not found in box/scope_release.rs; " + "the #10464 scope-exit release changed shape") + if "note_frame_released_cell" not in scope \ + or "FrameRelease::Deferred" not in scope: + return (False, "scope-exit release no longer defers a closure-captured " + "cell to closure death") + try: + with open(scope_path, encoding="utf-8", errors="replace") as fh: + scope_src = fh.read() + except OSError: + return (False, f"{scope_path} not readable") + if re.search(r"\bdealloc\s*\(|arena_alloc\w*\s*\(", scope_src): + return (False, "box/scope_release.rs frees or arena-allocates cells") release_ref = rust_fn_body("crates/perry-runtime/src/box.rs", "release_async_box_activation") if release_ref is None or "if new == 0" not in release_ref \ diff --git a/test-files/test_gap_10464_box_cell_release.ts b/test-files/test_gap_10464_box_cell_release.ts new file mode 100644 index 0000000000..58b0f24268 --- /dev/null +++ b/test-files/test_gap_10464_box_cell_release.ts @@ -0,0 +1,305 @@ +// #10464: a `let`/`var` captured by a closure and reassigned lives in a box +// cell. Outside a lowered async state machine those cells were never released, +// so each call leaked a registered GC root plus everything the binding last +// referenced. This test checks both halves of the fix: memory stays bounded +// relative to an equal-allocation control, and cells that escape through +// closures (returned, stored, nested, generators, async) keep their values +// across many collections. + +const maybeGc = (globalThis as any).gc as (() => void) | undefined; + +function churn(rounds: number): number { + let total = 0; + for (let r = 0; r < rounds; r++) { + const junk: Array<{ i: number; s: string; a: number[] }> = []; + for (let i = 0; i < 2000; i++) junk.push({ i, s: "x" + i, a: [i, i + 1] }); + total += junk.length; + } + if (typeof maybeGc === "function") maybeGc(); + return total; +} + +// ---- memory: the issue's repro, compared in-process against its control ---- + +function payload(i: number): number { + let buf: number[] = new Array(512).fill(i); + const swap = () => { + buf = new Array(512).fill(i + 1); + }; + swap(); + return buf.length; +} + +function noBox(i: number): number { + const holder = { buf: new Array(512).fill(i) }; + const swap = () => { + holder.buf = new Array(512).fill(i + 1); + }; + swap(); + return holder.buf.length; +} + +function counterCell(i: number): number { + let x = i; + const bump = () => { + x += 1; + }; + bump(); + return x; +} + +const rssMb = () => process.memoryUsage().rss / 1048576; + +function growthMb(run: (i: number) => number, calls: number): number { + const before = rssMb(); + let acc = 0; + for (let i = 1; i <= calls; i++) acc += run(i); + if (acc <= 0) throw new Error("no work done"); + return rssMb() - before; +} + +// Warm both shapes so allocator and heap high-water marks settle first; the +// control runs before the boxed shape so RSS reuse can only favor the control. +growthMb(noBox, 5000); +growthMb(payload, 5000); +const CALLS = 60000; +const controlGrowth = growthMb(noBox, CALLS); +const payloadGrowth = growthMb(payload, CALLS); +// Leaking every call's 512-element array costs ~5 KB * 60k = ~300 MB. +console.log("payload growth bounded by control:", payloadGrowth < Math.max(controlGrowth, 0) + 96); + +let cellAcc = 0; +for (let i = 1; i <= 200000; i++) cellAcc += counterCell(i); +console.log("counter cells:", cellAcc); + +// ---- escaping closures keep their cells ---- + +function makeCounter(start: number) { + let n = start; + return { + inc: () => ++n, + add: (k: number) => { + n += k; + return n; + }, + get: () => n, + }; +} + +const counters: Array> = []; +for (let i = 0; i < 3000; i++) counters.push(makeCounter(i)); +churn(20); +let counterSum = 0; +for (let i = 0; i < counters.length; i++) { + counters[i].inc(); + counterSum += counters[i].add(i); +} +churn(20); +for (let i = 0; i < counters.length; i += 7) counterSum += counters[i].get(); +console.log("returned counters:", counterSum); + +function makeAdders(): Map number> { + const map = new Map number>(); + for (let i = 0; i < 500; i++) { + let base = i; + map.set("k" + i, (v: number) => (base += v)); + } + return map; +} +const adders = makeAdders(); +churn(15); +let adderSum = 0; +for (const [, fn] of adders) adderSum += fn(1); +churn(15); +for (const [, fn] of adders) adderSum += fn(2); +console.log("per-iteration map closures:", adderSum); + +// A loop whose iterations sometimes capture and sometimes do not: released +// uncaptured cells are reused by the next iteration, captured ones must not be. +function mixedLoop(): number[] { + const kept: Array<() => number> = []; + for (let i = 0; i < 400; i++) { + let value = i * 3; + const touch = () => (value += 1); + if (i % 3 === 0) kept.push(touch); + else touch(); + value += 0; + } + churn(10); + return kept.map((f) => f()).slice(0, 8); +} +console.log("mixed loop:", mixedLoop().join(",")); + +// Nested closure created after the outer frame returned. +function outerFactory() { + let state = "a"; + const middle = () => { + state += "b"; + return () => { + state += "c"; + return state; + }; + }; + return middle; +} +const middles: Array<() => () => string> = []; +for (let i = 0; i < 200; i++) middles.push(outerFactory()); +churn(10); +const inners = middles.map((m) => m()); +churn(10); +console.log("nested after return:", inners[0](), inners[199](), middles[5]()()); + +// Self-referencing payload: the cell's value references its own closure. +function selfCycle(i: number) { + let self: any = null; + const getSelf = () => self; + self = { i, getSelf }; + return getSelf; +} +const cycles: Array<() => any> = []; +for (let i = 0; i < 20000; i++) { + const g = selfCycle(i); + if (i % 1000 === 0) cycles.push(g); +} +churn(15); +console.log("self cycles:", cycles.map((g) => g().getSelf().i).join(",")); + +// Generators capture boxed state across suspensions; abandoned ones are freed. +function* running(limit: number) { + let total = 0; + const add = (v: number) => { + total += v; + }; + for (let i = 1; i <= limit; i++) { + add(i); + yield total; + } + return total; +} +const gens: Array> = []; +for (let i = 0; i < 50; i++) gens.push(running(5)); +let genSum = 0; +for (let step = 0; step < 6; step++) { + churn(3); + for (const g of gens) { + const r = g.next(); + genSum += r.value ?? 0; + } +} +for (let i = 0; i < 5000; i++) running(3).next(); +console.log("generators:", genSum); + +// Class method frames and closures stored on the instance. +class Account { + report: () => string = () => ""; + owner: string; + constructor(owner: string) { + this.owner = owner; + } + open(deposit: number) { + let balance = deposit; + let history = [deposit]; + this.report = () => `${this.owner}:${balance}:${history.length}`; + return (amount: number) => { + balance += amount; + history = history.concat([amount]); + return balance; + }; + } +} +const accounts: Account[] = []; +const deposits: Array<(n: number) => number> = []; +for (let i = 0; i < 300; i++) { + const acct = new Account("u" + i); + accounts.push(acct); + deposits.push(acct.open(i)); +} +churn(10); +for (let i = 0; i < deposits.length; i++) deposits[i](10); +churn(10); +console.log("class frames:", accounts[0].report(), accounts[299].report()); + +// Early returns, throws and finally on frames that own cells. +function exits(mode: number): number { + let seen = mode; + const mark = () => (seen += 100); + try { + if (mode === 0) return mark(); + if (mode === 1) throw new Error("boom" + seen); + mark(); + } finally { + seen += 1; + } + return seen; +} +let exitSum = 0; +for (let i = 0; i < 3000; i++) { + try { + exitSum += exits(i % 3); + } catch (e) { + exitSum += (e as Error).message.length; + } +} +console.log("exits:", exitSum); + +// Recursion: every frame owns its own cell. +function depth(n: number): number { + let local = n; + const bump = () => (local += 1); + if (n > 0) local += depth(n - 1); + bump(); + return local; +} +console.log("recursion:", depth(200)); + +// Parameters captured and reassigned are boxed too. +function paramCell(a: number, b: string) { + const again = () => { + a += 1; + b = b + a; + }; + again(); + return () => b + ":" + a; +} +const paramFns: Array<() => string> = []; +for (let i = 0; i < 1000; i++) paramFns.push(paramCell(i, "p")); +churn(10); +console.log("params:", paramFns[0](), paramFns[999]()); + +// async without await, and an await-ing async closure that captures a cell +// owned by an enclosing synchronous frame which returns before it resumes. +async function noAwait(i: number) { + let y = i; + const f = () => { + y++; + }; + f(); + return y; +} + +function syncOwner(seed: number) { + let shared = seed; + const read = () => shared; + void (async () => { + await null; + churn(2); + shared += 1000; + })(); + shared += 1; + return read; +} + +async function main() { + let asyncSum = 0; + for (let i = 0; i < 2000; i++) asyncSum += await noAwait(i); + console.log("async no await:", asyncSum); + + const readers: Array<() => number> = []; + for (let i = 0; i < 20; i++) readers.push(syncOwner(i)); + churn(10); + await new Promise((resolve) => setTimeout(resolve, 10)); + churn(10); + console.log("async capture of outer cell:", readers.map((r) => r()).join(",")); +} + +main().then(() => console.log("done")); From 3f880a979c1f09c13a414ef3842d4305c994f651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 10:52:25 +0000 Subject: [PATCH 2/2] docs(changelog): add fragment for #10613 --- changelog.d/10613-box-cell-release.md | 52 +++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 changelog.d/10613-box-cell-release.md diff --git a/changelog.d/10613-box-cell-release.md b/changelog.d/10613-box-cell-release.md new file mode 100644 index 0000000000..55e6c61ee5 --- /dev/null +++ b/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).