fix(runtime): carry a mock timer's registry pin through its own dispatch

[proggeramlug]

Summary

Follow-up to #10538 (landed as part of v0.5.1592, fixing #10447). That PR
gave every scheduled timer a ScheduledTimerId pin that keeps its
ref-state/kind entry in the shared registry alive until the timer is fired or
cleared, closing the original #10447 eviction bug for setTimeout,
setInterval and setImmediate. node:test's mock timers
(MockCallbackTimer, one-shot) have the same pin field, but the mock
dispatch loop dropped it one step too early: before the callback ran,
not after. A one-shot mock timer whose own callback then churns more than
the registry's eviction cap (65,536) worth of other timers evicts its own
registry entry while it is still running — the same symptom #10447 fixed for
long-lived real timers, reopened for a narrower case by the mock path.

Root cause

crates/perry-runtime/src/timer.rs, mock_timers_advance_to: for a one-shot
mock callback (not an interval), the fire loop did

let timer = state.callbacks.remove(idx);
Some((timer.id, timer.callback, timer.args, timer.context))

state.callbacks.remove(idx) takes ownership of the whole queue entry,
including its _scheduled: ScheduledTimerId pin. The tuple only takes id,
callback, args and context out of it — timer (and therefore
_scheduled) drops, retiring the id, at the end of that block: before
call_timer_callback is even invoked below it, let alone before the
callback itself returns. Once retired, the id is an ordinary eviction
candidate; if the callback churns more timers than the cap before it
finishes, its own id — the oldest entry in the retired queue at that point —
is the first one evicted.

Interval mock timers don't have this bug: their queue entry is mutated in
place and stays in the queue (it needs to re-fire), so its pin is never
dropped by this loop. The real (non-mock) CallbackTimer/IntervalTimer
paths don't have it either — js_run_timer_phase's batch-drain keeps the
whole CallbackTimer (pin included) alive in the for (index, mut timer) in expired.into_iter() loop, dropping it only at the end of each iteration,
i.e. after that timer's callback has already run.

Fix

Carry the pin in the dispatch action instead of leaving it behind on the
popped queue entry:

} else {
    let timer = state.callbacks.remove(idx);
    Some((timer.id, timer.callback, timer.args, timer.context, Some(timer._scheduled)))
}
...
if let Some((id, callback, args, context, _pin)) = action {
    call_timer_callback(id, callback, &args, &context);
    // `_pin` drops here, after the callback has returned.
}

The interval branch now returns None for the pin slot (nothing to carry —
its entry stays in the queue), so the tuple shape is shared and the two
branches stay symmetric.

Tests

crates/perry-runtime/src/timer/tests_inline.rs,
mock_dispatch_own_pin_tests::a_one_shot_mock_timers_own_pin_survives_its_own_dispatch:
schedules one mock one-shot timer whose callback churns
TIMER_REF_STATES_CAP + 2_000 real setTimeout+clearTimeout cycles and
then checks its own id (is_known_timer_id, js_timer_has_ref) from
inside the still-running callback.

Reproduced on clean main first, then fixed (this repo's package-audit
convention): with the fix reverted (git show HEAD:crates/perry-runtime/src/timer.rs
restored, keeping only the new test) the test fails —

thread '...a_one_shot_mock_timers_own_pin_survives_its_own_dispatch' panicked at
crates/perry-runtime/src/timer/tests_inline.rs:...:
timer 1 was evicted from the registry by its own callback's churn

— and passes with the fix restored. Existing gap coverage re-run and
confirmed unaffected: test_gap_10447_timer_ref_state_eviction,
test_parity_timers, test_parity_timers_promises, test_parity_test
(covers node:test's mock.timers surface).

Validation

All on Linux x64, Node 26.5.1, cgu=16 release builds.

• RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --tests:
  3982 passed, 2 failed, 4 ignored. Both failures are pre-existing and
  unrelated (confirmed identical on an independent branch touching disjoint
  files — see the companion fix(codegen,runtime): remove the argument-count ceilings on dynamic calls #10532-followup PR for the same two):
  gc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds
  and gc::tests::copy_slot_decode::sabotaged_remembering_arm_is_refused_by_the_coverage_cross_check,
  both debug_assert!-gated checks a --release test build compiles out.

• Lint (SKIP_COMPILE_GATES=1 ./scripts/run_lint_gates.sh; the compile
  tier is documented as red on this Linux build host independent of any
  change, so it was not run — see host notes): 76 of 77 gates passed, 2
  CI-only skipped. The one red, benchmarks/ci_public_baseline_check.py, is
  pre-existing — confirmed failing identically on an unmodified c8cf450563
  checkout. scripts/gc_runtime_root_holders.py flagged the new test's
  SELF_ID: AtomicI64 (a timer id, never a GC pointer, #[cfg(test)]-only)
  — classified test_only in scripts/gc_runtime_root_holders.json.
  raw_handle_debt.py: unaffected (906, unchanged).

• Gap suite (targeted, not a full local sweep — this change is a
  narrow, mock-timers-only dispatch fix, not a hot path): all four filters
  above PASS.

• Performance (perf stat -e instructions,task-clock, 3 runs/arm,
  medians below, Linux x64, PERRY_NO_AUTO_OPTIMIZE=1, cgu=16 release
  binaries; baseline = unmodified c8cf450563). Output verified
  byte-identical between baseline and fix binaries before timing.

  workload	baseline instructions	fix	Δ
  200,000× (setTimeout(f,10) scheduled, then tick(10) fires it)	2,103,900,965	2,101,627,137	−0.11%

  Within noise — the fix moves one ScheduledTimerId (a single i64) one
  extra step through an existing tuple/Option instead of dropping it in
  place; there is no additional allocation or lock on the per-firing path.

Not verified

• Only Linux x64 was built and tested.
• Multi-agent / perry/thread interaction with mock timers was not
  exercised (mock timers are a single-realm feature; this was true before
  this fix too).
• setInterval/setImmediate mock-timer churn scenarios were not
  separately scaled — the bug is specific to the one-shot removal path, and
  the fix's None branch for intervals is unchanged behavior, covered by
  existing interval tests.

Recovery note

This fix (and its regression test) were originally developed and verified on
a per-hour build host that was shut down mid-validation (owner action, not
an incident) before either could be pushed. The recovered material applied
against an older base (7661bc05fe, before #10538 landed) and duplicated
most of #10538's now-landed ScheduledTimerId machinery rather than
containing the actual one-step fix over it. Rather than replay a stale diff,
this PR re-derives the fix directly against current main's real
mock_timers_advance_to/ref_states.rs, from the root cause described
above, and adds a fresh regression test reproduced against clean main
first. Everything in Validation above ran from scratch on a fresh clone.

Part of #10447 / #10538 (the original ref-state eviction bug for real
timers is already fixed and merged; this closes a narrower gap in the mock
timer dispatch path that #10538 did not cover).

Summary by CodeRabbit

• Bug Fixes

  • Fixed one-shot mock timers being prematurely retired while their callbacks were still running.
  • Timer handles now remain valid throughout callback execution, including when the callback creates and clears many other timers.

• Tests

  • Added regression coverage to verify that a timer remains recognized and referenced until its callback completes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: b5a7c77c-f4b5-453f-a8e7-8e2780489835

📥 Commits

Reviewing files that changed from the base of the PR and between 8f06128 and af7498c.

📒 Files selected for processing (1)

• scripts/gc_runtime_root_holders.json

Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The mock timer dispatch path now retains a one-shot timer’s scheduled ID through callback execution. A regression test verifies that the ID remains known and referenced during timer churn.

Changes

Mock timer dispatch

Layer / File(s)	Summary
Retain the one-shot pin during dispatch crates/perry-runtime/src/timer.rs, crates/perry-runtime/src/timer/tests_inline.rs, scripts/gc_runtime_root_holders.json, changelog.d/10588-mock-timer-dispatch-pin.md	The dispatch action carries the one-shot timer pin until call_timer_callback returns. The regression test validates timer identity and reference state during callback churn. The test root-holder configuration tracks the callback’s timer ID. The changelog records the fix.

Priority: ⚪ Not assessed

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to af749

The mock timer dispatch fix preserves timer identity through callback execution, keeps interval behavior unchanged, and adds focused regression coverage. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the main fix: preserving a mock timer's registry pin through its callback dispatch.
Description check	✅ Passed	The description is detailed and covers the change, root cause, fix, related issues, regression test, validation results, limitations, and recovery context. It does not use every template heading or ex…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Landed via merge train #10631 (v0.5.1595). All source commits preserve authorship; merged main matches the validated train exactly.