From 6de2fe88185f123c0521b1c4b4381a849ea5a3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 02:44:48 +0000 Subject: [PATCH 1/3] fix(runtime): carry a mock timer's registry pin through its own dispatch A one-shot mock timer (node:test's mock.timers) left the queue via state.callbacks.remove(idx) and was destructured into (id, callback, args, context), dropping the popped entry's ScheduledTimerId pin right there -- before call_timer_callback had run, let alone finished. A callback that then churned more than TIMER_REF_STATES_CAP timers evicted its own handle mid-dispatch, exactly as #10447 evicted long- lived timers before the pin existed. Carry the ScheduledTimerId in the dispatch action tuple instead, and let it drop only after call_timer_callback returns. Interval mock timers are unaffected: their entry stays in the queue across a tick, so their pin was never at risk. --- crates/perry-runtime/src/timer.rs | 24 ++++++- .../perry-runtime/src/timer/tests_inline.rs | 70 +++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/crates/perry-runtime/src/timer.rs b/crates/perry-runtime/src/timer.rs index a52fc5a5db..95b1162978 100644 --- a/crates/perry-runtime/src/timer.rs +++ b/crates/perry-runtime/src/timer.rs @@ -815,6 +815,8 @@ fn mock_timers_advance_to(target_ms: f64) { }; state.current_ms = due_ms; if is_interval { + // The interval entry stays in the queue (it re-fires), so its + // `_scheduled` pin is untouched here — nothing to carry. let timer = &mut state.intervals[idx]; timer.next_ms = due_ms + timer.interval_ms.max(1) as f64; Some(( @@ -822,14 +824,32 @@ fn mock_timers_advance_to(target_ms: f64) { timer.callback, timer.args.clone(), timer.context.clone(), + None, )) } else { + // #10447 follow-up: `remove` takes the WHOLE entry, including + // its `_scheduled` pin. Move that pin into the action too and + // hand it back below, instead of leaving it behind on `timer` + // to drop (and retire the id) right here — before + // `call_timer_callback` has even run, let alone finished. A + // one-shot mock timer otherwise loses its own registry entry + // if its callback churns more than the eviction cap's worth of + // other timers while it is still dispatching. let timer = state.callbacks.remove(idx); - Some((timer.id, timer.callback, timer.args, timer.context)) + Some(( + timer.id, + timer.callback, + timer.args, + timer.context, + Some(timer._scheduled), + )) } }; - if let Some((id, callback, args, context)) = action { + if let Some((id, callback, args, context, _pin)) = action { call_timer_callback(id, callback, &args, &context); + // `_pin` (the one-shot case's `ScheduledTimerId`, moved out of the + // popped queue entry above) stays alive across that call and only + // retires the id here, after the callback has returned. } } } diff --git a/crates/perry-runtime/src/timer/tests_inline.rs b/crates/perry-runtime/src/timer/tests_inline.rs index f365981d55..96c6255cad 100644 --- a/crates/perry-runtime/src/timer/tests_inline.rs +++ b/crates/perry-runtime/src/timer/tests_inline.rs @@ -244,3 +244,73 @@ mod expired_batch_order_tests { ); } } + +#[cfg(test)] +mod mock_dispatch_own_pin_tests { + use super::*; + use std::sync::atomic::{AtomicBool, AtomicI64, Ordering}; + + static SELF_ID: AtomicI64 = AtomicI64::new(0); + static SAW_KNOWN: AtomicBool = AtomicBool::new(false); + static SAW_HAS_REF: AtomicBool = AtomicBool::new(false); + static RAN: AtomicBool = AtomicBool::new(false); + + /// A one-shot mock timer's own callback: churns more real one-shot timers + /// than the registry's eviction cap, then checks its OWN id. If this + /// timer's `_scheduled` pin already retired the moment it was popped off + /// the mock queue for dispatch (the bug), it is the OLDEST retired id in + /// the shared registry when the churn starts, so it is the very first one + /// evicted once the churn passes the cap — and this callback observes its + /// own eviction while it is still running. + extern "C" fn churn_then_check_self(_closure: *const crate::closure::ClosureHeader) -> f64 { + let id = SELF_ID.load(Ordering::SeqCst); + for _ in 0..(ref_states::TIMER_REF_STATES_CAP + 2_000) { + clearTimeout(js_set_timeout_callback(0, 1_000.0)); + } + SAW_KNOWN.store(is_known_timer_id(id), Ordering::SeqCst); + SAW_HAS_REF.store(js_timer_has_ref(id) != 0, Ordering::SeqCst); + RAN.store(true, Ordering::SeqCst); + 0.0 + } + + /// #10447 follow-up: `mock_timers_advance_to` used to pop a one-shot mock + /// timer off the queue with `state.callbacks.remove(idx)` and destructure + /// out `(id, callback, args, context)` — leaving the popped entry's + /// `_scheduled: ScheduledTimerId` behind to drop, and retire the id, right + /// there, before `call_timer_callback` had even run, let alone finished. + /// A callback that then churned more timers than the eviction cap evicted + /// its OWN handle mid-dispatch. The fix carries the pin into the dispatch + /// action and drops it only after the callback returns. + #[test] + fn a_one_shot_mock_timers_own_pin_survives_its_own_dispatch() { + let _serial = crate::gc::global_side_table_test_lock(); + SAW_KNOWN.store(false, Ordering::SeqCst); + SAW_HAS_REF.store(false, Ordering::SeqCst); + RAN.store(false, Ordering::SeqCst); + js_mock_timers_reset(); + js_mock_timers_enable(MOCK_TIMERS_API_SET_TIMEOUT, 0.0); + + let closure = crate::closure::js_closure_alloc(churn_then_check_self as *const u8, 0); + let id = schedule_mock_callback_timer( + closure as i64, + 10.0, + Vec::new(), + CallbackTimerKind::Timeout, + ) + .expect("mock setTimeout must be enabled for this API set"); + SELF_ID.store(id, Ordering::SeqCst); + + js_mock_timers_tick(10.0); + + assert!(RAN.load(Ordering::SeqCst), "the mock timer never fired"); + assert!( + SAW_KNOWN.load(Ordering::SeqCst), + "timer {id} was evicted from the registry by its own callback's churn" + ); + assert!( + SAW_HAS_REF.load(Ordering::SeqCst), + "timer {id}'s ref state was lost to its own callback's churn" + ); + js_mock_timers_reset(); + } +} From 8f06128442d63649da2b5f82b7b1dafa8bc70d46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 06:11:21 +0000 Subject: [PATCH 2/3] docs(changelog): add fragment for #10588 --- changelog.d/10588-mock-timer-dispatch-pin.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changelog.d/10588-mock-timer-dispatch-pin.md diff --git a/changelog.d/10588-mock-timer-dispatch-pin.md b/changelog.d/10588-mock-timer-dispatch-pin.md new file mode 100644 index 0000000000..9dadce0dd3 --- /dev/null +++ b/changelog.d/10588-mock-timer-dispatch-pin.md @@ -0,0 +1,11 @@ +Fixed a follow-on to #10447/#10538: a one-shot `node:test` mock timer +(`setTimeout` under `mock.timers`) dropped its own registry pin +(`ScheduledTimerId`) the moment it left the mock queue for dispatch, rather +than after its callback returned. A callback that then scheduled and cleared +more than the registry's 65,536-entry eviction cap's worth of other timers +before finishing evicted its own handle mid-dispatch — the same symptom +#10447 fixed for long-lived real timers, reopened narrowly for the mock +dispatch path. The pin now rides along in the dispatch action tuple and +drops only after `call_timer_callback` returns. Real (non-mock) timers and +mock intervals were never affected. Reproduced against clean `main` with a +new regression test before the fix, confirmed passing after. From af7498c754d5760e5944d55ce7079f26ce41e000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 06:41:49 +0000 Subject: [PATCH 3/3] tooling: classify the mock-timer pin test's SELF_ID as test-only scripts/gc_runtime_root_holders.py flags any new static/thread_local whose type could hold a GC heap pointer that no registered scanner in its own file reaches. crates/perry-runtime/src/timer/tests_inline.rs's new SELF_ID: AtomicI64 (added by the mock-timer dispatch-pin regression test) stores a scheduled timer id, never a heap pointer, and only exists under #[cfg(test)]. Classify it test_only. --- scripts/gc_runtime_root_holders.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 70d46c0e73..1dc8faeeb6 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -2529,6 +2529,12 @@ "name": "WINDOW_ROOTS", "verdict": "not_a_gc_pointer", "why": "Window-root registry maps numeric window handles to numeric root-widget handles; neither value is a JavaScript heap pointer." + }, + { + "file": "crates/perry-runtime/src/timer/tests_inline.rs", + "name": "SELF_ID", + "verdict": "test_only", + "why": "AtomicI64 holding a scheduled mock timer's id (an i64 returned by schedule_mock_callback_timer, never a GC heap pointer) so the timer's own extern \"C\" callback can look itself up in the ref-state registry mid-dispatch. Declared under #[cfg(test)] only (tests_inline.rs, mock_dispatch_own_pin_tests), never live in a shipped binary." } ], "_FRONTIER_README": "Identity-pinned debt ratchet over new perry-ui* candidates and otherwise-unclassified core raw/Perry TLS 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.",