diff --git a/changelog.d/8213-next-async-box-closure-lifetime.md b/changelog.d/8213-next-async-box-closure-lifetime.md new file mode 100644 index 0000000000..6ac95d434b --- /dev/null +++ b/changelog.d/8213-next-async-box-closure-lifetime.md @@ -0,0 +1,12 @@ +## Fix completed async frames retaining closure-visible box cells + +Plain async functions now hand their complete boxed activation frame to the +release/reuse path. Closure capture counts follow GC moves, and authoritative +death pruning drops them. Full collections trace drained box payloads from +their live closures instead of rooting every pending box, so self-referential +box/closure cycles can die. After queued/running steps drain, uncaptured cells +publish immediately while each captured cell remains readable until its own +final closure dies, so one escaped closure cannot retain the rest of the frame. + +This removes the closure-visible residue left by #8208 without weakening raw +box-pointer rejection or returning box-cell memory to the allocator. diff --git a/crates/perry-codegen/src/expr/closure.rs b/crates/perry-codegen/src/expr/closure.rs index 329e3df40a..221a33bd48 100644 --- a/crates/perry-codegen/src/expr/closure.rs +++ b/crates/perry-codegen/src/expr/closure.rs @@ -186,27 +186,10 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // inner arrow `(eid, arch, compId) => ... changeset ...` // is created per-call but always with the same `this` (the // World) and same captures (`this._changeset`). - // Boxed captures still allow the cache path: the closure - // stores the BOX POINTER (a stable per-allocation address), - // and the box's contents are read dynamically inside the - // body via `js_box_get`. Two closure-literal sites that - // capture the same boxed local store identical box-pointer - // bits, so the cache (keyed on bit-equality of capture - // slots) still hits. The cache backing is a small LRU per - // func_ptr, which tolerates the parallel-instance pattern - // (50 concurrent unitOfWork calls each capturing a - // different `__async_step` box) by holding multiple - // captures rather than overwriting one slot per call. - // - // We previously bailed out when any captured local was - // boxed (`mutable_captures` non-empty). That made the - // async-to-generator transform's per-`await` `cb_v` / - // `cb_e` closures (which capture the boxed `__async_step` - // self-reference) miss the cache 100% of the time — - // 2 fresh closure allocs per await ≈ 300 ns of `gc_malloc` - // work even though the box pointers are stable across - // call sites. The relaxed gate plus the multi-slot LRU - // backing reclaims that overhead. + // Boxed captures may use the bulk cache helper, but codegen still + // follows it with `js_closure_set_box_capture_ptr` for only those + // slots. The idempotent write declares exact lifetime edges + // without guessing from arbitrary pointer-shaped values. // // IDENTITY CAVEAT (#4831 follow-up — Stripe `protoExtend`): // the singleton-sharing paths (`js_closure_alloc_singleton` / @@ -231,25 +214,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // symptom. // // To preserve the hot-path optimizations while restoring identity, - // a closure is singleton-eligible only when sharing one instance is - // observationally safe: - // - arrow functions: no own `.prototype`, not constructable, the - // `.map`/ECS callbacks the cache targets; OR - // - non-arrow closures all of whose captures are BOXED (mutable) - // locals: the compiler-synthesized async-step `cb_v`/`cb_e` - // per-await callbacks capture the boxed `__async_step` self-ref - // and are never used as constructors — keeping them cached - // avoids 2 `gc_malloc`s per `await`. - // A non-arrow closure capturing an UNBOXED value (Stripe's `Super`, - // or no captures at all) is treated as a potential constructor and - // always gets a fresh instance. + // arrow functions are singleton-eligible because they have no own + // `.prototype` and are not constructable. Compiler-synthesized + // non-arrow async callbacks whose captures are all boxes are also + // safe: they are never constructors and their cache key includes + // the box addresses. Other non-arrow closures are treated as + // potential constructors and always get a fresh instance. let mut write_ids = std::collections::HashSet::new(); crate::boxed_vars::collect_write_ids_in_stmts(body, &mut write_ids); let writes_unboxed_capture = auto_captures .iter() .any(|cap_id| !ctx.boxed_vars.contains(cap_id) && write_ids.contains(cap_id)); - // All captures boxed (and at least one), with no reserved `this` / - // `new.target` slot: the compiler-synthesized async-callback shape. let captures_all_boxed = !*captures_this && !*captures_new_target && !auto_captures.is_empty() @@ -346,15 +321,25 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { .call_void("js_register_closure_async_function", &[(PTR, &func_ref)]); } - // The captured-singleton helper writes captures internally - // (so the cached layout matches a fresh allocation). The - // other paths still need explicit per-slot writes. - if !captured_singleton { - let blk = ctx.block(); - for (idx, val_bits) in captured_value_bits.iter().enumerate() { + // The captured-singleton helper writes captures internally. Boxed + // slots still take the dedicated, idempotent setter afterward so + // their lifetime edges are declared; fresh closures need every + // slot initialized here. + let boxed_capture_slots = auto_captures + .iter() + .map(|cap_id| ctx.boxed_vars.contains(cap_id)) + .collect::>(); + let blk = ctx.block(); + for (idx, val_bits) in captured_value_bits.iter().enumerate() { + if !captured_singleton || boxed_capture_slots[idx] { let idx_str = idx.to_string(); + let setter = if boxed_capture_slots[idx] { + "js_closure_set_box_capture_ptr" + } else { + "js_closure_set_capture_bits" + }; blk.call_void( - "js_closure_set_capture_bits", + setter, &[(I64, &closure_handle), (I32, &idx_str), (I64, val_bits)], ); } diff --git a/crates/perry-codegen/src/expr/literals_vars.rs b/crates/perry-codegen/src/expr/literals_vars.rs index 64afa1e3df..030ca27a6c 100644 --- a/crates/perry-codegen/src/expr/literals_vars.rs +++ b/crates/perry-codegen/src/expr/literals_vars.rs @@ -724,15 +724,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // // #8208 added a release/reuse path for completed async // activations, so "never freed" is no longer literally true and - // the argument is now stated on the two properties that ARE: + // the argument is now stated on the properties that ARE: // (1) cell memory is never returned to the allocator, so the // address never stops naming 8 bytes of box cell; (2) the - // release transform excludes closure-visible locals while a - // capture can remain reachable; and (3) a released cell stays - // PARKED until its owning activation has no queued or running - // async step. A capture from an enclosing activation therefore - // cannot become reusable inside the nested user frame - // `coerce_old`/`step_new` may enter. + // runtime counts each raw box capture; and (3) a + // terminal cell stays live until both queued/running steps and + // capturing closures are gone. A capture from an enclosing + // activation therefore cannot become reusable inside the + // nested user frame `coerce_old`/`step_new` may enter. if ctx.boxed_vars.contains(id) { let blk = ctx.block(); let box_ptr = blk.call( diff --git a/crates/perry-codegen/src/gc_call_effects.rs b/crates/perry-codegen/src/gc_call_effects.rs index e3f8224b31..bf0231f968 100644 --- a/crates/perry-codegen/src/gc_call_effects.rs +++ b/crates/perry-codegen/src/gc_call_effects.rs @@ -131,6 +131,7 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect { // module factory these were 1,495 of 5,537 statepoints. | "js_closure_get_capture_bits" | "js_closure_set_capture_bits" + | "js_closure_set_box_capture_ptr" | "js_closure_get_capture_ptr" | "js_closure_set_capture_ptr" // Variable-box accessors and allocators (#8132), `box.rs`. Boxes are @@ -435,6 +436,7 @@ mod tests { for name in [ "js_closure_get_capture_bits", "js_closure_set_capture_bits", + "js_closure_set_box_capture_ptr", "js_closure_get_capture_ptr", "js_closure_set_capture_ptr", "js_box_alloc_bits", diff --git a/crates/perry-codegen/src/root_reload.rs b/crates/perry-codegen/src/root_reload.rs index 9a329979e7..d33a945328 100644 --- a/crates/perry-codegen/src/root_reload.rs +++ b/crates/perry-codegen/src/root_reload.rs @@ -218,6 +218,7 @@ const NON_COLLECTING: &[&str] = &[ "js_typed_feedback_closure_direct_call_guard", // verified non-allocating bookkeeping stores/reads "js_closure_set_capture_bits", + "js_closure_set_box_capture_ptr", "js_closure_get_capture_bits", "js_closure_set_capture_ptr", "js_closure_get_capture_ptr", @@ -274,14 +275,19 @@ const TRANSPARENT_CAST: &[&str] = &["bitcast", "ptrtoint", "inttoptr", "trunc", /// Re-CALLING it below a collection point is sound under the same argument #7664's string-handle /// reload uses: the closure struct is a first-class root (rooted at `current_closure_slot`, /// relocated/rewritten on evacuation), so re-reading its capture array returns the -/// post-relocation value — UNLESS a `js_closure_set_capture_bits` to the same index ran in the -/// window, which is the store side-condition below. +/// post-relocation value — UNLESS a generic or declared-box capture setter to +/// the same index ran in the window, which is the store side-condition below. const CAPTURE_GET_CALLEE: &str = "js_closure_get_capture_bits"; /// The write half. Not itself collecting (`NON_COLLECTING` above), but its side effect on the /// closure's capture slot must invalidate a [`CAPTURE_GET_CALLEE`] reload the same way a `store` /// invalidates a shadow-slot one — `stores_to` carries a synthetic per-index key for exactly /// that (#7725). -const CAPTURE_SET_CALLEE: &str = "js_closure_set_capture_bits"; +fn is_capture_set_callee(callee: &str) -> bool { + matches!( + callee, + "js_closure_set_capture_bits" | "js_closure_set_box_capture_ptr" + ) +} /// A synthetic "location" key for capture-slot index `idx` — never a real pointer token (no `%` /// / `@` sigil), so it cannot collide with an actual shadow-slot register or handle-global name. @@ -935,7 +941,7 @@ fn facts_of(inst: &LlInst, slots: &HashSet) -> Facts { transparent = true; capture_get_key = Some(capture_slot_key(idx)); } - } else if callee == CAPTURE_SET_CALLEE { + } else if is_capture_set_callee(callee) { if let Some(idx) = literal_capture_idx(args) { stores_to = Some(capture_slot_key(idx)); } @@ -1109,7 +1115,7 @@ fn raw_facts(text: &str, slots: &HashSet) -> Facts { transparent = true; capture_get_key = Some(capture_slot_key(&idx)); } - } else if name == CAPTURE_SET_CALLEE { + } else if is_capture_set_callee(&name) { if let Some(idx) = raw_call_literal_arg(rhs, &name, 1) { stores_to = Some(capture_slot_key(&idx)); } diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 3a0044c54a..f8819347a8 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -197,6 +197,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { &[PTR, I32, PTR], ); module.declare_function("js_closure_set_capture_bits", VOID, &[I64, I32, I64]); + module.declare_function("js_closure_set_box_capture_ptr", VOID, &[I64, I32, I64]); module.declare_function("js_closure_get_capture_bits", I64, &[I64, I32]); module.declare_function("js_closure_set_capture_f64", VOID, &[I64, I32, DOUBLE]); module.declare_function("js_closure_get_capture_f64", DOUBLE, &[I64, I32]); diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index 5d5fae9c98..bb594065f3 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -595,10 +595,9 @@ pub(crate) fn lower_stmt(ctx: &mut FnCtx<'_>, stmt: &Stmt) -> Result<()> { Stmt::PreallocateTdzBoxes(ids) => emit_preallocate_boxes(ctx, ids, true), // #7933 follow-up (async-state RSS accumulation): a plain-async - // activation's terminal states release the box cells the escape - // analysis proved unobservable — clear + de-register + park for - // reuse, so completed activations stop accumulating malloc-side - // memory (cell + registry entry) for the life of the process. + // activation's terminal states hand its complete frame to runtime + // lifetime tracking. Uncaptured cells publish after queued/running + // steps drain; captured cells wait for their last GC closure. Stmt::ReleaseBoxes(ids) => emit_release_boxes(ctx, ids), // #853: every current `perry_hir::Stmt` variant is matched above. diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index e6807f3e4e..d702f979a9 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -7081,9 +7081,8 @@ fn boxed_local_slot_uses_i64_js_value_bits_until_helper_edges() { ); assert!( ir.contains("call i64 @js_closure_get_capture_bits") - && (ir.contains("call void @js_closure_set_capture_bits") - || ir.contains("call i64 @js_closure_alloc_with_captures_singleton")), - "generated boxed capture traffic should use exact i64 closure capture slots:\n{ir}" + && ir.contains("call void @js_closure_set_box_capture_ptr"), + "generated boxed capture traffic should declare exact i64 box capture slots:\n{ir}" ); for old_helper in [ "call void @js_closure_set_capture_f64", diff --git a/crates/perry-hir/src/ir/stmt.rs b/crates/perry-hir/src/ir/stmt.rs index 0ce94a79e6..2ce2a54c33 100644 --- a/crates/perry-hir/src/ir/stmt.rs +++ b/crates/perry-hir/src/ir/stmt.rs @@ -73,11 +73,11 @@ pub enum Stmt { /// ReferenceError; the `Stmt::Let` (or `let x;` with no init) overwrites /// the sentinel with the real value / `undefined`, ending the dead zone. PreallocateTdzBoxes(Vec), - /// Release the heap box cells behind a set of boxed LocalIds: clear each - /// cell to `undefined`, de-register it, and park it for reuse by a later - /// `js_box_alloc*` (#7933 / async-state RSS accumulation). Emitted by the - /// async-to-generator transform at a plain-async activation's terminal - /// states, ONLY for ids proven unobservable (`generator/box_release.rs`). + /// Hand the heap box cells behind a set of boxed LocalIds to the async + /// activation lifetime tracker (#7933 / #8213). A cell no closure captures + /// is cleared, de-registered, and parked when the activation's queued and + /// running steps drain. A closure-captured cell remains live until the GC + /// proves the final capturing closure dead. /// /// Semantics are a *reclamation hint*: dropping this statement is always /// correct (the cells just stay live, as before #7933). Carrying it diff --git a/crates/perry-runtime/src/box.rs b/crates/perry-runtime/src/box.rs index 4d500b9aa2..e9e688ebe7 100644 --- a/crates/perry-runtime/src/box.rs +++ b/crates/perry-runtime/src/box.rs @@ -172,30 +172,22 @@ crate::perry_thread_local! { /// #7933 follow-up: reusable cells for each box kind, plus the fallback /// quarantine used by release calls made outside a tracked activation. /// - /// `js_*box_release` (emitted at a plain-async activation's terminal - /// states, ONLY for cells the transform's escape analysis proved no - /// closure can observe — `perry-transform/src/generator/box_release.rs`) - /// clears the cell, removes it from its registry, and parks the address in - /// that activation's tagged release range. The async pump retains the - /// activation token for each queued/running `Task::AsyncStep`; the final - /// decrement publishes the range to these free lists. `js_*box_alloc*` - /// pops the matching list before touching `std::alloc`. + /// `js_*box_release` names every cell in a completed plain-async frame. + /// The async pump retains the activation token for queued/running steps. + /// When those drain, each uncaptured cell clears and publishes while a + /// closure-captured cell remains pending until its own capture count is + /// zero. `js_*box_alloc*` pops the matching free list before touching + /// `std::alloc`. /// /// ## Why the per-activation boundary is sound /// - /// A released cell's address can still be REACHED (not legitimately - /// read) by one thing: a duplicate resume of the already-terminal - /// activation, which can only exist as a `Task::AsyncStep` already - /// sitting in this thread's TASK_QUEUE (every suspend registers the step - /// on a native, settle-once Promise, so each registration fires at most - /// once; user thenables are assimilated first and cannot double-fire the - /// step). While parked, the address is INERT: it is out of the registry, - /// so `js_box_set` drops the write and `js_box_get` returns `undefined`, - /// which routes a stray resume into the dispatch loop's default - /// done-arm — byte-for-byte the behavior of the pre-existing cleared-cell - /// path. When this activation's reference count reaches zero, no queued or - /// running resume can still carry its step closure; reusing its cells is - /// unobservable even while unrelated tasks remain in the queue. + /// Before the activation reference count reaches zero, every terminal cell + /// remains registered and unchanged because a queued/running resume can + /// still observe the frame. At zero, the frame splits into independent + /// cells: GC closure capture indexes follow moves and keep only the exact + /// captured cells live until authoritative death pruning drops their final + /// counts. Clearing and reuse therefore happen at each exact reachability + /// boundary instead of one captured cell retaining the whole frame. /// /// Memory safety is unconditional either way: cells only ever move /// between the registry, the quarantine and the pool — they are never @@ -206,7 +198,7 @@ crate::perry_thread_local! { /// live registered cell or an inert parked one) and #7906's positive /// pointer cache ("was a box" can never become "is another object"). /// - /// NOT a GC root: parked cells are cleared before parking, and the + /// NOT a GC root: published cells are cleared before parking, and the /// addresses themselves are `std::alloc` memory, not GC-heap pointers. /// The root-holder census intentionally does not classify bare core-crate /// integer tables of this shape; its documented rule-B limit applies. @@ -221,11 +213,11 @@ crate::perry_thread_local! { /// side table was ~1 MB and made small async workloads a net RSS /// REGRESSION; threading the list through the cells removes it entirely. /// - /// Overwriting the cell is why this list holds only cells that are PAST - /// their activation's reachability boundary. A parked cell must keep the - /// terminal value a stray duplicate resume reads (`-1` / `true` / - /// `undefined`); only the final activation decrement makes its bytes free - /// to become an intrusive link. + /// Overwriting the cell is why this list holds only cells that are past + /// their activation's reachability boundary. Pending cells retain their + /// real value while a closure can observe it; both the activation step + /// boundary and that cell's capture count must be clear before its bytes + /// become an intrusive link. static BOX_FREE_HEAD: std::cell::Cell = const { std::cell::Cell::new(0) }; static I32_BOX_FREE_HEAD: std::cell::Cell = const { std::cell::Cell::new(0) }; static BOOL_BOX_FREE_HEAD: std::cell::Cell = const { std::cell::Cell::new(0) }; @@ -251,15 +243,20 @@ crate::perry_thread_local! { /// activations plus one releasing frame rather than process history. static ASYNC_RELEASED_CELLS: std::cell::RefCell> = const { std::cell::RefCell::new(Vec::new()) }; + /// Cells already named by a terminal `ReleaseBoxes`. They remain live and + /// registered while an escaped closure can still read them. The value is + /// the cell-kind tag plus `ASYNC_RELEASE_DRAINED` once queued/running step + /// owners are gone; at that point a zero capture count publishes the cell. + static ASYNC_PENDING_RELEASES: std::cell::RefCell> = + std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_map()); } /// Malloc-side reachability token for one lowered plain-async activation. /// -/// `refs` counts the lifecycle owner plus every queued/running async-step -/// owner. Released cells stay parked at their terminal values in the token's -/// tagged range until the last owner goes away; that zero transition publishes -/// the whole frame to the intrusive free pools. This is the exact reachability -/// boundary that a global "task queue empty" flush could only approximate. +/// `refs` counts the lifecycle owner plus queued/running async-step owners. +/// At zero, unobserved terminal cells publish immediately; a closure-captured +/// cell detaches from the activation and publishes independently when its own +/// capture count reaches zero. pub(crate) struct AsyncBoxActivation { id: u64, refs: std::cell::Cell, @@ -274,6 +271,7 @@ const ASYNC_RELEASE_JS: usize = 1; const ASYNC_RELEASE_I32: usize = 2; const ASYNC_RELEASE_BOOL: usize = 3; const ASYNC_RELEASE_TAG_MASK: usize = 0b11; +const ASYNC_RELEASE_DRAINED: usize = 0b100; /// Create the stable token for a plain-async activation. The activation /// lifecycle owns the initial reference until a terminal release (or @@ -359,6 +357,18 @@ pub(crate) fn retain_async_box_activation(ptr: *mut AsyncBoxActivation) { } } +/// Resolve a raw closure-capture word to a currently registered box address. +pub(crate) fn registered_box_capture_addr(addr: usize) -> Option { + if !is_plausible_box_ptr(addr as *mut Box) { + return None; + } + let ptr = addr as *mut Box; + let is_live_box = is_registered_box_ptr(ptr) + || is_registered_i32_box_ptr(ptr.cast::()) + || is_registered_bool_box_ptr(ptr.cast::()); + is_live_box.then_some(addr) +} + #[inline] pub(crate) fn release_async_box_activation(ptr: *mut AsyncBoxActivation) { if ptr.is_null() { @@ -412,6 +422,81 @@ fn push_free_cell(addr: usize, head: &'static crate::tls_hot::HotKey { + BOX_REGISTRY.with(|r| { + r.borrow_mut().remove(&addr); + }); + 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); + }); + 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); + }); + box_ptr_cache_evict(&BOOL_BOX_PTR_CACHE, addr); + unsafe { (*(addr as *mut BoolBox)).value = true }; + push_free_cell(addr, &BOOL_BOX_FREE_HEAD); + } + _ => unreachable!("invalid async released-cell tag"), + } + ASYNC_PENDING_RELEASES.with(|pending| { + pending.borrow_mut().remove(&addr); + }); + BOX_FLUSH_PUBLISHED.fetch_add(1, Ordering::Relaxed); +} + +pub(crate) fn box_capture_count_reached_zero(addr: usize) { + let pending = ASYNC_PENDING_RELEASES + .with(|releases| releases.borrow().get(&addr).copied()) + .unwrap_or(0); + if pending & ASYNC_RELEASE_DRAINED != 0 { + publish_box_cell(addr, pending & ASYNC_RELEASE_TAG_MASK); + } +} + +/// 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`]. +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| { + 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)) { + let ptr = addr as *mut Box; + unsafe { visit(&raw mut (*ptr).value) }; + } +} + +fn begin_pending_release(addr: usize, tag: usize) -> bool { + ASYNC_PENDING_RELEASES.with(|pending| { + let mut pending = pending.borrow_mut(); + if pending.contains_key(&addr) { + false + } else { + pending.insert(addr, tag); + true + } + }) +} + fn publish_async_activation_cells(activation: *mut AsyncBoxActivation) { let (start, end) = unsafe { ( @@ -422,7 +507,6 @@ fn publish_async_activation_cells(activation: *mut AsyncBoxActivation) { if start == NO_RELEASE_RANGE { return; } - let mut published = 0u64; ASYNC_RELEASED_CELLS.with(|cells| { let mut cells = cells.borrow_mut(); debug_assert!(start <= end && end <= cells.len()); @@ -432,20 +516,23 @@ fn publish_async_activation_cells(activation: *mut AsyncBoxActivation) { continue; } let addr = value & !ASYNC_RELEASE_TAG_MASK; - match value & ASYNC_RELEASE_TAG_MASK { - ASYNC_RELEASE_JS => push_free_cell(addr, &BOX_FREE_HEAD), - ASYNC_RELEASE_I32 => push_free_cell(addr, &I32_BOX_FREE_HEAD), - ASYNC_RELEASE_BOOL => push_free_cell(addr, &BOOL_BOX_FREE_HEAD), - _ => unreachable!("invalid async released-cell tag"), + let tag = value & ASYNC_RELEASE_TAG_MASK; + if crate::closure::box_capture_count(addr) == 0 { + publish_box_cell(addr, tag); + } else { + ASYNC_PENDING_RELEASES.with(|pending| { + let previous = pending + .borrow_mut() + .insert(addr, tag | ASYNC_RELEASE_DRAINED); + debug_assert_eq!(previous, Some(tag)); + }); } *tagged = 0; - published += 1; } while cells.last() == Some(&0) { cells.pop(); } }); - BOX_FLUSH_PUBLISHED.fetch_add(published, Ordering::Relaxed); } /// Drop the activation lifecycle's owner at terminal state. Queued or running @@ -551,7 +638,7 @@ fn box_ptr_cache_record(cache: &'static BoxPtrCache, addr: usize) { } /// Evict `addr` from its direct-mapped cache slot if it currently occupies -/// it. Called on release so a parked cell is invisible to the positive cache +/// it. Called on publication so a parked cell is invisible to the positive cache /// too — the parked-cell inertness argument in the QUARANTINE doc relies on /// every `js_box_get`/`js_box_set` on a parked address falling through to /// the registry probe and missing. @@ -687,13 +774,10 @@ pub extern "C" fn js_bool_box_alloc(initial_value: i32) -> *mut BoolBox { /// #7933 follow-up: release one JSValue box cell at a plain-async /// activation's terminal state. /// -/// Emitted by codegen for `Stmt::ReleaseBoxes` — ONLY for cells the -/// transform's escape analysis proved no closure can observe -/// (`perry-transform/src/generator/box_release.rs`), which is the same -/// precondition the pre-existing clear-to-`undefined` release relied on. -/// Clears the cell, removes it from the registry, evicts the positive-cache -/// slot, and parks the address for reuse after this activation's final queued -/// or running async-step reference is released (see the pool doc above). +/// Emitted by codegen for every cell in a completed plain-async frame. It +/// records the cell in the activation's pending terminal release range. +/// Clearing, de-registration and reuse wait for both async-step references and +/// GC closures capturing the cell to disappear (see the pool doc above). /// /// Idempotent and foreign-pointer-safe by the same gate: a pointer that is /// not currently registered — already released, never a box, or a @@ -707,6 +791,19 @@ pub extern "C" fn js_box_release(ptr: *mut Box) { if !is_plausible_box_ptr(ptr) { return; } + let activation = crate::promise::current_async_box_activation(); + if !activation.is_null() { + if !BOX_REGISTRY.with(|r| r.borrow().contains(&addr)) { + return; + } + if !begin_pending_release(addr, ASYNC_RELEASE_JS) { + return; + } + park_async_activation_cell(activation, addr, ASYNC_RELEASE_JS); + finish_async_box_activation(activation); + BOX_RELEASE_COUNT.fetch_add(1, Ordering::Relaxed); + return; + } let was_registered = BOX_REGISTRY.with(|r| r.borrow_mut().remove(&addr)); if !was_registered { return; @@ -718,13 +815,7 @@ pub extern "C" fn js_box_release(ptr: *mut Box) { // root scanner only walks the registry, which no longer has it). (*ptr).value = crate::value::TAG_UNDEFINED; } - let activation = crate::promise::current_async_box_activation(); - if activation.is_null() { - BOX_RELEASE_QUARANTINE.with(|q| q.borrow_mut().push(addr)); - } else { - park_async_activation_cell(activation, addr, ASYNC_RELEASE_JS); - finish_async_box_activation(activation); - } + BOX_RELEASE_QUARANTINE.with(|q| q.borrow_mut().push(addr)); BOX_RELEASE_COUNT.fetch_add(1, Ordering::Relaxed); } @@ -744,6 +835,19 @@ pub extern "C" fn js_i32_box_release(ptr: *mut I32Box) { if !is_plausible_box_ptr(ptr.cast::()) { return; } + let activation = crate::promise::current_async_box_activation(); + if !activation.is_null() { + if !I32_BOX_REGISTRY.with(|r| r.borrow().contains(&addr)) { + return; + } + if !begin_pending_release(addr, ASYNC_RELEASE_I32) { + return; + } + park_async_activation_cell(activation, addr, ASYNC_RELEASE_I32); + finish_async_box_activation(activation); + BOX_RELEASE_COUNT.fetch_add(1, Ordering::Relaxed); + return; + } let was_registered = I32_BOX_REGISTRY.with(|r| r.borrow_mut().remove(&addr)); if !was_registered { return; @@ -752,13 +856,7 @@ pub extern "C" fn js_i32_box_release(ptr: *mut I32Box) { unsafe { (*ptr).value = -1; } - let activation = crate::promise::current_async_box_activation(); - if activation.is_null() { - I32_BOX_RELEASE_QUARANTINE.with(|q| q.borrow_mut().push(addr)); - } else { - park_async_activation_cell(activation, addr, ASYNC_RELEASE_I32); - finish_async_box_activation(activation); - } + I32_BOX_RELEASE_QUARANTINE.with(|q| q.borrow_mut().push(addr)); BOX_RELEASE_COUNT.fetch_add(1, Ordering::Relaxed); } @@ -779,6 +877,19 @@ pub extern "C" fn js_bool_box_release(ptr: *mut BoolBox) { if !is_plausible_box_ptr(ptr.cast::()) { return; } + let activation = crate::promise::current_async_box_activation(); + if !activation.is_null() { + if !BOOL_BOX_REGISTRY.with(|r| r.borrow().contains(&addr)) { + return; + } + if !begin_pending_release(addr, ASYNC_RELEASE_BOOL) { + return; + } + park_async_activation_cell(activation, addr, ASYNC_RELEASE_BOOL); + finish_async_box_activation(activation); + BOX_RELEASE_COUNT.fetch_add(1, Ordering::Relaxed); + return; + } let was_registered = BOOL_BOX_REGISTRY.with(|r| r.borrow_mut().remove(&addr)); if !was_registered { return; @@ -787,13 +898,7 @@ pub extern "C" fn js_bool_box_release(ptr: *mut BoolBox) { unsafe { (*ptr).value = true; } - let activation = crate::promise::current_async_box_activation(); - if activation.is_null() { - BOOL_BOX_RELEASE_QUARANTINE.with(|q| q.borrow_mut().push(addr)); - } else { - park_async_activation_cell(activation, addr, ASYNC_RELEASE_BOOL); - finish_async_box_activation(activation); - } + BOOL_BOX_RELEASE_QUARANTINE.with(|q| q.borrow_mut().push(addr)); BOX_RELEASE_COUNT.fetch_add(1, Ordering::Relaxed); } @@ -811,22 +916,39 @@ pub fn scan_box_roots(mark: &mut dyn FnMut(f64)) { } pub fn scan_box_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - BOX_REGISTRY.with(|r| { - let r = r.borrow(); - for &addr in r.iter() { - let ptr = addr as *mut Box; - // Defensive: the registry should only contain valid live - // pointers, but if a stale entry slipped through we'd - // segfault on the deref. The tight bounds check on the - // 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 { - unsafe { - visitor.visit_nanbox_u64_raw_slot(&raw mut (*ptr).value); + let full_trace = crate::gc::full_trace_active(); + ASYNC_PENDING_RELEASES.with(|pending| { + let pending = pending.borrow(); + 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 + // segfault on the deref. The tight bounds check on the + // 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 { + unsafe { + visitor.visit_nanbox_u64_raw_slot(&raw mut (*ptr).value); + } } } - } + }); }); } @@ -1230,6 +1352,7 @@ static KEEP_JS_BOOL_BOX_SET: extern "C" fn(*mut BoolBox, i32) = js_bool_box_set; #[cfg(test)] pub(crate) fn test_clear_box_registry() { + crate::closure::test_clear_closure_box_capture_indexes(); BOX_REGISTRY.with(|r| r.borrow_mut().clear()); I32_BOX_REGISTRY.with(|r| r.borrow_mut().clear()); BOOL_BOX_REGISTRY.with(|r| r.borrow_mut().clear()); @@ -1240,6 +1363,7 @@ pub(crate) fn test_clear_box_registry() { I32_BOX_RELEASE_QUARANTINE.with(|q| q.borrow_mut().clear()); BOOL_BOX_RELEASE_QUARANTINE.with(|q| q.borrow_mut().clear()); ASYNC_RELEASED_CELLS.with(|cells| cells.borrow_mut().clear()); + ASYNC_PENDING_RELEASES.with(|pending| pending.borrow_mut().clear()); // Registry membership is not monotonic any more (#8208: `js_*box_release` // de-registers a completed activation's cells), so the positive cache is // kept coherent by an eviction on every un-registration rather than by @@ -1489,415 +1613,5 @@ mod tests { } #[cfg(test)] -mod release_tests { - use super::*; - - fn install_test_activation(activation: *mut AsyncBoxActivation) -> crate::promise::InlineTrap { - crate::promise::INLINE_TRAP.with(|trap| { - trap.replace(crate::promise::InlineTrap { - trap_next: std::ptr::null_mut(), - current_step: 0, - box_activation: activation, - }) - }) - } - - /// `BOX_ALLOC_COUNT` / `BOX_POOL_REUSE_COUNT` / `BOX_RELEASE_COUNT` are - /// process-global atomics, while the registries, quarantines and free - /// lists they describe are THREAD-LOCAL. Any test that asserts on a - /// counter *delta* is therefore not isolated by `test_clear_box_registry` - /// alone — a sibling test allocating on another harness thread lands in - /// the same atomics and moves the delta under it. Observed exactly that: - /// these tests pass under `--test-threads=1` and fail in parallel. - /// - /// Serialise the counter-asserting tests against each other. Tests that - /// only assert on addresses and registry membership are thread-local and - /// need no lock. - fn counter_guard() -> std::sync::MutexGuard<'static, ()> { - static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); - // A panicking test poisons the lock; the data is `()`, so recovering - // is right — otherwise one failure cascades into spurious ones. - LOCK.lock().unwrap_or_else(|e| e.into_inner()) - } - - /// A released cell must be INERT: de-registered (reads `undefined`, - /// writes dropped), evicted from the positive cache, and parked exactly - /// once no matter how many times the terminal arm re-runs (#7933 - /// follow-up; the stray-duplicate-resume path re-runs the release list). - #[test] - fn released_cell_is_inert_and_release_is_idempotent() { - super::test_clear_box_registry(); - let ptr = js_box_alloc_bits(crate::value::TAG_TRUE as i64); - assert!(is_registered_box_ptr(ptr)); - js_box_release(ptr); - assert!( - !is_registered_box_ptr(ptr), - "released cell must be de-registered (and cache-evicted)" - ); - assert_eq!( - js_box_get_bits(ptr) as u64, - crate::value::TAG_UNDEFINED, - "released cell must read undefined" - ); - js_box_set_bits(ptr, crate::value::TAG_TRUE as i64); - assert_eq!( - unsafe { (*ptr).value }, - crate::value::TAG_UNDEFINED, - "write to a released cell must be dropped" - ); - // Idempotence: a second release must not double-park the address — - // a double-park would hand the same cell to two future activations. - js_box_release(ptr); - js_box_release(ptr); - let parked = BOX_RELEASE_QUARANTINE - .with(|q| q.borrow().iter().filter(|&&a| a == ptr as usize).count()); - assert_eq!(parked, 1, "double release must park exactly once"); - } - - /// Fallback reuse contract: an untracked released cell becomes allocatable - /// only AFTER the outermost-pump quarantine flush, and the reused cell is - /// re-registered with the fresh initial value. - #[test] - fn released_cell_is_reused_only_after_flush() { - super::test_clear_box_registry(); - let first = js_box_alloc_bits(1.0f64.to_bits() as i64); - js_box_release(first); - // Not flushed yet: allocation must NOT reuse the parked cell. - let second = js_box_alloc_bits(2.0f64.to_bits() as i64); - assert_ne!( - first as usize, second as usize, - "quarantined cell must not be reused before the flush boundary" - ); - flush_released_boxes(); - let third = js_box_alloc_bits(3.0f64.to_bits() as i64); - assert_eq!( - first as usize, third as usize, - "flushed cell must be reused by the next allocation" - ); - assert!(is_registered_box_ptr(third), "reused cell re-registers"); - assert_eq!(js_box_get_bits(third), 3.0f64.to_bits() as i64); - } - - /// The #8208 floor-closing contract: terminal release alone does not make - /// a cell reusable while another queued resume still owns the activation. - /// The last queued/running-reference decrement publishes it immediately, - /// without waiting for the whole thread's task queue to drain. - #[test] - fn activation_cells_publish_at_its_reachability_zero() { - test_clear_box_registry(); - let activation = new_async_box_activation(); // lifecycle owner - retain_async_box_activation(activation); // currently running step - retain_async_box_activation(activation); // duplicate queued step - let previous = install_test_activation(activation); - - let released = js_box_alloc_bits(1.0f64.to_bits() as i64); - js_box_release(released); // also drops the lifecycle owner - - let before_zero = js_box_alloc_bits(2.0f64.to_bits() as i64); - assert_ne!( - released, before_zero, - "a queued resume still reaches the frame" - ); - release_async_box_activation(activation); // running step exits - let still_reachable = js_box_alloc_bits(3.0f64.to_bits() as i64); - assert_ne!( - released, still_reachable, - "duplicate task still owns the frame" - ); - - release_async_box_activation(activation); // duplicate dispatch exits - let after_zero = js_box_alloc_bits(4.0f64.to_bits() as i64); - assert_eq!( - released, after_zero, - "the final decrement must publish immediately" - ); - crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); - } - - /// Pending-await thunks carry a raw malloc-token pointer so the moving GC - /// cannot invalidate it. Reusing that token must not make an old thunk - /// name a new activation; the captured generation is the discriminator. - #[test] - fn recycled_activation_token_rejects_a_stale_generation() { - test_clear_box_registry(); - let first = new_async_box_activation(); - let first_id = async_box_activation_id(first); - assert_eq!(find_async_box_activation(first, first_id), first); - finish_async_box_activation(first); - assert!(find_async_box_activation(first, first_id).is_null()); - - let second = new_async_box_activation(); - let second_id = async_box_activation_id(second); - assert_eq!(second, first, "the test must exercise token recycling"); - assert_ne!(second_id, first_id); - assert!(find_async_box_activation(second, first_id).is_null()); - assert_eq!(find_async_box_activation(second, second_id), second); - finish_async_box_activation(second); - } - - /// Reachability is per activation, not a renamed global queue-empty gate: - /// B's completed frame is reusable while A still has a stale queued task. - #[test] - fn one_activation_does_not_quarantine_an_unrelated_completed_frame() { - test_clear_box_registry(); - - let activation_a = new_async_box_activation(); - retain_async_box_activation(activation_a); // running - retain_async_box_activation(activation_a); // delayed duplicate - let previous = install_test_activation(activation_a); - let a = js_box_alloc_bits(10.0f64.to_bits() as i64); - js_box_release(a); - release_async_box_activation(activation_a); // leave duplicate alive - - let activation_b = new_async_box_activation(); - retain_async_box_activation(activation_b); // running - install_test_activation(activation_b); - let b = js_box_alloc_bits(20.0f64.to_bits() as i64); - js_box_release(b); - release_async_box_activation(activation_b); // B reaches zero - - let reused_b = js_box_alloc_bits(30.0f64.to_bits() as i64); - assert_eq!( - b, reused_b, - "B must publish independently of A's queued task" - ); - assert_ne!(a, reused_b, "A must remain parked"); - - release_async_box_activation(activation_a); - let reused_a = js_box_alloc_bits(40.0f64.to_bits() as i64); - assert_eq!(a, reused_a, "A publishes when its own duplicate exits"); - crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); - } - - /// The pump's setjmp recovery must release the inner task reference that - /// `longjmp` skipped, while leaving a re-entrant caller's activation below - /// the saved depth untouched. - #[test] - fn exception_unwind_releases_only_this_pumps_activation_refs() { - test_clear_box_registry(); - let base_depth = crate::promise::async_box_execution_ref_depth(); - - let outer = new_async_box_activation(); - retain_async_box_activation(outer); // running owner - crate::promise::push_async_box_execution_ref(outer); - let previous = install_test_activation(outer); - let outer_cell = js_box_alloc_bits(1.0f64.to_bits() as i64); - js_box_release(outer_cell); // drop outer lifecycle owner - - let nested_depth = crate::promise::async_box_execution_ref_depth(); - let inner = new_async_box_activation(); - retain_async_box_activation(inner); // running owner skipped by longjmp - crate::promise::push_async_box_execution_ref(inner); - install_test_activation(inner); - let inner_cell = js_box_alloc_bits(2.0f64.to_bits() as i64); - js_box_release(inner_cell); // drop inner lifecycle owner - - crate::promise::unwind_async_box_execution_refs(nested_depth); - let reused_inner = js_box_alloc_bits(3.0f64.to_bits() as i64); - assert_eq!( - inner_cell, reused_inner, - "inner unwind must release its owner" - ); - assert_ne!( - outer_cell, reused_inner, - "outer owner is below nested depth" - ); - - crate::promise::pop_async_box_execution_ref(outer); - release_async_box_activation(outer); - assert_eq!( - crate::promise::async_box_execution_ref_depth(), - base_depth, - "test must restore the execution-ref stack" - ); - let reused_outer = js_box_alloc_bits(4.0f64.to_bits() as i64); - assert_eq!(outer_cell, reused_outer, "outer publishes at its own tail"); - crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); - } - - /// Generated async-step code reads the compiler-private control cells - /// with RAW loads (`load_async_i32_control_cell` / - /// `load_async_i1_control_cell`), never through the registry-checked - /// getters — so the PARKED VALUES are load-bearing: a stray duplicate - /// resume must observe `__gen_done == true` (the terminal short-circuit) - /// and, were it ever to read state, `-1` (no dispatch case matches). - #[test] - fn typed_control_cells_park_terminal_values() { - super::test_clear_box_registry(); - let state = js_i32_box_alloc(7); - let done = js_bool_box_alloc(0); - js_i32_box_release(state); - js_bool_box_release(done); - assert_eq!( - unsafe { (*state).value }, - -1, - "parked i32 control cell must raw-read as -1 (no state)" - ); - assert!( - unsafe { (*done).value }, - "parked i1 control cell must raw-read as true (done)" - ); - // And the checked getters treat them as not-a-box. - assert_eq!(js_i32_box_get(state), 0); - assert_eq!(js_bool_box_get(done), 0); - } - - /// The intrusive free list must round-trip a WHOLE cohort, not just one - /// cell. Each free cell's own 8 bytes hold the link to the next, so a - /// mis-written link would either lose most of the pool (silently - /// reverting to `std::alloc` and re-growing the residue) or splice a cell - /// in twice and hand one address to two live activations. - /// - /// Asserts all three: every cell comes back, each exactly once, and each - /// carries its own fresh value rather than a leftover link. - #[test] - fn the_intrusive_free_list_round_trips_a_whole_cohort() { - let _guard = counter_guard(); - super::test_clear_box_registry(); - const N: usize = 512; - let first: Vec<*mut Box> = (0..N) - .map(|i| js_box_alloc_bits((i as f64).to_bits() as i64)) - .collect(); - let minted: std::collections::HashSet = first.iter().map(|p| *p as usize).collect(); - assert_eq!(minted.len(), N, "the fixture must mint N distinct cells"); - - for p in &first { - js_box_release(*p); - } - flush_released_boxes(); - - let (a0, r0, _) = box_release_stats(); - let second: Vec<*mut Box> = (0..N) - .map(|i| js_box_alloc_bits((1000.0 + i as f64).to_bits() as i64)) - .collect(); - let (a1, r1, _) = box_release_stats(); - assert_eq!(a1 - a0, N as u64, "second cohort allocates N cells"); - assert_eq!( - r1 - r0, - N as u64, - "ALL N must come from the free list; {} fell through to std::alloc", - N as u64 - (r1 - r0) - ); - - let reused: std::collections::HashSet = second.iter().map(|p| *p as usize).collect(); - assert_eq!(reused.len(), N, "an address was handed out twice"); - assert_eq!( - reused, minted, - "reused cells must be exactly the minted set" - ); - - for (i, p) in second.iter().enumerate() { - assert_eq!( - js_box_get_bits(*p), - (1000.0 + i as f64).to_bits() as i64, - "cell {i} kept a stale free-list link instead of its value" - ); - } - // Drained: the next allocation has to mint. - let before = box_release_stats().1; - let _fresh = js_box_alloc_bits(0); - assert_eq!( - box_release_stats().1, - before, - "the list was drained, so this must be a fresh std::alloc" - ); - } - - /// perry#4898 discipline extends to release: a structurally-plausible - /// pointer that was never minted as a box must be a TOTAL no-op — no - /// deref, no park. - #[test] - fn foreign_pointer_release_is_a_total_noop() { - super::test_clear_box_registry(); - static RODATA: [u64; 2] = [0xDEAD_BEEF, 0xFEED_FACE]; - let fake = (&RODATA[0] as *const u64) as *mut Box; - js_box_release(fake); - assert_eq!(RODATA[0], 0xDEAD_BEEF, "rodata must be untouched"); - let parked = BOX_RELEASE_QUARANTINE.with(|q| q.borrow().len()); - assert_eq!(parked, 0, "foreign pointer must not be parked"); - } - - /// THE #7933-follow-up regression gate, as a counter assertion (the leak - /// is behaviorally invisible — a test that merely runs to completion - /// cannot fail on it). Simulate N async-activation lifecycles (alloc a - /// frame of cells, release it at terminal, hit the drain boundary every - /// "turn"): the malloc-side residue — cells that cost a real - /// `std::alloc` allocation, `allocs - pool_reuses` — must stay bounded - /// by one turn's working set instead of growing linearly with N. Before - /// the release/reuse machinery existed, residue == every cell ever - /// allocated (~500 B/activation of cells + registry, 119 MB on - /// asyncpipe_big). - #[test] - fn completed_activation_residue_is_bounded_not_linear() { - let _guard = counter_guard(); - super::test_clear_box_registry(); - const TURNS: usize = 100; - const ACTIVATIONS_PER_TURN: usize = 20; - // handle()-shaped frame: 3 JSValue cells + 1 i32 + 2 bool controls. - const CELLS_PER_ACTIVATION: usize = 6; - let (a0, r0, _) = box_release_stats(); - let mut distinct = std::collections::HashSet::new(); - for _ in 0..TURNS { - for _ in 0..ACTIVATIONS_PER_TURN { - let b1 = js_box_alloc_bits(crate::value::TAG_UNDEFINED as i64); - let b2 = js_box_alloc_bits(crate::value::TAG_UNDEFINED as i64); - let b3 = js_box_alloc_bits(crate::value::TAG_UNDEFINED as i64); - let state = js_i32_box_alloc(0); - let done = js_bool_box_alloc(0); - let exec = js_bool_box_alloc(0); - for b in [b1, b2, b3] { - distinct.insert(b as usize); - } - distinct.insert(state as usize); - distinct.insert(done as usize); - distinct.insert(exec as usize); - // Terminal state: release the whole frame. - for b in [b1, b2, b3] { - js_box_release(b); - } - js_i32_box_release(state); - js_bool_box_release(done); - js_bool_box_release(exec); - } - // Outermost microtask-pump boundary, task queue empty. - flush_released_boxes(); - } - let (a1, r1, _) = box_release_stats(); - // The counters are process-global; sibling tests on other threads - // also allocate boxes, so assert lower bounds and give the residue - // bound slack instead of demanding exact equality. - let total_allocs = (a1 - a0) as usize; - let residue = total_allocs.saturating_sub((r1 - r0) as usize); - let own_allocs = TURNS * ACTIVATIONS_PER_TURN * CELLS_PER_ACTIVATION; - assert!( - total_allocs >= own_allocs, - "every lifecycle allocates its frame ({total_allocs} < {own_allocs})" - ); - // One turn's working set (the first turn mints real cells; every - // later turn reuses them), plus generous slack for whatever the - // parallel sibling tests allocate (they use a handful of cells - // each). The pre-fix residue is TURNS * the per-turn bound, two - // orders of magnitude past this. - let bound = 4 * ACTIVATIONS_PER_TURN * CELLS_PER_ACTIVATION; - assert!( - residue <= bound, - "malloc residue must be bounded by one turn's working set: \ - residue={residue} bound={bound} (linear would be {total_allocs})" - ); - assert!( - distinct.len() <= bound, - "distinct cell addresses must be bounded (got {})", - distinct.len() - ); - // The registries hold only the (small) final turn's live set — the - // linear-growth signature is gone from the scan population too. - let reg_total = BOX_REGISTRY.with(|r| r.borrow().len()) - + I32_BOX_REGISTRY.with(|r| r.borrow().len()) - + BOOL_BOX_REGISTRY.with(|r| r.borrow().len()); - assert!( - reg_total <= bound, - "registry population must not scale with completed activations \ - (got {reg_total})" - ); - } -} +#[path = "box/release_tests.rs"] +mod release_tests; diff --git a/crates/perry-runtime/src/box/release_tests.rs b/crates/perry-runtime/src/box/release_tests.rs new file mode 100644 index 0000000000..ba8d4c28cd --- /dev/null +++ b/crates/perry-runtime/src/box/release_tests.rs @@ -0,0 +1,615 @@ +//! Terminal-release and closure-visibility tests for async boxes, split out of +//! `box.rs` to keep it under the 2000-line cap (#8303 took it to 2228). + +use super::*; + +use super::*; + +fn install_test_activation(activation: *mut AsyncBoxActivation) -> crate::promise::InlineTrap { + crate::promise::INLINE_TRAP.with(|trap| { + trap.replace(crate::promise::InlineTrap { + trap_next: std::ptr::null_mut(), + current_step: 0, + box_activation: activation, + }) + }) +} + +/// `BOX_ALLOC_COUNT` / `BOX_POOL_REUSE_COUNT` / `BOX_RELEASE_COUNT` are +/// process-global atomics, while the registries, quarantines and free +/// lists they describe are THREAD-LOCAL. Any test that asserts on a +/// counter *delta* is therefore not isolated by `test_clear_box_registry` +/// alone — a sibling test allocating on another harness thread lands in +/// the same atomics and moves the delta under it. Observed exactly that: +/// these tests pass under `--test-threads=1` and fail in parallel. +/// +/// Serialise the counter-asserting tests against each other. Tests that +/// only assert on addresses and registry membership are thread-local and +/// need no lock. +fn counter_guard() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + // A panicking test poisons the lock; the data is `()`, so recovering + // is right — otherwise one failure cascades into spurious ones. + LOCK.lock().unwrap_or_else(|e| e.into_inner()) +} + +/// A released cell must be INERT: de-registered (reads `undefined`, +/// writes dropped), evicted from the positive cache, and parked exactly +/// once no matter how many times the terminal arm re-runs (#7933 +/// follow-up; the stray-duplicate-resume path re-runs the release list). +#[test] +fn released_cell_is_inert_and_release_is_idempotent() { + super::test_clear_box_registry(); + let ptr = js_box_alloc_bits(crate::value::TAG_TRUE as i64); + assert!(is_registered_box_ptr(ptr)); + js_box_release(ptr); + assert!( + !is_registered_box_ptr(ptr), + "released cell must be de-registered (and cache-evicted)" + ); + assert_eq!( + js_box_get_bits(ptr) as u64, + crate::value::TAG_UNDEFINED, + "released cell must read undefined" + ); + js_box_set_bits(ptr, crate::value::TAG_TRUE as i64); + assert_eq!( + unsafe { (*ptr).value }, + crate::value::TAG_UNDEFINED, + "write to a released cell must be dropped" + ); + // Idempotence: a second release must not double-park the address — + // a double-park would hand the same cell to two future activations. + js_box_release(ptr); + js_box_release(ptr); + let parked = + BOX_RELEASE_QUARANTINE.with(|q| q.borrow().iter().filter(|&&a| a == ptr as usize).count()); + assert_eq!(parked, 1, "double release must park exactly once"); +} + +/// Fallback reuse contract: an untracked released cell becomes allocatable +/// only AFTER the outermost-pump quarantine flush, and the reused cell is +/// re-registered with the fresh initial value. +#[test] +fn released_cell_is_reused_only_after_flush() { + super::test_clear_box_registry(); + let first = js_box_alloc_bits(1.0f64.to_bits() as i64); + js_box_release(first); + // Not flushed yet: allocation must NOT reuse the parked cell. + let second = js_box_alloc_bits(2.0f64.to_bits() as i64); + assert_ne!( + first as usize, second as usize, + "quarantined cell must not be reused before the flush boundary" + ); + flush_released_boxes(); + let third = js_box_alloc_bits(3.0f64.to_bits() as i64); + assert_eq!( + first as usize, third as usize, + "flushed cell must be reused by the next allocation" + ); + assert!(is_registered_box_ptr(third), "reused cell re-registers"); + assert_eq!(js_box_get_bits(third), 3.0f64.to_bits() as i64); +} + +/// The #8208 floor-closing contract: terminal release alone does not make +/// a cell reusable while another queued resume still owns the activation. +/// The last queued/running-reference decrement publishes it immediately, +/// without waiting for the whole thread's task queue to drain. +#[test] +fn activation_cells_publish_at_its_reachability_zero() { + test_clear_box_registry(); + let activation = new_async_box_activation(); // lifecycle owner + retain_async_box_activation(activation); // currently running step + retain_async_box_activation(activation); // duplicate queued step + let previous = install_test_activation(activation); + + let released = js_box_alloc_bits(1.0f64.to_bits() as i64); + js_box_release(released); // also drops the lifecycle owner + + let before_zero = js_box_alloc_bits(2.0f64.to_bits() as i64); + assert_ne!( + released, before_zero, + "a queued resume still reaches the frame" + ); + release_async_box_activation(activation); // running step exits + let still_reachable = js_box_alloc_bits(3.0f64.to_bits() as i64); + assert_ne!( + released, still_reachable, + "duplicate task still owns the frame" + ); + + release_async_box_activation(activation); // duplicate dispatch exits + let after_zero = js_box_alloc_bits(4.0f64.to_bits() as i64); + assert_eq!( + released, after_zero, + "the final decrement must publish immediately" + ); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +/// #8213: a closure may outlive the async function that created it. Its +/// captured box remains readable after terminal release, and a child +/// closure created later increments that same cell's capture count. Only +/// death of the last such closure publishes the cell. +#[test] +fn escaped_closures_defer_activation_cell_publication_until_gc_death() { + test_clear_box_registry(); + let activation = new_async_box_activation(); + retain_async_box_activation(activation); // running step + let previous = install_test_activation(activation); + + let cell = js_box_alloc_bits(crate::value::JSValue::int32(41).bits() as i64); + let uncaptured = js_box_alloc_bits(crate::value::JSValue::int32(40).bits() as i64); + let outer = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::js_closure_set_box_capture_ptr(outer, 0, cell as i64); + crate::closure::js_closure_set_box_capture_ptr(outer, 0, cell as i64); + assert_eq!( + crate::closure::box_capture_count(cell as usize), + 1, + "a singleton cache hit may redeclare the same slot idempotently" + ); + + js_box_release(cell); // terminal lifecycle owner drops + js_box_release(uncaptured); + release_async_box_activation(activation); // running step exits + assert!(is_registered_box_ptr(cell)); + assert!( + !is_registered_box_ptr(uncaptured), + "one captured cell must not retain the rest of the async frame" + ); + assert_eq!( + js_box_get_bits(cell) as u64, + crate::value::JSValue::int32(41).bits(), + "an escaped closure must still observe its captured value" + ); + + // Model invoking `outer` after the async activation returned: no + // ambient async token exists, but the registered pending cell still + // lets a nested closure acquire its own capture count. + crate::promise::INLINE_TRAP.with(|trap| trap.set(crate::promise::InlineTrap::empty())); + 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 == outer as usize); + assert!( + is_registered_box_ptr(cell), + "the child closure remains live" + ); + + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == child as usize); + assert!(!is_registered_box_ptr(cell)); + let reused = js_box_alloc_bits(crate::value::JSValue::int32(42).bits() as i64); + assert_eq!(cell, reused, "the last closure death publishes the cell"); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +/// The two halves of the full-trace ownership rule must move together: +/// the drained box stops being a global root, while a closure already +/// proven live exposes that box's JSValue payload as its external child. +/// Otherwise a self-cycle is immortal (first half missing) or an escaped +/// closure observes a collected payload (second half missing). +#[test] +fn full_trace_treats_drained_closure_boxes_as_ephemeron_edges() { + test_clear_box_registry(); + let activation = new_async_box_activation(); + retain_async_box_activation(activation); + let previous = install_test_activation(activation); + 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); + js_box_release(cell); + release_async_box_activation(activation); + assert!(is_registered_box_ptr(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), + "the drained box must not root its own closure during 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"); + assert!( + slots.contains(&(cell as usize)), + "a marked closure must trace the drained box payload" + ); + unsafe { (*header).gc_flags = saved_flags }; + crate::gc::finish_full_trace(); + + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == closure as usize); + assert!(!is_registered_box_ptr(cell)); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +/// Pointer-shaped generic captures are common in runtime-created +/// closures. Even if their bits happen to equal a live box address, only +/// the compiler-declared setter may create a box lifetime edge. +#[test] +fn generic_pointer_capture_does_not_retain_an_async_box() { + test_clear_box_registry(); + let activation = new_async_box_activation(); + retain_async_box_activation(activation); + let previous = install_test_activation(activation); + let cell = js_box_alloc_bits(crate::value::JSValue::int32(5).bits() as i64); + let closure = crate::closure::js_closure_alloc(std::ptr::null(), 1); + + crate::closure::js_closure_set_capture_ptr(closure, 0, cell as i64); + assert_eq!(crate::closure::box_capture_count(cell as usize), 0); + js_box_release(cell); + release_async_box_activation(activation); + assert!(!is_registered_box_ptr(cell)); + + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == closure as usize); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +/// A closure created during an async step can also capture a mutable box +/// owned by an outer scope. The raw capture count must not make that box a +/// terminal-release candidate for the ambient activation. +#[test] +fn outer_scope_box_capture_is_not_bound_to_the_ambient_activation() { + test_clear_box_registry(); + let outer_cell = js_box_alloc_bits(crate::value::JSValue::int32(7).bits() as i64); + let activation = new_async_box_activation(); + retain_async_box_activation(activation); + let previous = install_test_activation(activation); + + let closure = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::js_closure_set_box_capture_ptr(closure, 0, outer_cell as i64); + assert_eq!(crate::closure::box_capture_count(outer_cell as usize), 1); + + let frame_cell = js_box_alloc_bits(crate::value::JSValue::int32(8).bits() as i64); + js_box_release(frame_cell); + release_async_box_activation(activation); + assert!(!is_registered_box_ptr(frame_cell)); + assert!(is_registered_box_ptr(outer_cell)); + assert_eq!( + js_box_get_bits(outer_cell) as u64, + crate::value::JSValue::int32(7).bits() + ); + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == closure as usize); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +#[test] +fn closure_move_rekeys_the_capture_count_before_death_pruning() { + test_clear_box_registry(); + let activation = new_async_box_activation(); + retain_async_box_activation(activation); + let previous = install_test_activation(activation); + let cell = js_box_alloc_bits(crate::value::JSValue::int32(9).bits() 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); + js_box_release(cell); + release_async_box_activation(activation); + + let moved = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::closure_box_captures_owner_moved(closure as usize, moved as usize); + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == closure as usize); + assert!( + is_registered_box_ptr(cell), + "the old address no longer owns the retain" + ); + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == moved as usize); + assert!(!is_registered_box_ptr(cell)); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +#[test] +fn runtime_closure_clone_copies_exact_box_capture_edges() { + test_clear_box_registry(); + let activation = new_async_box_activation(); + retain_async_box_activation(activation); + let previous = install_test_activation(activation); + let cell = js_box_alloc_bits(crate::value::JSValue::int32(10).bits() as i64); + let source = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::js_closure_set_box_capture_ptr(source, 0, cell as i64); + let cloned = crate::closure::js_closure_alloc(std::ptr::null(), 1); + crate::closure::clone_closure_box_captures(source, cloned); + assert_eq!(crate::closure::box_capture_count(cell as usize), 2); + + js_box_release(cell); + release_async_box_activation(activation); + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == source as usize); + assert!( + is_registered_box_ptr(cell), + "the clone retains its exact edge" + ); + crate::closure::prune_dead_closure_box_capture_owners(&|owner| owner == cloned as usize); + assert!(!is_registered_box_ptr(cell)); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +/// Pending-await thunks carry a raw malloc-token pointer so the moving GC +/// cannot invalidate it. Reusing that token must not make an old thunk +/// name a new activation; the captured generation is the discriminator. +#[test] +fn recycled_activation_token_rejects_a_stale_generation() { + test_clear_box_registry(); + let first = new_async_box_activation(); + let first_id = async_box_activation_id(first); + assert_eq!(find_async_box_activation(first, first_id), first); + finish_async_box_activation(first); + assert!(find_async_box_activation(first, first_id).is_null()); + + let second = new_async_box_activation(); + let second_id = async_box_activation_id(second); + assert_eq!(second, first, "the test must exercise token recycling"); + assert_ne!(second_id, first_id); + assert!(find_async_box_activation(second, first_id).is_null()); + assert_eq!(find_async_box_activation(second, second_id), second); + finish_async_box_activation(second); +} + +/// Reachability is per activation, not a renamed global queue-empty gate: +/// B's completed frame is reusable while A still has a stale queued task. +#[test] +fn one_activation_does_not_quarantine_an_unrelated_completed_frame() { + test_clear_box_registry(); + + let activation_a = new_async_box_activation(); + retain_async_box_activation(activation_a); // running + retain_async_box_activation(activation_a); // delayed duplicate + let previous = install_test_activation(activation_a); + let a = js_box_alloc_bits(10.0f64.to_bits() as i64); + js_box_release(a); + release_async_box_activation(activation_a); // leave duplicate alive + + let activation_b = new_async_box_activation(); + retain_async_box_activation(activation_b); // running + install_test_activation(activation_b); + let b = js_box_alloc_bits(20.0f64.to_bits() as i64); + js_box_release(b); + release_async_box_activation(activation_b); // B reaches zero + + let reused_b = js_box_alloc_bits(30.0f64.to_bits() as i64); + assert_eq!( + b, reused_b, + "B must publish independently of A's queued task" + ); + assert_ne!(a, reused_b, "A must remain parked"); + + release_async_box_activation(activation_a); + let reused_a = js_box_alloc_bits(40.0f64.to_bits() as i64); + assert_eq!(a, reused_a, "A publishes when its own duplicate exits"); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +/// The pump's setjmp recovery must release the inner task reference that +/// `longjmp` skipped, while leaving a re-entrant caller's activation below +/// the saved depth untouched. +#[test] +fn exception_unwind_releases_only_this_pumps_activation_refs() { + test_clear_box_registry(); + let base_depth = crate::promise::async_box_execution_ref_depth(); + + let outer = new_async_box_activation(); + retain_async_box_activation(outer); // running owner + crate::promise::push_async_box_execution_ref(outer); + let previous = install_test_activation(outer); + let outer_cell = js_box_alloc_bits(1.0f64.to_bits() as i64); + js_box_release(outer_cell); // drop outer lifecycle owner + + let nested_depth = crate::promise::async_box_execution_ref_depth(); + let inner = new_async_box_activation(); + retain_async_box_activation(inner); // running owner skipped by longjmp + crate::promise::push_async_box_execution_ref(inner); + install_test_activation(inner); + let inner_cell = js_box_alloc_bits(2.0f64.to_bits() as i64); + js_box_release(inner_cell); // drop inner lifecycle owner + + crate::promise::unwind_async_box_execution_refs(nested_depth); + let reused_inner = js_box_alloc_bits(3.0f64.to_bits() as i64); + assert_eq!( + inner_cell, reused_inner, + "inner unwind must release its owner" + ); + assert_ne!( + outer_cell, reused_inner, + "outer owner is below nested depth" + ); + + crate::promise::pop_async_box_execution_ref(outer); + release_async_box_activation(outer); + assert_eq!( + crate::promise::async_box_execution_ref_depth(), + base_depth, + "test must restore the execution-ref stack" + ); + let reused_outer = js_box_alloc_bits(4.0f64.to_bits() as i64); + assert_eq!(outer_cell, reused_outer, "outer publishes at its own tail"); + crate::promise::INLINE_TRAP.with(|trap| trap.set(previous)); +} + +/// Generated async-step code reads the compiler-private control cells +/// with RAW loads (`load_async_i32_control_cell` / +/// `load_async_i1_control_cell`), never through the registry-checked +/// getters — so the PARKED VALUES are load-bearing: a stray duplicate +/// resume must observe `__gen_done == true` (the terminal short-circuit) +/// and, were it ever to read state, `-1` (no dispatch case matches). +#[test] +fn typed_control_cells_park_terminal_values() { + super::test_clear_box_registry(); + let state = js_i32_box_alloc(7); + let done = js_bool_box_alloc(0); + js_i32_box_release(state); + js_bool_box_release(done); + assert_eq!( + unsafe { (*state).value }, + -1, + "parked i32 control cell must raw-read as -1 (no state)" + ); + assert!( + unsafe { (*done).value }, + "parked i1 control cell must raw-read as true (done)" + ); + // And the checked getters treat them as not-a-box. + assert_eq!(js_i32_box_get(state), 0); + assert_eq!(js_bool_box_get(done), 0); +} + +/// The intrusive free list must round-trip a WHOLE cohort, not just one +/// cell. Each free cell's own 8 bytes hold the link to the next, so a +/// mis-written link would either lose most of the pool (silently +/// reverting to `std::alloc` and re-growing the residue) or splice a cell +/// in twice and hand one address to two live activations. +/// +/// Asserts all three: every cell comes back, each exactly once, and each +/// carries its own fresh value rather than a leftover link. +#[test] +fn the_intrusive_free_list_round_trips_a_whole_cohort() { + let _guard = counter_guard(); + super::test_clear_box_registry(); + const N: usize = 512; + let first: Vec<*mut Box> = (0..N) + .map(|i| js_box_alloc_bits((i as f64).to_bits() as i64)) + .collect(); + let minted: std::collections::HashSet = first.iter().map(|p| *p as usize).collect(); + assert_eq!(minted.len(), N, "the fixture must mint N distinct cells"); + + for p in &first { + js_box_release(*p); + } + flush_released_boxes(); + + let (a0, r0, _) = box_release_stats(); + let second: Vec<*mut Box> = (0..N) + .map(|i| js_box_alloc_bits((1000.0 + i as f64).to_bits() as i64)) + .collect(); + let (a1, r1, _) = box_release_stats(); + assert_eq!(a1 - a0, N as u64, "second cohort allocates N cells"); + assert_eq!( + r1 - r0, + N as u64, + "ALL N must come from the free list; {} fell through to std::alloc", + N as u64 - (r1 - r0) + ); + + let reused: std::collections::HashSet = second.iter().map(|p| *p as usize).collect(); + assert_eq!(reused.len(), N, "an address was handed out twice"); + assert_eq!( + reused, minted, + "reused cells must be exactly the minted set" + ); + + for (i, p) in second.iter().enumerate() { + assert_eq!( + js_box_get_bits(*p), + (1000.0 + i as f64).to_bits() as i64, + "cell {i} kept a stale free-list link instead of its value" + ); + } + // Drained: the next allocation has to mint. + let before = box_release_stats().1; + let _fresh = js_box_alloc_bits(0); + assert_eq!( + box_release_stats().1, + before, + "the list was drained, so this must be a fresh std::alloc" + ); +} + +/// perry#4898 discipline extends to release: a structurally-plausible +/// pointer that was never minted as a box must be a TOTAL no-op — no +/// deref, no park. +#[test] +fn foreign_pointer_release_is_a_total_noop() { + super::test_clear_box_registry(); + static RODATA: [u64; 2] = [0xDEAD_BEEF, 0xFEED_FACE]; + let fake = (&RODATA[0] as *const u64) as *mut Box; + js_box_release(fake); + assert_eq!(RODATA[0], 0xDEAD_BEEF, "rodata must be untouched"); + let parked = BOX_RELEASE_QUARANTINE.with(|q| q.borrow().len()); + assert_eq!(parked, 0, "foreign pointer must not be parked"); +} + +/// THE #7933-follow-up regression gate, as a counter assertion (the leak +/// is behaviorally invisible — a test that merely runs to completion +/// cannot fail on it). Simulate N async-activation lifecycles (alloc a +/// frame of cells, release it at terminal, hit the drain boundary every +/// "turn"): the malloc-side residue — cells that cost a real +/// `std::alloc` allocation, `allocs - pool_reuses` — must stay bounded +/// by one turn's working set instead of growing linearly with N. Before +/// the release/reuse machinery existed, residue == every cell ever +/// allocated (~500 B/activation of cells + registry, 119 MB on +/// asyncpipe_big). +#[test] +fn completed_activation_residue_is_bounded_not_linear() { + let _guard = counter_guard(); + super::test_clear_box_registry(); + const TURNS: usize = 100; + const ACTIVATIONS_PER_TURN: usize = 20; + // handle()-shaped frame: 3 JSValue cells + 1 i32 + 2 bool controls. + const CELLS_PER_ACTIVATION: usize = 6; + let (a0, r0, _) = box_release_stats(); + let mut distinct = std::collections::HashSet::new(); + for _ in 0..TURNS { + for _ in 0..ACTIVATIONS_PER_TURN { + let b1 = js_box_alloc_bits(crate::value::TAG_UNDEFINED as i64); + let b2 = js_box_alloc_bits(crate::value::TAG_UNDEFINED as i64); + let b3 = js_box_alloc_bits(crate::value::TAG_UNDEFINED as i64); + let state = js_i32_box_alloc(0); + let done = js_bool_box_alloc(0); + let exec = js_bool_box_alloc(0); + for b in [b1, b2, b3] { + distinct.insert(b as usize); + } + distinct.insert(state as usize); + distinct.insert(done as usize); + distinct.insert(exec as usize); + // Terminal state: release the whole frame. + for b in [b1, b2, b3] { + js_box_release(b); + } + js_i32_box_release(state); + js_bool_box_release(done); + js_bool_box_release(exec); + } + // Outermost microtask-pump boundary, task queue empty. + flush_released_boxes(); + } + let (a1, r1, _) = box_release_stats(); + // The counters are process-global; sibling tests on other threads + // also allocate boxes, so assert lower bounds and give the residue + // bound slack instead of demanding exact equality. + let total_allocs = (a1 - a0) as usize; + let residue = total_allocs.saturating_sub((r1 - r0) as usize); + let own_allocs = TURNS * ACTIVATIONS_PER_TURN * CELLS_PER_ACTIVATION; + assert!( + total_allocs >= own_allocs, + "every lifecycle allocates its frame ({total_allocs} < {own_allocs})" + ); + // One turn's working set (the first turn mints real cells; every + // later turn reuses them), plus generous slack for whatever the + // parallel sibling tests allocate (they use a handful of cells + // each). The pre-fix residue is TURNS * the per-turn bound, two + // orders of magnitude past this. + let bound = 4 * ACTIVATIONS_PER_TURN * CELLS_PER_ACTIVATION; + assert!( + residue <= bound, + "malloc residue must be bounded by one turn's working set: \ + residue={residue} bound={bound} (linear would be {total_allocs})" + ); + assert!( + distinct.len() <= bound, + "distinct cell addresses must be bounded (got {})", + distinct.len() + ); + // The registries hold only the (small) final turn's live set — the + // linear-growth signature is gone from the scan population too. + let reg_total = BOX_REGISTRY.with(|r| r.borrow().len()) + + I32_BOX_REGISTRY.with(|r| r.borrow().len()) + + BOOL_BOX_REGISTRY.with(|r| r.borrow().len()); + assert!( + reg_total <= bound, + "registry population must not scale with completed activations \ + (got {reg_total})" + ); +} diff --git a/crates/perry-runtime/src/closure/alloc.rs b/crates/perry-runtime/src/closure/alloc.rs index 6fc236a1cd..70c85b989a 100644 --- a/crates/perry-runtime/src/closure/alloc.rs +++ b/crates/perry-runtime/src/closure/alloc.rs @@ -490,6 +490,20 @@ pub extern "C" fn js_closure_set_capture_bits( } } +/// Set a capture slot which codegen has proven contains a raw variable-box +/// pointer. Keeping this separate from the generic setter prevents arbitrary +/// pointer-shaped JS values from becoming false box-lifetime edges. +#[no_mangle] +pub extern "C" fn js_closure_set_box_capture_ptr( + closure: *mut ClosureHeader, + index: u32, + value: i64, +) { + js_closure_set_capture_bits(closure, index, value as u64); + let cell = crate::r#box::registered_box_capture_addr(value as usize); + super::box_captures::set_closure_box_capture(closure, index, cell); +} + /// Get a captured value (as i64 pointer) by index #[no_mangle] pub extern "C" fn js_closure_get_capture_ptr(closure: *const ClosureHeader, index: u32) -> i64 { @@ -510,3 +524,7 @@ static KEEP_JS_CLOSURE_GET_CAPTURE_BITS: extern "C" fn(*const ClosureHeader, u32 #[used] static KEEP_JS_CLOSURE_SET_CAPTURE_BITS: extern "C" fn(*mut ClosureHeader, u32, u64) = js_closure_set_capture_bits; +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_CLOSURE_SET_BOX_CAPTURE_PTR: extern "C" fn(*mut ClosureHeader, u32, i64) = + js_closure_set_box_capture_ptr; diff --git a/crates/perry-runtime/src/closure/box_captures.rs b/crates/perry-runtime/src/closure/box_captures.rs new file mode 100644 index 0000000000..2cc7fd7b4a --- /dev/null +++ b/crates/perry-runtime/src/closure/box_captures.rs @@ -0,0 +1,177 @@ +//! Lifetime bridge between GC closures and malloc-side async box cells. +//! +//! Codegen identifies the capture slots which contain raw box addresses. We +//! count only those declared edges before a box reaches terminal +//! `ReleaseBoxes`; arbitrary JS values must never be guessed to be boxes from +//! 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. + +use super::ClosureHeader; +use std::cell::RefCell; + +type BoxCaptureSlots = Vec<(u32, usize)>; + +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. + static BOX_CAPTURE_COUNTS: RefCell> = + RefCell::new(crate::fast_hash::new_ptr_hash_map()); +} + +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 + .checked_add(amount) + .expect("box capture count overflow"); + }); +} + +fn decrement_cell_capture_count(cell: usize, amount: usize) { + 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 + } else { + false + } + }); + if reached_zero { + crate::r#box::box_capture_count_reached_zero(cell); + } +} + +pub(crate) fn box_capture_count(cell: usize) -> usize { + BOX_CAPTURE_COUNTS + .with(|counts| counts.borrow().get(&cell).copied()) + .unwrap_or(0) +} + +/// Visit the JSValue payload slots reached through one live closure's exact +/// compiler-declared box captures. During a full trace drained async boxes are +/// no longer global roots: this is their ephemeron half, preserving +/// `live closure -> box payload` without allowing `box payload -> closure` to +/// keep an otherwise unreachable cycle alive. +pub(crate) fn visit_closure_box_payload_slots_mut(closure: usize, mut visit: impl FnMut(*mut u64)) { + if closure == 0 || !crate::gc::full_trace_active() { + return; + } + CLOSURE_BOX_CELLS.with(|captures| { + let captures = captures.borrow(); + let Some(cells) = captures.get(&closure) else { + return; + }; + for &(_, cell) in cells { + crate::r#box::visit_pending_captured_js_box_payload_slot(cell, &mut visit); + } + }); +} + +/// Record a compiler-declared boxed capture slot. +pub(super) fn set_closure_box_capture( + closure: *mut ClosureHeader, + index: u32, + cell: Option, +) { + if closure.is_null() { + return; + } + let closure = closure as usize; + 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); + if let Some(cell) = cell { + slots.push((index, cell)); + } + if slots.is_empty() { + captures.remove(&closure); + } + previous + }); + if previous == cell { + return; + } + if let Some(cell) = cell { + increment_cell_capture_count(cell, 1); + } + if let Some(previous) = previous { + decrement_cell_capture_count(previous, 1); + } +} + +/// Copy exact boxed-slot metadata when runtime code clones a closure. +pub(crate) fn clone_closure_box_captures( + source: *const ClosureHeader, + destination: *mut ClosureHeader, +) { + if source.is_null() || destination.is_null() || source.cast_mut() == destination { + return; + } + let source = source as usize; + let destination = destination as usize; + let copied = CLOSURE_BOX_CELLS.with(|all| { + let mut all = all.borrow_mut(); + debug_assert!(!all.contains_key(&destination)); + let copied = all.get(&source).cloned().unwrap_or_default(); + if !copied.is_empty() { + all.insert(destination, copied.clone()); + } + copied + }); + for (_, cell) in copied { + increment_cell_capture_count(cell, 1); + } +} + +pub(crate) fn closure_box_captures_owner_moved(old_owner: usize, new_owner: usize) { + if old_owner == 0 || new_owner == 0 || old_owner == new_owner { + return; + } + CLOSURE_BOX_CELLS.with(|all| { + let mut all = all.borrow_mut(); + if let Some(cells) = all.remove(&old_owner) { + debug_assert!(!all.contains_key(&new_owner)); + all.insert(new_owner, cells); + } + }); +} + +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::>() + }); + 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); + } + } +} + +#[cfg(test)] +pub(crate) fn test_clear_closure_box_capture_indexes() { + CLOSURE_BOX_CELLS.with(|all| all.borrow_mut().clear()); + BOX_CAPTURE_COUNTS.with(|all| all.borrow_mut().clear()); +} diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index 2dbca70570..e1fa71728d 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -197,6 +197,7 @@ pub(crate) fn closure_dynamic_side_tables_nonempty() -> bool { /// are process-global: foreign threads' closure addresses don't attribute /// and are skipped (documented residual). pub(crate) fn prune_dead_closure_side_table_owners(is_dead_closure: &dyn Fn(usize) -> bool) { + super::prune_dead_closure_box_capture_owners(is_dead_closure); let mut verdicts: HashMap = HashMap::new(); let mut is_dead = |owner: usize| -> bool { *verdicts @@ -851,6 +852,7 @@ pub extern "C" fn js_closure_unbind_this(val: f64) -> f64 { *dst_captures.add(i) = *src_captures.add(i); } rebuild_closure_layout_and_barriers(new_closure, count); + super::clone_closure_box_captures(source_ptr as *const ClosureHeader, new_closure); // NaN-box the new closure pointer let new_ptr = new_closure as u64; f64::from_bits(0x7FFD_0000_0000_0000 | (new_ptr & 0x0000_FFFF_FFFF_FFFF)) @@ -1102,6 +1104,7 @@ pub(crate) fn clone_closure_rebind_this(closure_bits: u64, recv_box: f64) -> u64 // GC_STORE_AUDIT(BARRIERED): rebound this capture is included in the layout/barrier rebuild. *dst_captures.add(this_slot) = recv_handle.get_nanbox_f64().to_bits(); rebuild_closure_layout_and_barriers(new_closure, count); + super::clone_closure_box_captures(source_ptr as *const ClosureHeader, new_closure); let new_ptr = new_closure as u64; 0x7FFD_0000_0000_0000 | (new_ptr & 0x0000_FFFF_FFFF_FFFF) } diff --git a/crates/perry-runtime/src/closure/mod.rs b/crates/perry-runtime/src/closure/mod.rs index 05e2464433..b2c039d807 100644 --- a/crates/perry-runtime/src/closure/mod.rs +++ b/crates/perry-runtime/src/closure/mod.rs @@ -6,6 +6,7 @@ //! - Followed by captured values (as f64 or i64 pointers) mod alloc; +mod box_captures; mod dispatch; mod dynamic_props; mod registry; @@ -20,10 +21,11 @@ pub use alloc::{ closure_alloc_storage, closure_capture_slots_mut, closure_payload_size, js_closure_alloc, js_closure_alloc_singleton, js_closure_alloc_with_captures_singleton, js_closure_get_capture_bits, js_closure_get_capture_f64, js_closure_get_capture_ptr, - js_closure_get_func, js_closure_set_capture_bits, js_closure_set_capture_f64, - js_closure_set_capture_ptr, note_closure_capture_slot, rebuild_closure_layout_and_barriers, - scan_singleton_closure_roots_mut, ClosureHeader, CLOSURE_ALLOC_COUNT, - CLOSURE_CAP_SINGLETON_HIT, CLOSURE_CAP_SINGLETON_MISS, CLOSURE_TYPE_TAG_OFFSET, + js_closure_get_func, js_closure_set_box_capture_ptr, js_closure_set_capture_bits, + js_closure_set_capture_f64, js_closure_set_capture_ptr, note_closure_capture_slot, + rebuild_closure_layout_and_barriers, scan_singleton_closure_roots_mut, ClosureHeader, + CLOSURE_ALLOC_COUNT, CLOSURE_CAP_SINGLETON_HIT, CLOSURE_CAP_SINGLETON_MISS, + CLOSURE_TYPE_TAG_OFFSET, }; pub use registry::{ @@ -55,6 +57,12 @@ pub(crate) use dispatch::{ }; pub use unbox::js_closure_unbox_callee_checked; +#[cfg(test)] +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, +}; #[cfg(test)] pub(crate) use dynamic_props::test_clear_closure_side_tables; pub(crate) use dynamic_props::{ diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index 9ff1dc2796..af71d2b60f 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -1166,7 +1166,7 @@ impl GcCycleState { .expect("valid-pointer builder exists"); self.valid_ptrs = Some(builder.finish()); if self.minor.is_none() { - crate::proxy::gc_begin_full_trace(); + begin_full_trace(); } trace_phase_record(&mut self.trace, "build_valid_pointer_set", phase_start); // Enable the incremental mark barrier for BOTH kinds. A budgeted @@ -1692,7 +1692,7 @@ impl GcCycleState { incremental_mark_barrier_disable(); } if full_trace { - crate::proxy::gc_finish_full_trace(); + finish_full_trace(); } let (do_age_bump, reclaim_dead_old_blocks, targeted_old_blocks, sweep_malloc) = diff --git a/crates/perry-runtime/src/gc/dead_owner.rs b/crates/perry-runtime/src/gc/dead_owner.rs index 62a53466a7..334cc429da 100644 --- a/crates/perry-runtime/src/gc/dead_owner.rs +++ b/crates/perry-runtime/src/gc/dead_owner.rs @@ -348,7 +348,8 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ prune: crate::symbol::prune_dead_symbol_pointers, }, DeadKeyPrune { - table: "CLOSURE_PROPS + CLOSURE_STATIC_PROTOTYPES + CLOSURE_DELETED_KEYS", + table: + "CLOSURE_PROPS + CLOSURE_STATIC_PROTOTYPES + CLOSURE_DELETED_KEYS + CLOSURE_BOX_CELLS", owner: DeadKeyOwner::Closure, prune: crate::closure::prune_dead_closure_side_table_owners, }, diff --git a/crates/perry-runtime/src/gc/full_trace.rs b/crates/perry-runtime/src/gc/full_trace.rs new file mode 100644 index 0000000000..da220e9a14 --- /dev/null +++ b/crates/perry-runtime/src/gc/full_trace.rs @@ -0,0 +1,31 @@ +//! Full-heap trace scope shared by weak/ephemeron-style runtime owners. +//! +//! Minors cannot infer whether an old owner is live because they deliberately +//! do not trace the whole old generation. Runtime registries which become weak +//! only for a full trace use this scope to distinguish those collections from +//! non-copying minors without coupling their lifetime rules to one another. + +use std::cell::Cell; + +crate::perry_thread_local! { + static FULL_TRACE_ACTIVE: Cell = const { Cell::new(false) }; +} + +pub(crate) fn begin_full_trace() { + FULL_TRACE_ACTIVE.with(|active| { + assert!(!active.replace(true), "full trace already active"); + }); + crate::proxy::gc_begin_full_trace(); +} + +pub(crate) fn finish_full_trace() { + crate::proxy::gc_finish_full_trace(); + FULL_TRACE_ACTIVE.with(|active| { + assert!(active.replace(false), "no full trace active"); + }); +} + +#[inline(always)] +pub(crate) fn full_trace_active() -> bool { + FULL_TRACE_ACTIVE.with(Cell::get) +} diff --git a/crates/perry-runtime/src/gc/layout_slot_visit.rs b/crates/perry-runtime/src/gc/layout_slot_visit.rs index ebc0aeabb8..74ccb51d6d 100644 --- a/crates/perry-runtime/src/gc/layout_slot_visit.rs +++ b/crates/perry-runtime/src/gc/layout_slot_visit.rs @@ -15,6 +15,21 @@ pub(super) unsafe fn visit_gc_layout_slot_descriptors( visit: &mut dyn FnMut(GcMutableSlotDescriptor), ) { let mut child_slots = gc_child_slots(header); + // #8213: drained async box cells are weak registry entries during a full + // trace. A closure proven live by the mark set is their owner, so enumerate + // the malloc-side JSValue payload as an external child slot. Requiring a + // marked/pinned closure prevents generic descriptor walks over dead old + // objects from accidentally resurrecting the cycle this edge is meant to + // break. + if (*header).obj_type == GC_TYPE_CLOSURE + && (*header).gc_flags & (GC_FLAG_MARKED | GC_FLAG_PINNED) != 0 + && full_trace_active() + { + let closure = (header as *mut u8).add(GC_HEADER_SIZE) as usize; + crate::closure::visit_closure_box_payload_slots_mut(closure, |slot| { + visit(fixed_slot(slot)); + }); + } // #8112: the authoritative ordered-keys edge, taken from the descriptor // `gc_child_slots` already resolved for this receiver. It is the boxed // record's OWN `keys` word, so the collector marks through it and rewrites diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 5b5335da82..9b6d5ff566 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -58,6 +58,8 @@ mod hot_tls; pub(crate) use hot_tls::*; mod roots; pub use roots::*; +mod full_trace; +pub(crate) use full_trace::*; #[cfg(test)] /// Rewrite runtime-handle roots only; this deliberately does not rewrite the /// installed `INLINE_TRAP`, whose scanner is exercised separately. diff --git a/crates/perry-runtime/src/gc/types.rs b/crates/perry-runtime/src/gc/types.rs index 4af28ec24d..a707a8ab47 100644 --- a/crates/perry-runtime/src/gc/types.rs +++ b/crates/perry-runtime/src/gc/types.rs @@ -772,6 +772,7 @@ pub(crate) fn gc_type_after_payload_move(obj_type: u8, old_user: usize, new_user } GcMoveHookKind::ClosureDynamicProps => { crate::closure::closure_dynamic_props_owner_moved(old_user, new_user); + crate::closure::closure_box_captures_owner_moved(old_user, new_user); } GcMoveHookKind::MapSideTables => { crate::map::map_header_moved_for_gc(old_user, new_user); diff --git a/crates/perry-transform/src/generator/box_release.rs b/crates/perry-transform/src/generator/box_release.rs index 57232df851..9a8ade3e15 100644 --- a/crates/perry-transform/src/generator/box_release.rs +++ b/crates/perry-transform/src/generator/box_release.rs @@ -30,185 +30,15 @@ //! box cell for the life of the thread, so "was a box" can never become "is //! another object". //! -//! Releasing a cell whose value is still *reachable* would be a silent -//! use-after-release (a wrong answer, not a crash), so a cell is only released -//! when no closure in the function can hold its address. This module computes -//! that set. -//! -//! ## Why "referenced by a closure" is the right, and sufficient, test -//! -//! A box address is never a JS value: `LocalGet`/`LocalSet` on a boxed local -//! lower to `js_box_get`/`js_box_set` on the cell, and the raw address only -//! ever leaves the activation through a **closure capture slot**. Codegen -//! forwards the address into a capture slot for exactly the ids in -//! `compute_auto_captures(closure) ∩ boxed_vars`, and `compute_auto_captures` -//! is `explicit captures ∪ collect_ref_ids_in_stmts(closure body)`. -//! -//! [`closure_visible_ids`] returns a **superset** of that: the explicit -//! `captures` *and* `mutable_captures` lists plus -//! `perry_hir::analysis::collect_local_refs_expr` over the whole closure -//! expression (which descends into nested closures). An id it misses is an id -//! codegen's own free-variable walk also misses, so no capture slot for that id -//! exists and clearing its cell is unobservable. -//! -//! The one construct that breaks that argument is sloppy-mode `with`: -//! `Expr::WithGet`/`Expr::WithSet` carry a fallback `LocalId` as a *leaf field* -//! that `collect_local_refs_expr` does not report. A body containing either -//! poisons the analysis outright (`None`), and the caller clears nothing. +//! Closure-visible cells are named by the terminal release too, but the runtime +//! keeps them live and registered while a GC closure still carries their raw +//! address. Closure move/death hooks maintain per-cell capture counts. Once the +//! queued/running activation steps drain, every uncaptured cell publishes and +//! each captured cell waits independently for its final count to disappear. +//! Thus one escaped closure does not retain the complete activation frame. use perry_hir::ir::*; use perry_hir::types::LocalId; -use std::collections::HashSet; - -struct Scan { - out: HashSet, - /// Set when a construct is seen whose LocalId references cannot be - /// enumerated (sloppy `with`). The whole analysis is then unusable. - poisoned: bool, -} - -/// Every `LocalId` that some closure inside `stmts` can observe — its declared -/// capture lists plus every local referenced anywhere in its body (transitively -/// through nested closures). -/// -/// Returns `None` when the body contains a construct whose local references -/// cannot be enumerated; callers must then treat *every* id as escaping. -pub(crate) fn closure_visible_ids(stmts: &[Stmt]) -> Option> { - let mut scan = Scan { - out: HashSet::new(), - poisoned: false, - }; - scan_stmts(stmts, &mut scan); - if scan.poisoned { - None - } else { - Some(scan.out) - } -} - -fn scan_stmts(stmts: &[Stmt], scan: &mut Scan) { - for stmt in stmts { - scan_stmt(stmt, scan); - } -} - -/// Exhaustive over `Stmt` on purpose: a new statement variant that can hold an -/// expression must be routed here explicitly rather than silently hiding a -/// closure from the escape analysis. -fn scan_stmt(stmt: &Stmt, scan: &mut Scan) { - match stmt { - Stmt::Let { init, .. } => { - if let Some(e) = init { - scan_expr(e, scan); - } - } - Stmt::Expr(e) | Stmt::Throw(e) => scan_expr(e, scan), - Stmt::Return(e) => { - if let Some(e) = e { - scan_expr(e, scan); - } - } - Stmt::If { - condition, - then_branch, - else_branch, - } => { - scan_expr(condition, scan); - scan_stmts(then_branch, scan); - if let Some(eb) = else_branch { - scan_stmts(eb, scan); - } - } - Stmt::While { condition, body } => { - scan_expr(condition, scan); - scan_stmts(body, scan); - } - Stmt::DoWhile { body, condition } => { - scan_stmts(body, scan); - scan_expr(condition, scan); - } - Stmt::For { - init, - condition, - update, - body, - } => { - if let Some(init) = init { - scan_stmt(init, scan); - } - if let Some(c) = condition { - scan_expr(c, scan); - } - if let Some(u) = update { - scan_expr(u, scan); - } - scan_stmts(body, scan); - } - Stmt::Try { - body, - catch, - finally, - } => { - scan_stmts(body, scan); - if let Some(c) = catch { - scan_stmts(&c.body, scan); - } - if let Some(f) = finally { - scan_stmts(f, scan); - } - } - Stmt::Switch { - discriminant, - cases, - } => { - scan_expr(discriminant, scan); - for case in cases { - if let Some(t) = &case.test { - scan_expr(t, scan); - } - scan_stmts(&case.body, scan); - } - } - Stmt::Labeled { body, .. } => scan_stmt(body, scan), - Stmt::Break - | Stmt::Continue - | Stmt::LabeledBreak(_) - | Stmt::LabeledContinue(_) - | Stmt::PreallocateBoxes(_) - | Stmt::PreallocateTdzBoxes(_) - | Stmt::ReleaseBoxes(_) => {} - } -} - -fn scan_expr(expr: &Expr, scan: &mut Scan) { - match expr { - // Sloppy-mode `with`: the fallback LocalId is a leaf field that the - // shared free-variable walk does not report, so the analysis cannot be - // trusted on this body at all. - Expr::WithGet { .. } | Expr::WithSet { .. } => { - scan.poisoned = true; - } - Expr::Closure { - body, - captures, - mutable_captures, - .. - } => { - scan.out.extend(captures.iter().copied()); - scan.out.extend(mutable_captures.iter().copied()); - let mut refs: Vec = Vec::new(); - let mut visited: HashSet = HashSet::new(); - perry_hir::analysis::collect_local_refs_expr(expr, &mut refs, &mut visited); - scan.out.extend(refs); - // Keep descending: nested closures contribute their own explicit - // capture lists, and a `with` anywhere inside must still poison. - scan_stmts(body, scan); - return; - } - _ => {} - } - perry_hir::walker::walk_expr_children(expr, &mut |child| scan_expr(child, scan)); -} /// One `Stmt::ReleaseBoxes` naming every id in the terminal release set. /// @@ -254,98 +84,7 @@ mod tests { } } - /// A local read only by straight-line body code is not closure-visible, so - /// its cell is clearable. - #[test] - fn plain_body_local_is_not_closure_visible() { - let body = vec![ - Stmt::Let { - id: 1, - name: "v".into(), - ty: perry_hir::types::Type::Any, - mutable: true, - init: Some(Expr::Number(1.0)), - }, - Stmt::Return(Some(local_get(1))), - ]; - let ids = closure_visible_ids(&body).expect("not poisoned"); - assert!(ids.is_empty(), "no closure in the body: {:?}", ids); - } - - /// The negative case this whole module exists for: a local a closure can - /// read must be reported even when the HIR capture list is empty (codegen - /// auto-detects those captures from the body). - #[test] - fn closure_body_reference_is_visible_without_an_explicit_capture() { - let inner = vec![Stmt::Return(Some(local_get(7)))]; - let body = vec![Stmt::Return(Some(closure(inner, Vec::new())))]; - let ids = closure_visible_ids(&body).expect("not poisoned"); - assert!( - ids.contains(&7), - "auto-detected capture must escape: {ids:?}" - ); - } - - /// An explicit capture list entry counts even if the body never mentions it. - #[test] - fn explicit_capture_list_entry_is_visible() { - let body = vec![Stmt::Expr(closure(Vec::new(), vec![11]))]; - let ids = closure_visible_ids(&body).expect("not poisoned"); - assert!(ids.contains(&11), "{ids:?}"); - } - - /// Transitive: a closure nested two deep still exposes the outer local. - #[test] - fn nested_closure_reference_is_visible() { - let innermost = vec![Stmt::Return(Some(local_get(21)))]; - let middle = vec![Stmt::Return(Some(closure(innermost, Vec::new())))]; - let body = vec![Stmt::Expr(closure(middle, Vec::new()))]; - let ids = closure_visible_ids(&body).expect("not poisoned"); - assert!(ids.contains(&21), "{ids:?}"); - } - - /// Closures buried under control flow are reached (a `_ => {}` statement - /// arm here would silently make every such local look clearable). - #[test] - fn closure_under_control_flow_is_visible() { - let inner = vec![Stmt::Return(Some(local_get(31)))]; - let body = vec![Stmt::Try { - body: vec![Stmt::Switch { - discriminant: Expr::Number(0.0), - cases: vec![SwitchCase { - test: None, - body: vec![Stmt::Labeled { - label: "l".into(), - body: Box::new(Stmt::While { - condition: Expr::Bool(true), - body: vec![Stmt::Expr(closure(inner, Vec::new()))], - }), - }], - }], - }], - catch: None, - finally: None, - }]; - let ids = closure_visible_ids(&body).expect("not poisoned"); - assert!(ids.contains(&31), "{ids:?}"); - } - - /// Sloppy `with` poisons the analysis: its fallback LocalId is a leaf the - /// shared walk does not report, so nothing may be cleared. - #[test] - fn with_expression_poisons_the_analysis() { - let body = vec![Stmt::Expr(Expr::WithGet { - object: Box::new(Expr::Undefined), - property: "x".into(), - fallback: Box::new(local_get(41)), - })]; - assert!( - closure_visible_ids(&body).is_none(), - "`with` must poison the analysis" - ); - } - - // ── End-to-end: the transform actually emits (and withholds) the stores ── + // ── End-to-end: the transform emits the complete terminal release set ── fn async_module(body: Vec) -> Module { let f = Function { @@ -491,18 +230,18 @@ mod tests { ); } - /// The negative case that makes this safe: the same local, but a closure - /// escapes with it. Releasing it would be a silent use-after-clear, so the - /// transform must emit no store at all. + /// Closure-visible locals are also named at both terminal arms. Runtime + /// closure ownership defers their actual clear/publication until the last + /// capturing closure dies. #[test] - fn a_body_local_a_closure_can_see_is_never_released() { + fn a_body_local_an_escaping_closure_can_see_is_deferred_at_runtime() { let escaping = closure(vec![Stmt::Return(Some(local_get(50)))], Vec::new()); let mut m = async_module(vec![awaited_let(50), Stmt::Return(Some(escaping))]); run_async_pipeline(&mut m); assert_eq!( count_release_stores(&m.functions[0].body, 50), - 0, - "a closure-visible local must never be released:\n{:#?}", + 2, + "a closure-visible local must be handed to runtime lifetime tracking:\n{:#?}", m.functions[0].body ); } diff --git a/crates/perry-transform/src/generator/lower.rs b/crates/perry-transform/src/generator/lower.rs index 49556d3865..46facb2fbb 100644 --- a/crates/perry-transform/src/generator/lower.rs +++ b/crates/perry-transform/src/generator/lower.rs @@ -316,27 +316,6 @@ pub fn transform_generator_function_with_extra_captures( // must be boxed captures like any other cross-state local. let prologue_hoist = collect_hoisted_vars(¶m_prologue); - // #7933: every local a closure in this body can observe. Read from the - // ORIGINAL body (before hoisting/linearization move statements around) — - // those passes rewrite references but never introduce a user closure, and - // the ones that do rewrite closure bodies (`rewrite_written_captures_to_cells`, - // `snapshot_suspended_loop_captures`) only ever redirect a closure onto a - // *different* id, which leaves the pre-pass answer conservative. `None` - // means the analysis is unusable (sloppy `with`) and nothing may be - // released. See `box_release.rs` for why closure visibility is the exact - // condition. - // Consumed only by the `was_plain_async` arm below — a generator's - // `{next, return, throw}` object is user-visible, so "done" is not the end - // of observability there and nothing is ever released. - let closure_visible_before: Option> = { - closure_visible_ids(&func.body).and_then(|mut ids| { - closure_visible_ids(¶m_prologue).map(|p| { - ids.extend(p); - ids - }) - }) - }; - // #321: hoist `yield` / `yield*` that live inside a larger expression // (`return (yield 1) + (yield 2)`, call args, array/object literals, etc.) // into ordered `let __ygen_N = yield E;` temps so the linearizer below only @@ -885,67 +864,29 @@ pub fn transform_generator_function_with_extra_captures( // emit `Stmt::Throw(value)` inline in its is-error arm, saving one // closure allocation per async-fn invocation (50k/run on the // promise_all_chains kernel). - // #7933: the activation's boxed body locals that no closure can - // observe, released (set to `undefined`) at the state machine's - // terminal states. Second, independent scan of the POST-linearization - // bodies, unioned with the pre-pass one: an id either scan calls - // closure-visible is kept. + // Every cell in the activation is named at its terminal states. The + // runtime publishes unobserved cells immediately; closure-visible + // cells remain registered until the GC proves the final capturing + // closure dead, so an escaped closure remains fully observable. let release_ids: Vec = { - let post_visible = closure_visible_ids(&next_resume_body).and_then(|mut ids| { - let routes_ok = - catches - .iter() - .all(|route| match closure_visible_ids(&route.body) { - Some(r) => { - ids.extend(r); - true - } - None => false, - }); - if routes_ok { - Some(ids) - } else { - None - } - }); - match (&closure_visible_before, &post_visible) { - (Some(before), Some(after)) => { - let machinery = [state_id, done_id, executing_id].into_iter().chain( - if has_yielding_finally { - vec![pending_type_id, pending_value_id] - } else { - Vec::new() - }, - ); - let mut ids: Vec = hoisted - .iter() - .map(|(id, _, _)| *id) - .chain(extra_local_ids.iter().copied()) - // `__gen_sent` holds the value the last `await` - // delivered — a first-class retainer, and re-entry - // overwrites it before any read. - .chain(std::iter::once(sent_id)) - // The state-machine control locals release too, now - // that a release parks the cell instead of writing - // `undefined` (#7933 follow-up). A stray duplicate - // resume stays on today's exact path via the PARKED - // VALUES: generated code reads the control cells with - // raw loads, `js_bool_box_release` parks - // `__gen_done`-shaped cells as `true` (the terminal - // short-circuit), and `js_i32_box_release` parks - // `__gen_state`-shaped cells as `-1` (matches no - // dispatch case, no catch route, no completion-check - // state). User closures cannot name these locals, so - // the visibility filter below is pure defense. - .chain(machinery) - .filter(|id| !before.contains(id) && !after.contains(id)) - .collect(); - ids.sort(); - ids.dedup(); - ids - } - _ => Vec::new(), - } + let machinery = + [state_id, done_id, executing_id] + .into_iter() + .chain(if has_yielding_finally { + vec![pending_type_id, pending_value_id] + } else { + Vec::new() + }); + let mut ids: Vec = hoisted + .iter() + .map(|(id, _, _)| *id) + .chain(extra_local_ids.iter().copied()) + .chain(std::iter::once(sent_id)) + .chain(machinery) + .collect(); + ids.sort(); + ids.dedup(); + ids }; let throw_routes_for_step = if catches.is_empty() { None diff --git a/crates/perry-transform/src/generator/lower/async_step.rs b/crates/perry-transform/src/generator/lower/async_step.rs index cfbd5a10b6..9dbc446b13 100644 --- a/crates/perry-transform/src/generator/lower/async_step.rs +++ b/crates/perry-transform/src/generator/lower/async_step.rs @@ -88,10 +88,9 @@ pub fn build_async_step_driver_direct( captures_new_target: bool, enclosing_class: Option, is_strict: bool, - // #7933: boxed body locals of this activation that no closure can observe. - // Released (`js_box_set(cell, undefined)`) at the step machine's terminal - // states so a completed activation stops retaining its locals through the - // never-freed `BOX_REGISTRY`. See `generator/box_release.rs`. + // Boxed body and control locals in this activation. Named at terminal + // states; runtime closure lifetime tracking defers publication for any + // cell an escaped closure can still observe. release_ids: &[LocalId], ) -> Vec { // When `throw_closure_expr` is None, the function had no awaiting diff --git a/crates/perry-transform/src/generator/mod.rs b/crates/perry-transform/src/generator/mod.rs index 479b567c71..6432b496ae 100644 --- a/crates/perry-transform/src/generator/mod.rs +++ b/crates/perry-transform/src/generator/mod.rs @@ -25,7 +25,7 @@ mod rewrite_returns; // Explicit named re-exports so siblings can reach each other via // `use super::*;`. Globs don't propagate transitively, so spell every // cross-module symbol here. -pub(crate) use box_release::{build_box_release_stmts, closure_visible_ids}; +pub(crate) use box_release::build_box_release_stmts; pub(crate) use break_continue::{ body_contains_yield, collect_hoisted_vars, fix_break_continue_sentinels, fix_break_continue_sentinels_in_catches, fix_break_continue_sentinels_in_stmts, diff --git a/scripts/addr_class_allowlist.txt b/scripts/addr_class_allowlist.txt index cbd021ed22..59dc50a577 100644 --- a/scripts/addr_class_allowlist.txt +++ b/scripts/addr_class_allowlist.txt @@ -167,3 +167,4 @@ crates/perry-ext-ratelimit/src/lib.rs | (obj as usize) >= 0x100000 | #7272: same crates/perry-ext-slugify/src/lib.rs | (obj as usize) < 0x100000 | #7272: same handle-vs-pointer guard before an ObjectHeader read crates/perry-runtime/src/arena/page_meta.rs | let header = addr as *const GcHeader; | promoted-page-run expansion: `addr` starts at a `first_header` recorded by arena/promote.rs's linear block iteration (its grandfathered sibling entry above) and advances by `GcHeader::size` from there, so every address is a block-interior header, never a NaN-box payload; the parse stops at the first implausible size exactly as the arena walkers do crates/perry-runtime/src/arena/tests_promoted_runs.rs | * | arena promoted-run tests: header addresses are offsets into a buffer the test itself allocated and initialised, never NaN-box payloads -- same discipline as the arena/tests.rs entry above +crates/perry-runtime/src/box/release_tests.rs | * | async-box release tests: the closure whose GcHeader is read is allocated by the test itself, and the test needs a MUTABLE header to toggle GC_FLAG_MARKED and restore it -- try_read_gc_header yields a shared ref, so it cannot express this. Same discipline as the arena/tests_promoted_runs.rs entry above. diff --git a/scripts/check_runtime_symbols.sh b/scripts/check_runtime_symbols.sh index 438f571c09..abfae684f9 100755 --- a/scripts/check_runtime_symbols.sh +++ b/scripts/check_runtime_symbols.sh @@ -51,6 +51,7 @@ SENTINELS=( js_box_set_bits js_closure_get_capture_bits js_closure_set_capture_bits + js_closure_set_box_capture_ptr js_object_get_field_by_property_id_f64 js_object_set_field_by_property_id js_native_call_method_by_id diff --git a/scripts/gc_root_dominance_check.py b/scripts/gc_root_dominance_check.py index 94e505455f..9a18100f1c 100755 --- a/scripts/gc_root_dominance_check.py +++ b/scripts/gc_root_dominance_check.py @@ -479,6 +479,7 @@ def build_cfg(f): "llvm.lifetime.start.p0", "llvm.lifetime.end.p0", # verified non-allocating bookkeeping stores/reads (perry-runtime) "js_closure_set_capture_bits", # closure/alloc.rs:477 raw slot write + layout note + "js_closure_set_box_capture_ptr", # declared box edge + same raw slot write "js_closure_get_capture_bits", # closure/alloc.rs:463 raw slot read "js_closure_set_capture_ptr", "js_closure_get_capture_ptr", "js_box_set_bits", "js_box_get_bits", # box.rs:317 raw cell write @@ -2693,9 +2694,18 @@ def _probe_boxes_outside_the_gc_heap(): "zero-reference transition") publish = rust_fn_body("crates/perry-runtime/src/box.rs", "publish_async_activation_cells") - if publish is None or "push_free_cell" not in publish: + publish_cell = rust_fn_body("crates/perry-runtime/src/box.rs", + "publish_box_cell") + if publish is None or "publish_box_cell" not in publish \ + or publish_cell is None or "push_free_cell" not in publish_cell: return (False, "the activation zero-reference publisher no longer " "feeds the intrusive free pools") + capture_zero = rust_fn_body("crates/perry-runtime/src/box.rs", + "box_capture_count_reached_zero") + if capture_zero is None or "ASYNC_RELEASE_DRAINED" not in capture_zero \ + or "publish_box_cell" not in capture_zero: + return (False, "closure-death publication is no longer gated on an " + "already-drained async activation") try: with open("crates/perry-runtime/src/promise/async_step.rs", encoding="utf-8", errors="replace") as fh: