Skip to content

fix(gc): root the callback and every staged argument of the timer family #7230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
proggeramlug merged 4 commits into from
Aug 2, 2026
Merged

fix(gc): root the callback and every staged argument of the timer family #7230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
22018e0
fix(gc): root the callback and every staged argument of the timer family
Aug 2, 2026
cc91790
docs: changelog fragment for #7230
Aug 2, 2026
12bf05b
fix(gc): scan setInterval's trailing arguments
Aug 2, 2026
a8a6d3b
docs: changelog for the setInterval args scanner fix
Aug 2, 2026
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
78 changes: 78 additions & 0 deletions changelog.d/7230-timer-arg-staging-rooting.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
### Fixed

- **GC: the timer family's callback and trailing arguments are rooted across their own argument list (#7210).**
`setTimeout(cb, 0, {…}, churn())` had two unrooted windows, and the callback's
was a live deterministic crash on `main`, not the staleness the issue predicted.

`cb_box` was lowered first and read at `js_timer_validate_callback` — after the
delay's `lower_expr` and after every trailing argument's. A freshly-allocated
closure therefore sat in a bare SSA register across a user call carrying loop
back-edge polls, and the moving minor inside it left the register naming
from-space: `TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be
of type function. Received an instance of Object`, 3/3 under
`PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_HEAP_LIMIT=8` and identically on the
`evac_minor` arm.
Comment on lines +12 to +14

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Reconcile the base-failure repetition count with the corpus entry.

This fragment records the base failure as "3/3" under PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_HEAP_LIMIT=8 (Line 12) and as "red 3/3 at base" (Line 39). The corpus entry in test-parity/gc_repsel_corpus.txt (Line 271) records "5/5" for the same configuration. Align both records on the count that was actually measured.

Also applies to: 38-40

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@changelog.d/7230-timer-arg-staging-rooting.md` around lines 12 - 14, Update
the base-failure repetition counts in the changelog entries for the
PERRY_GC_MOVING_LOOP_POLLS=1 and PERRY_GC_HEAP_LIMIT=8 configuration, including
the “red … at base” record, to match the actually measured count recorded in
test-parity/gc_repsel_corpus.txt. Keep the moving-loop and evac_minor results
unchanged.

Source: Path instructions


The trailing-argument staging buffer is the second window and is worse in kind:
argument *i* sits in a bare `alloca_entry_array`, storage the precise root walk
never visits, while argument *i+1* is lowered. Nothing anywhere refers to the
object at that moment, so it is a premature **sweep**, not a stale address.

The `timers`-namespace forms carried a third: `cb_handle` was `unbox_to_i64`'d
— a raw heap address, not even NaN-boxed — above the trailing arguments.

All three close with one change: lower the whole argument list through
`expr::temp_root::lower_exprs_rooted`, then fill the buffer in a second,
lowering-free pass, and release the guard only after the consuming call (which
reads the buffer). Cost is zero when nothing in the list can collect
(`OperandProtection::Reuse`) — `setTimeout(fn, 0, someLocal)` emits
byte-identical IR. Covers global `setTimeout`/`setInterval`/`setImmediate`,
their `timers`-namespace siblings, and `process.nextTick`.

`crates/perry-codegen/src/lower_call/extern_func.rs` crossed the 2000-line cap,
so the timer arms moved to a new `lower_call/extern_timers.rs` — a pure
mechanical move, dispatched immediately above the match so arm order is
unchanged. Timer routing after the split was re-verified against node 26.5.1 on
a program exercising every moved arm.

Witness `test-files/test_gap_gc_staging_args_rooting.ts`, registered in
`test-parity/gc_repsel_corpus.txt`: red 3/3 at base, `bad 0` 5/5 after, clean
3/3 on the shipped default, byte-exact against node.

### Notes

- The `#7210` triage that motivated this change found that the **66**
moving-reachable `gc_root_dominance_check.py --unrooted-allocas` reports are
**all false positives**, and says so rather than adding roots: 64 are the
`@perry_class_keys_*` pointer cache, whose array is allocated by
`js_array_alloc_with_length_longlived` into the old arena and is therefore
moved only by old-page defrag — which is **off by default** since #6206
(`PERRY_GC_OLD_DEFRAG=1` opt-in); the other 2 are `js_box_alloc_bits` results,
which come from `std::alloc::alloc`, are never freed and are never relocated
(`scan_box_roots_mut` rewrites the JSValue *inside* the box, not its address).
The checker's `_is_heap_source` conflates "a location the collector rewrites"
with "an object the collector can move"; the sites it names are the former and
not the latter.

- **GC: `setInterval`'s trailing arguments are now scanned (#7210, runtime half).**
`scan_timer_roots_mut` walked `timer.args` for `CALLBACK_TIMERS` and for both
`MOCK_TIMERS` lists, and never for `INTERVAL_TIMERS`; the incremental twin
`scan_interval_timers_step` had the same hole, and cycle-based collections run
**only** the step scanner. So `setInterval(fn, delay, { … })` left the object
in a table nothing scanned — swept at the first collection, then handed to the
callback as a dangling pointer on the next tick.

This is a different failure class from a stale register: it does not need a
collection to land in a narrow window, it goes wrong at collection #0 and
stays wrong, and no static IR checker can see it (the table is not in the
emitted IR). A partially-correct scanner is worse than an absent one, because
it reads as covered.

It is also the other half of the codegen fix above — rooting an argument
across its own lowering buys nothing if the table it then lands in is not a
root. Witness `test-files/test_gap_gc_interval_args_rooting.ts`, registered in
the corpus: `BAD interval.a`/`BAD interval.b` from tick 1, 3/3 at base under
`PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_HEAP_LIMIT=8`; `bad 0` 5/5 after; clean
3/3 on the shipped default; byte-exact against node 26.5.1.

Comment on lines +73 to +77

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Reconcile the interval-witness tick count and failing variable with the corpus entry.

This fragment states BAD interval.a/BAD interval.b "from tick 1" and "3/3 at base." test-parity/gc_repsel_corpus.txt Line 359 records only BAD interval.a "from tick 2 onward," with no repetition count given. The test file's own header comment in test-files/test_gap_gc_interval_args_rooting.ts states the first tick can precede the first collection, which supports "tick 2" as the correct onset rather than "tick 1." Align this fragment's tick number, failing-variable claim, and repetition count with the actually measured corpus record.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@changelog.d/7230-timer-arg-staging-rooting.md` around lines 73 - 77, Update
the changelog fragment’s interval-witness result to match the corpus entry:
report BAD interval.a beginning from tick 2 onward, remove the unsupported BAD
interval.b claim, and omit the unrecorded 3/3 repetition count while preserving
the remaining measured outcomes.

The wider population this came from is enumerated in #7231.
19 changes: 15 additions & 4 deletions crates/perry-codegen/src/expr/instance_misc1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,8 +1015,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
// the varargs runtime entry; the no-args form goes through the
// simpler `js_queue_next_tick` to avoid the alloca cost.
Expr::ProcessNextTick { callback, args } => {
let cb_box = lower_expr(ctx, callback)?;
if args.is_empty() {
let cb_box = lower_expr(ctx, callback)?;
let blk = ctx.block();
// #3046: validate the callback (non-callable → Node's
// `ERR_INVALID_ARG_TYPE` "callback" message) before queueing.
Expand All @@ -1031,13 +1031,23 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
blk.call_void("js_queue_next_tick", &[(I64, &cb_handle)]);
return Ok(double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)));
}
// #7210: the callback and every trailing argument go through
// `lower_exprs_rooted` together. Two windows closed at once — the
// callback register, held across every argument's `lower_expr`, and
// the staging buffer, in which argument *i* has no root at all
// while argument *i+1* is lowered. See `lower_call/extern_func.rs`'s
// `setTimeout` arm for the full argument.
let n = args.len();
let mut arg_refs: Vec<&Expr> = Vec::with_capacity(n + 1);
arg_refs.push(callback);
arg_refs.extend(args.iter());
let (vals, guard) = super::temp_root::lower_exprs_rooted(ctx, &arg_refs)?;
let cb_box = vals[0].clone();
let buf = ctx.func.alloca_entry_array(DOUBLE, n);
for (i, a) in args.iter().enumerate() {
let v = lower_expr(ctx, a)?;
for (i, v) in vals.iter().skip(1).enumerate() {
let blk = ctx.block();
let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]);
blk.store(DOUBLE, &v, &slot);
blk.store(DOUBLE, v, &slot);
}
let ptr_reg = ctx.block().next_reg();
ctx.block().emit_raw(format!(
Expand All @@ -1055,6 +1065,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
"js_queue_next_tick_args",
&[(I64, &cb_handle), (PTR, &ptr_reg), (I32, &n.to_string())],
);
super::temp_root::temp_root_release(ctx, guard);
Ok(double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)))
}

Expand Down
219 changes: 12 additions & 207 deletions crates/perry-codegen/src/lower_call/extern_func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1095,214 +1095,19 @@ pub fn try_lower_extern_func_call(
if ctx.import_function_node_submodule.contains_key(name) {
return Ok(None);
}
// Timers (`setTimeout`/`setInterval`/`setImmediate` and their `clear*`
// siblings) live in `extern_timers.rs`. Split out under #7210, when the GC
// rooting fix for their trailing-argument staging buffers pushed this file
// over the 2000-line cap (`scripts/check_file_size.sh`). Dispatched HERE,
// immediately above the match, because they were its first arms: arm order
// is observable (`"setTimeout" if args.len() == 1` must still beat the
// generic consumer-prefix path at the bottom), and every guard they carry
// is on `args.len()`, so a non-matching timer name or arity falls through
// to exactly the arm it fell through to before.
if let Some(v) = super::extern_timers::try_lower_extern_timer_call(ctx, name.as_str(), args)? {
return Ok(Some(v));
}
match name.as_str() {
// #1671: `setTimeout(fn)` with no explicit delay. Node treats a
// missing/undefined delay as 0 (fires on the next timer tick).
// Without this arm a 1-arg `setTimeout` falls through to the
// catch-all below, which emits a bare LLVM call to `@setTimeout`
// and the linker fails with `Undefined symbols: _setTimeout`
// (hit by hono/jsx's `hooks/index.js`, which schedules a re-render
// via `setTimeout(() => { … })`). Route it to the same runtime
// entry as the 2-arg form with a zero delay.
"setTimeout" if args.len() == 1 => {
let cb_box = lower_expr(ctx, &args[0])?;
let blk = ctx.block();
// #2013 — validate the callback type before unboxing the
// pointer. `js_timer_validate_callback` throws
// ERR_INVALID_ARG_TYPE for any non-callable value and
// returns the raw closure pointer otherwise; the second
// arg `0` is the type-name index for "setTimeout".
let zero_idx = "0";
let cb_handle = blk.call(
I64,
"js_timer_validate_callback",
&[(DOUBLE, &cb_box), (I32, zero_idx)],
);
let zero = double_literal(0.0);
let id = blk.call(
I64,
"js_set_timeout_callback",
&[(I64, &cb_handle), (DOUBLE, &zero)],
);
return Ok(Some(nanbox_pointer_inline(blk, &id)));
}
"setTimeout" if args.len() == 2 => {
let cb_box = lower_expr(ctx, &args[0])?;
let delay_box = lower_expr(ctx, &args[1])?;
let blk = ctx.block();
let zero_idx = "0";
let cb_handle = blk.call(
I64,
"js_timer_validate_callback",
&[(DOUBLE, &cb_box), (I32, zero_idx)],
);
let id = blk.call(
I64,
"js_set_timeout_callback",
&[(I64, &cb_handle), (DOUBLE, &delay_box)],
);
return Ok(Some(nanbox_pointer_inline(blk, &id)));
}
"setImmediate" if !args.is_empty() => {
let cb_box = lower_expr(ctx, &args[0])?;
if args.len() == 1 {
let blk = ctx.block();
let two_idx = "2";
let cb_handle = blk.call(
I64,
"js_timer_validate_callback",
&[(DOUBLE, &cb_box), (I32, two_idx)],
);
let id = blk.call(I64, "js_set_immediate_callback", &[(I64, &cb_handle)]);
return Ok(Some(nanbox_pointer_inline(blk, &id)));
}

let n = args.len() - 1;
let buf = ctx.func.alloca_entry_array(DOUBLE, n);
for (i, a) in args.iter().skip(1).enumerate() {
let v = lower_expr(ctx, a)?;
let blk = ctx.block();
let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]);
blk.store(DOUBLE, &v, &slot);
}
let ptr_reg = ctx.block().next_reg();
ctx.block().emit_raw(format!(
"{} = getelementptr [{} x double], ptr {}, i64 0, i64 0",
ptr_reg, n, buf
));
let blk = ctx.block();
let two_idx = "2";
let cb_handle = blk.call(
I64,
"js_timer_validate_callback",
&[(DOUBLE, &cb_box), (I32, two_idx)],
);
let id = blk.call(
I64,
"js_set_immediate_callback_args",
&[(I64, &cb_handle), (PTR, &ptr_reg), (I32, &n.to_string())],
);
return Ok(Some(nanbox_pointer_inline(blk, &id)));
}
// Refs #665: `setTimeout(fn, delay, ...args)` — JS spec forwards
// the trailing args to `fn` when the timer fires. Pack them into
// a stack buffer of doubles and hand off to the varargs runtime
// entry. Used by Promise-executor patterns like
// `setTimeout(resolve, delay, res)` (rate-limiter-flexible's
// `RateLimiterMemory.consume` is the discovering call site).
"setTimeout" if args.len() >= 3 => {
let cb_box = lower_expr(ctx, &args[0])?;
let delay_box = lower_expr(ctx, &args[1])?;
let n = args.len() - 2;
let buf = ctx.func.alloca_entry_array(DOUBLE, n);
for (i, a) in args.iter().skip(2).enumerate() {
let v = lower_expr(ctx, a)?;
let blk = ctx.block();
let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]);
blk.store(DOUBLE, &v, &slot);
}
let ptr_reg = ctx.block().next_reg();
ctx.block().emit_raw(format!(
"{} = getelementptr [{} x double], ptr {}, i64 0, i64 0",
ptr_reg, n, buf
));
let blk = ctx.block();
let zero_idx = "0";
let cb_handle = blk.call(
I64,
"js_timer_validate_callback",
&[(DOUBLE, &cb_box), (I32, zero_idx)],
);
let id = blk.call(
I64,
"js_set_timeout_callback_args",
&[
(I64, &cb_handle),
(DOUBLE, &delay_box),
(crate::types::PTR, &ptr_reg),
(I32, &n.to_string()),
],
);
return Ok(Some(nanbox_pointer_inline(blk, &id)));
}
"setInterval" if args.len() == 2 => {
let cb_box = lower_expr(ctx, &args[0])?;
let delay_box = lower_expr(ctx, &args[1])?;
let blk = ctx.block();
let one_idx = "1";
let cb_handle = blk.call(
I64,
"js_timer_validate_callback",
&[(DOUBLE, &cb_box), (I32, one_idx)],
);
let id = blk.call(
I64,
"setInterval",
&[(I64, &cb_handle), (DOUBLE, &delay_box)],
);
return Ok(Some(nanbox_pointer_inline(blk, &id)));
}
"setInterval" if args.len() >= 3 => {
let cb_box = lower_expr(ctx, &args[0])?;
let delay_box = lower_expr(ctx, &args[1])?;
let n = args.len() - 2;
let buf = ctx.func.alloca_entry_array(DOUBLE, n);
for (i, a) in args.iter().skip(2).enumerate() {
let v = lower_expr(ctx, a)?;
let blk = ctx.block();
let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]);
blk.store(DOUBLE, &v, &slot);
}
let ptr_reg = ctx.block().next_reg();
ctx.block().emit_raw(format!(
"{} = getelementptr [{} x double], ptr {}, i64 0, i64 0",
ptr_reg, n, buf
));
let blk = ctx.block();
let one_idx = "1";
let cb_handle = blk.call(
I64,
"js_timer_validate_callback",
&[(DOUBLE, &cb_box), (I32, one_idx)],
);
let id = blk.call(
I64,
"js_set_interval_callback_args",
&[
(I64, &cb_handle),
(DOUBLE, &delay_box),
(crate::types::PTR, &ptr_reg),
(I32, &n.to_string()),
],
);
return Ok(Some(nanbox_pointer_inline(blk, &id)));
}
"clearTimeout" if args.len() == 1 => {
// Pass the raw NaN-boxed arg so the runtime accepts both the
// handle and its primitive numeric id (`clearTimeout(+t)`, #1213).
let id_box = lower_expr(ctx, &args[0])?;
ctx.block()
.call_void("js_clear_timeout_value", &[(DOUBLE, &id_box)]);
return Ok(Some(double_literal(f64::from_bits(
crate::nanbox::TAG_UNDEFINED,
))));
}
"clearInterval" if args.len() == 1 => {
let id_box = lower_expr(ctx, &args[0])?;
ctx.block()
.call_void("js_clear_interval_value", &[(DOUBLE, &id_box)]);
return Ok(Some(double_literal(f64::from_bits(
crate::nanbox::TAG_UNDEFINED,
))));
}
"clearImmediate" if args.len() == 1 => {
let id_box = lower_expr(ctx, &args[0])?;
ctx.block()
.call_void("js_clear_immediate_value", &[(DOUBLE, &id_box)]);
return Ok(Some(double_literal(f64::from_bits(
crate::nanbox::TAG_UNDEFINED,
))));
}
"gc" => {
ctx.block().call_void("js_gc_collect", &[]);
return Ok(Some(double_literal(f64::from_bits(
Expand Down
Loading
Loading