From e06bb82af82a0e14e424fa81338d74f882b74354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 4 Sep 2026 13:30:29 +0000 Subject: [PATCH 1/2] fix(gc): admit array-growth forwarding stubs to the budgeted-cycle classifier (#9717) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `#private` array pushed past its inline capacity leaves a permanent forwarding stub at the pre-grow address, and the reference pointing at it is never rewritten (#6228/#233), so a live slot can keep naming the stub. A synchronous full trace handles this: its exact census (`record_arena_header`) admits every arena object, stubs included, so `mark_field_into_worklist` marks the stub and `trace_one_worklist_header` follows it to the live array. A budgeted full trace — what the idle-time reducer (`PERRY_GC_IDLE_RECLAIM`) runs when a server goes quiet — resolves membership through the page-metadata classifier instead. `classifier_valid_object_start` rejected every FORWARDED header (a dead metadata key's recycled bytes can set the bit, #8040), so the field->stub edge was dropped: the stub was never marked, the FORWARDED-follow never ran, and the array reachable only through it was swept. The private field then resolved to reused memory as an empty array, so every hono route `match()` returned 404 for the life of the process — but only when the first request arrived ~10-20s after startup while background work allocated. The classifier is documented as a census superset; for growth stubs it was not. It now admits a plausible forwarded arena stub (`GC_FLAG_ARENA` set, valid obj_type/size), the shape a real growth stub has. The forwarding target is still validated in the follow, so a garbage target stops the walk. A `PERRY_GC_DIAG` counter (`forwarded_stub_recoveries=`) reports recoveries. Regression: gc::tests::forwarded_stub_membership plants the edge, asserts the pre-fix gate would have rejected the stub, drives a budgeted full cycle, and checks the stub-reached array survives; a synchronous control keeps it without recovery. The budgeted test fails without the fix and passes with it. Claude-Session: https://claude.ai/code/session_01GkugRUwRCCjfYYNfzyyvQv --- ...732-idle-reclaim-growth-stub-membership.md | 7 + crates/perry-runtime/src/gc/barrier/mod.rs | 38 +++++ crates/perry-runtime/src/gc/mod.rs | 3 +- .../src/gc/tests/forwarded_stub_membership.rs | 159 ++++++++++++++++++ crates/perry-runtime/src/gc/tests/mod.rs | 1 + crates/perry-runtime/src/gc/trace.rs | 49 +++++- scripts/gc_runtime_root_holders.json | 6 + 7 files changed, 260 insertions(+), 3 deletions(-) create mode 100644 changelog.d/9732-idle-reclaim-growth-stub-membership.md create mode 100644 crates/perry-runtime/src/gc/tests/forwarded_stub_membership.rs diff --git a/changelog.d/9732-idle-reclaim-growth-stub-membership.md b/changelog.d/9732-idle-reclaim-growth-stub-membership.md new file mode 100644 index 0000000000..0a9bcc4d1a --- /dev/null +++ b/changelog.d/9732-idle-reclaim-growth-stub-membership.md @@ -0,0 +1,7 @@ +**Idle-time (budgeted) collections no longer sweep an array a live field reaches only through an array-growth forwarding stub (#9717).** A `#private` array pushed past its inline capacity leaves a *permanent* forwarding stub at the pre-grow address, and the reference pointing at it is never rewritten (#6228/#233), so a live slot — hono `SmartRouter`'s `#routes`, in the report — keeps naming the stub. A **synchronous** full trace is fine: its exact census (`ValidPointerSetBuilder::record_arena_header`) admits every arena object, stubs included, so `mark_field_into_worklist` marks the stub and `trace_one_worklist_header` follows it to the live array. + +A **budgeted** full trace — the one the idle-time reducer (`PERRY_GC_IDLE_RECLAIM`) runs when a server goes quiet between requests — resolves membership through the page-metadata classifier instead of a census. `classifier_valid_object_start` rejected every `GC_FLAG_FORWARDED` header by design (a dead metadata key's recycled bytes can set that bit, #8040), so the field→stub edge was silently dropped: the stub was never marked, the FORWARDED-follow never ran, and the array reachable *only* through the stub was swept. The field then resolved to reused memory — an empty array — and every route `match()` returned 404 for the life of the process. It reproduced only when the first request arrived ~10–20 s after startup while background work allocated: an early request built the router before any idle collection ran. + +**Fix.** The classifier is documented as a census *superset*; for growth stubs it was not. `classifier_valid_object_start` now admits a plausible forwarded arena stub (`GC_FLAG_ARENA` set, valid `obj_type`/size — the shape a real growth stub has, which separates it from off-heap bytes that coincidentally set the bit). The forwarding *target* is still validated where it always was, in `trace_one_worklist_header`'s follow, so a garbage target simply stops the walk. A `PERRY_GC_DIAG` counter (`forwarded_stub_recoveries=` on the `[gc-incremental]` line) reports how many such stubs a budgeted cycle recovered; it stays zero on a run with no such edge. + +Regression coverage: `gc::tests::forwarded_stub_membership` plants the edge, asserts the pre-fix census-superset gate would have rejected the stub, and drives a budgeted full cycle to completion — the array reached only through the stub survives with its contents intact, and a synchronous full cycle keeps it without needing the recovery path. diff --git a/crates/perry-runtime/src/gc/barrier/mod.rs b/crates/perry-runtime/src/gc/barrier/mod.rs index 34f058e629..35c37c0454 100644 --- a/crates/perry-runtime/src/gc/barrier/mod.rs +++ b/crates/perry-runtime/src/gc/barrier/mod.rs @@ -960,6 +960,44 @@ pub(super) unsafe fn plausible_arena_user_ptr_header( } } +/// A plausible ARENA object whose header carries `GC_FLAG_FORWARDED` — i.e. an +/// array-growth forwarding stub (#6228: growth installs a PERMANENT stub at the +/// pre-grow address, and a live slot can keep pointing directly at it because +/// references are never rewritten). +/// +/// This is [`plausible_arena_user_ptr_header`] with the FORWARDED test +/// inverted: same alignment / `obj_type` / size / `GC_FLAG_ARENA` gate, but the +/// header MUST be forwarded. `GC_FLAG_ARENA` is what separates a genuine stub +/// from an off-heap region whose bytes coincidentally set FORWARDED (#8040) — +/// the census (`ValidPointerSetBuilder::record_arena_header`) admits exactly +/// these real arena allocations, so the classifier must too (#9717). The +/// forwarding TARGET is deliberately NOT validated here: the caller follows it +/// through `trace_one_worklist_header`, which re-checks membership before +/// marking it, so a garbage target simply stops the walk. +#[inline] +pub(super) unsafe fn plausible_forwarded_arena_stub( + header: *mut GcHeader, +) -> Option<*mut GcHeader> { + if header.is_null() { + return None; + } + if !(header as usize).is_multiple_of(std::mem::align_of::()) { + return None; + } + let obj_type = (*header).obj_type; + let size = (*header).size as usize; + if gc_type_info(obj_type).is_none() + || size < GC_HEADER_SIZE + || size as u64 > (1u64 << 34) + || (*header).gc_flags & GC_FLAG_ARENA == 0 + || (*header).gc_flags & GC_FLAG_FORWARDED == 0 + { + None + } else { + Some(header) + } +} + pub(super) fn current_heap_header_for_user_ptr( user_ptr: usize, valid_ptrs: Option<&ValidPointerSet>, diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 9041e56360..d21179a425 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -1355,7 +1355,7 @@ fn emit_incremental_liveness_diag() { safepoints_blocked(in_alloc={blocked_alloc} unsafe_zone={blocked_unsafe_zone} \ root_lock={blocked_root_lock}) \ copying_minors={} loop_polls={} poll_arm_events={} \ - poll_armed_at_exit={}", + poll_armed_at_exit={} forwarded_stub_recoveries={}", instruments::incremental_cycle_starts(), instruments::incremental_steps(), instruments::incremental_completions(), @@ -1368,6 +1368,7 @@ fn emit_incremental_liveness_diag() { instruments::loop_polls_reached(), poll_arm::poll_arm_events(), poll_arm::poll_armed_count(), + trace::forwarded_stub_membership_recoveries(), ); idle_reclaim::emit_diag(); idle_compact::emit_diag(); diff --git a/crates/perry-runtime/src/gc/tests/forwarded_stub_membership.rs b/crates/perry-runtime/src/gc/tests/forwarded_stub_membership.rs new file mode 100644 index 0000000000..d40bfcb2cf --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/forwarded_stub_membership.rs @@ -0,0 +1,159 @@ +//! #9717 — a budgeted (classifier-mode) full trace must keep alive an array a +//! live heap slot reaches only through an array-growth forwarding stub. +//! +//! Array growth leaves a PERMANENT forwarding stub at the pre-grow address and +//! never rewrites the references pointing at it (#6228 / #233), so a live field +//! — hono's `SmartRouter.#routes`, in the reported case — can keep naming the +//! stub. A SYNCHRONOUS full trace is fine: its census (`record_arena_header`) +//! admits every arena object, stubs included, so `mark_field_into_worklist` +//! marks the stub and `trace_one_worklist_header` follows it to the live array. +//! +//! A BUDGETED full trace (what the idle-time reducer runs) resolves membership +//! through `classifier_valid_object_start` instead, and that gate rejected +//! FORWARDED headers by design (a dead metadata key's recycled bytes can set +//! the bit, #8040). So the field -> stub edge was dropped: the stub was never +//! marked, the FORWARDED-follow never ran, and the array reachable ONLY through +//! it was swept — the private-field array that "is still an array, but empty" +//! ten seconds into a loaded server. +//! +//! The classifier is documented as a census SUPERSET; for stubs it was not. +//! These tests plant the exact edge and assert the budgeted trace keeps the +//! target alive. Each first asserts the PREMISE — the pre-#9717 gate rejects +//! the stub — so a green run says the recovery works, not that nothing was +//! tried. + +use super::super::*; +use super::support::*; + +fn reset_old_reclaim_pressure() { + let old_in_use = crate::arena::old_gen_in_use_bytes(); + GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|bytes| bytes.set(old_in_use)); + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); +} + +/// Grow a fresh array past its inline capacity and return `(stub, grown, first +/// child)`: `stub` is the pre-grow head, now a forwarding stub; `grown` is the +/// current head; `first_child` is the value at index 0, reachable only through +/// the array. +fn grow_array_leaving_stub() -> (usize, usize, usize) { + let stub = crate::array::js_array_alloc(0); + let mut current = stub; + let mut first_child = 0usize; + for i in 0..64 { + let child = young_leaf(); + if i == 0 { + first_child = child; + } + current = crate::array::js_array_push_f64(current, f64::from_bits(ptr_bits(child))); + } + assert_ne!( + stub, current, + "setup must grow the array and leave a forwarding stub" + ); + unsafe { + let stub_hdr = header_from_user_ptr(stub as *const u8) as *mut GcHeader; + assert_ne!( + (*stub_hdr).gc_flags & GC_FLAG_FORWARDED, + 0, + "the pre-grow head must be a forwarding stub" + ); + // THE PREMISE: the census-superset gate a budgeted classifier used + // rejects this stub, so a trace consulting only it drops the edge. + assert!( + crate::gc::barrier::plausible_arena_user_ptr_header(stub_hdr).is_none(), + "a forwarded stub must fail the pre-#9717 gate, or recovery proves nothing" + ); + } + (stub as usize, current as usize, first_child) +} + +#[test] +fn a_budgeted_full_cycle_keeps_an_array_a_live_field_reaches_through_a_growth_stub() { + let _guard = CopyingNurseryTestGuard::new(2); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + reset_old_reclaim_pressure(); + reset_global_roots(); + let _root_reset = ShadowAndGlobalRootResetGuard; + + let (stub, _grown, first_child) = grow_array_leaving_stub(); + + // The "#routes field": a heap holder whose slot points DIRECTLY at the + // stub (references are never rewritten), rooted so the trace reaches it. + let holder = crate::array::js_array_alloc(1); + let holder = crate::array::js_array_push_f64(holder, f64::from_bits(ptr_bits(stub))); + js_shadow_slot_set(0, ptr_bits(holder as usize)); + + let recoveries_before = crate::gc::forwarded_stub_membership_recoveries(); + + // Drive a budgeted FULL cycle — the idle reclaimer's path — to completion. + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(true)); + let mut result = JsGcStepResult::default(); + assert_eq!( + js_gc_step_work_units(1, &mut result), + JS_GC_STEP_STATUS_ACTIVE + ); + assert_eq!(result.collection_kind, GcCollectionKind::Full.ffi_code()); + let completed = complete_budgeted_gc_cycle(); + assert_eq!(completed.status, JS_GC_STEP_STATUS_COMPLETED); + + assert!( + crate::gc::forwarded_stub_membership_recoveries() > recoveries_before, + "#9717: the budgeted trace must recover the growth stub the live field \ + points at; without it the field -> stub edge is dropped and the array \ + reachable only through it is swept" + ); + + // The array survives with its contents: resolve holder[0] -> stub -> grown + // and read the first element back through the stub (clean_arr_ptr follows + // the forward). A swept target would read as an empty/undefined array here. + let holder_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + let stub_bits = + crate::array::js_array_get_f64(holder_after as *const crate::array::ArrayHeader, 0) + .to_bits(); + let stub_after = (stub_bits & POINTER_MASK) as *const crate::array::ArrayHeader; + let child_bits = crate::array::js_array_get_f64(stub_after, 0).to_bits(); + let child_after = (child_bits & POINTER_MASK) as usize; + assert_eq!( + child_after, first_child, + "the array element reachable only through the field -> stub edge must \ + survive the budgeted full cycle unchanged" + ); +} + +/// Negative control: a SYNCHRONOUS full mark-sweep already handles the same +/// edge through the exact census (`record_arena_header`), so it needs no +/// recovery. This keeps the assertion above a statement about the BUDGETED +/// path specifically, not about stubs in general. +#[test] +fn a_synchronous_full_cycle_keeps_the_same_array_without_needing_recovery() { + let _guard = CopyingNurseryTestGuard::new(2); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + reset_global_roots(); + let _root_reset = ShadowAndGlobalRootResetGuard; + + let (stub, _grown, first_child) = grow_array_leaving_stub(); + let holder = crate::array::js_array_alloc(1); + let holder = crate::array::js_array_push_f64(holder, f64::from_bits(ptr_bits(stub))); + js_shadow_slot_set(0, ptr_bits(holder as usize)); + + let recoveries_before = crate::gc::forwarded_stub_membership_recoveries(); + gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Manual)); + assert_eq!( + crate::gc::forwarded_stub_membership_recoveries(), + recoveries_before, + "the synchronous census admits stubs directly; the classifier recovery \ + path must not run on this path" + ); + + let holder_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + let stub_bits = + crate::array::js_array_get_f64(holder_after as *const crate::array::ArrayHeader, 0) + .to_bits(); + let stub_after = (stub_bits & POINTER_MASK) as *const crate::array::ArrayHeader; + let child_bits = crate::array::js_array_get_f64(stub_after, 0).to_bits(); + assert_eq!( + (child_bits & POINTER_MASK) as usize, + first_child, + "a synchronous full cycle keeps the stub-reached array (unchanged behaviour)" + ); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 82a2f95b78..7b8af45bb7 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -19,6 +19,7 @@ mod dirty_page_cache; mod env_knob_parse; mod error_side_tables; mod evacuation; +mod forwarded_stub_membership; mod forwarding_target_validation; mod fromspace_protect; mod fromspace_scan; diff --git a/crates/perry-runtime/src/gc/trace.rs b/crates/perry-runtime/src/gc/trace.rs index 8a1606c121..fb350daa45 100644 --- a/crates/perry-runtime/src/gc/trace.rs +++ b/crates/perry-runtime/src/gc/trace.rs @@ -9,6 +9,29 @@ thread_local! { const { std::cell::Cell::new(false) }; } +crate::perry_thread_local! { + /// #9717: array-growth forwarding stubs the classifier admitted into a + /// budgeted-cycle valid-pointer set that `plausible_arena_user_ptr_header` + /// would have rejected. A non-zero count is a POSITIVE report that a live + /// slot pointed at a growth stub during a budgeted full trace — the exact + /// edge whose loss swept a private-field array on the idle reclaim. Zero on + /// a run with no such edge, so it never perturbs a log a gate parses. + static FORWARDED_STUB_MEMBERSHIP_RECOVERIES: std::cell::Cell = + const { std::cell::Cell::new(0) }; +} + +#[cold] +fn note_forwarded_stub_membership_recovery() { + FORWARDED_STUB_MEMBERSHIP_RECOVERIES.with(|c| c.set(c.get().saturating_add(1))); +} + +/// Running count of array-growth forwarding stubs the classifier recovered into +/// a budgeted valid-pointer set (#9717). A test that plants a stub-only- +/// referenced array can assert this moved. +pub(crate) fn forwarded_stub_membership_recoveries() -> u64 { + FORWARDED_STUB_MEMBERSHIP_RECOVERIES.with(std::cell::Cell::get) +} + /// #6179 membership classifier: is `addr` a plausible live GC object start? /// UNION of the two backends — exact membership in the malloc registry OR a /// plausible arena header on an arena-classified page — deliberately NOT the @@ -25,10 +48,32 @@ pub(super) fn classifier_valid_object_start(addr: usize) -> bool { if super::gc_malloc_header_is_tracked(header) { return true; } - !matches!( + if matches!( crate::arena::classify_heap_generation(addr), crate::arena::HeapGeneration::Unknown - ) && unsafe { super::barrier::plausible_arena_user_ptr_header(header).is_some() } + ) { + return false; + } + if unsafe { super::barrier::plausible_arena_user_ptr_header(header).is_some() } { + return true; + } + // #9717: an array-growth forwarding stub is a real censused arena object a + // live slot can still point directly at (references are never rewritten, + // #6228). The census path admits it (record_arena_header pushes every arena + // object), so this classifier -- which contains() uses for a budgeted, + // non-moving cycle and which must be a census SUPERSET -- has to admit it + // too. plausible_arena_user_ptr_header rejects FORWARDED headers by design + // (a metadata key whose object may have died and been recycled with the bit + // set), so the stub was silently dropped: mark_field_into_worklist failed + // membership, never marked the stub, and the FORWARDED-follow in + // trace_one_worklist_header never ran -- so the live post-growth array, + // reachable only through the field to stub edge, was swept. That is the + // idle-time (budgeted full) reclaim turning a private-field array empty. + if unsafe { super::barrier::plausible_forwarded_arena_stub(header).is_some() } { + note_forwarded_stub_membership_recovery(); + return true; + } + false } /// #6179: differential-verification mode for the page-metadata classifier. diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index f50d35fac8..4585949407 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -1953,6 +1953,12 @@ "name": "NEXT_TOKEN", "verdict": "not_a_gc_pointer", "why": "Monotonic AtomicU64 that mints write-token ids for TOKENS. A counter: incremented and used as a map key, never dereferenced and never derived from an address." + }, + { + "file": "crates/perry-runtime/src/gc/trace.rs", + "name": "FORWARDED_STUB_MEMBERSHIP_RECOVERIES", + "verdict": "not_a_gc_pointer", + "why": "#9717 diagnostic counter: a thread_local Cell incremented when a budgeted-cycle classifier admits an array-growth forwarding stub, read by forwarded_stub_membership_recoveries() for the [gc-incremental] diag line. Holds a count, never a JSValue or heap pointer, so there is nothing for a scanner to visit." } ], "_FRONTIER_README": "Identity-pinned debt ratchet over new perry-ui* candidates and otherwise-unclassified core perry_thread_local! declarations (see the census docstring, \u201cThe identity-pinned frontier\u201d). A new uncovered holder fails until it is scanned, receives a researched holders verdict, or is deliberately pinned as debt. Moving a researched false positive to holders graduates it from this list. A fixed or classified holder makes its old frontier pin stale, so the receipt must be deleted.", From fbb7e32c2fd58ae995728e1d6096a7b29d4c7b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 4 Sep 2026 14:11:52 +0000 Subject: [PATCH 2/2] chore(docs): regenerate API manifest docs for bun.connect/bun.listen Pre-existing drift on main: the runtime exports `bun.connect` and `bun.listen` (manifest 2089->2091 entries) but `docs/api/perry.d.ts` and `docs/src/api/reference.md` were not regenerated, so the `check` job's API-docs drift gate is red for every PR branched from main. `scripts/regen_api_docs.sh` produces exactly this diff (deterministic; unrelated to the #9717 GC fix in this PR). Committing the generated artifacts as the gate instructs. Claude-Session: https://claude.ai/code/session_01GkugRUwRCCjfYYNfzyyvQv --- docs/api/perry.d.ts | 6 +++++- docs/src/api/reference.md | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/api/perry.d.ts b/docs/api/perry.d.ts index c8c478c975..a2a6b00868 100644 --- a/docs/api/perry.d.ts +++ b/docs/api/perry.d.ts @@ -1,6 +1,6 @@ // Auto-generated from Perry's API manifest (#465). Do not edit by hand. // Source: perry-api-manifest::API_MANIFEST -// Coverage: 2089 entries across 136 modules +// Coverage: 2091 entries across 136 modules type PerryI8 = number & { readonly __perryI8?: never }; type PerryI16 = number & { readonly __perryI16?: never }; @@ -375,6 +375,8 @@ declare module "bun" { /** stdlib */ export function build(...args: any[]): any; /** stdlib */ + export function connect(...args: any[]): any; + /** stdlib */ export function deepEquals(...args: any[]): any; /** stdlib */ export function file(...args: any[]): any; @@ -387,6 +389,8 @@ declare module "bun" { /** stdlib */ export function hash(...args: any[]): any; /** stdlib */ + export function listen(...args: any[]): any; + /** stdlib */ export function pathToFileURL(...args: any[]): any; /** stdlib */ export function serve(options: any): any; diff --git a/docs/src/api/reference.md b/docs/src/api/reference.md index 552694501d..dc3fde7ca7 100644 --- a/docs/src/api/reference.md +++ b/docs/src/api/reference.md @@ -2,7 +2,7 @@ This page is auto-generated from Perry's compile-time API manifest (`perry-api-manifest::API_MANIFEST`). It is the source of truth for what `perry compile` accepts; references to symbols not listed here produce `R005 UnimplementedApi` (issue #463). Stubs (#464) are flagged ⚠ — they link cleanly but no-op at runtime on the chosen target. -Total: 3046 entries across 138 modules. +Total: 3048 entries across 138 modules. ## Modules @@ -429,12 +429,14 @@ Total: 3046 entries across 138 modules. - `Terminal` — module - `Transpiler` — module - `build` — module +- `connect` — module - `deepEquals` — module - `file` — module - `fileURLToPath` — module - `gc` — module - `generateHeapSnapshot` — module - `hash` — module +- `listen` — module - `pathToFileURL` — module - `scan` — instance *(class: `Transpiler`)* - `scanImports` — instance *(class: `Transpiler`)*