Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog.d/9732-idle-reclaim-growth-stub-membership.md
Original file line number Diff line number Diff line change
@@ -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.
38 changes: 38 additions & 0 deletions crates/perry-runtime/src/gc/barrier/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<GcHeader>()) {
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>,
Expand Down
3 changes: 2 additions & 1 deletion crates/perry-runtime/src/gc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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();
Expand Down
159 changes: 159 additions & 0 deletions crates/perry-runtime/src/gc/tests/forwarded_stub_membership.rs
Original file line number Diff line number Diff line change
@@ -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)"
);
}
1 change: 1 addition & 0 deletions crates/perry-runtime/src/gc/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
49 changes: 47 additions & 2 deletions crates/perry-runtime/src/gc/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u64> =
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
Expand All @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion docs/api/perry.d.ts
Original file line number Diff line number Diff line change
@@ -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 };
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion docs/src/api/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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`)*
Expand Down
6 changes: 6 additions & 0 deletions scripts/gc_runtime_root_holders.json
Original file line number Diff line number Diff line change
Expand Up @@ -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<u64> 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.",
Expand Down
Loading