fix(runtime): timeout.refresh() preserves ref state; Immediate has no numeric conversion

[proggeramlug]

Summary

Two small timer-semantics defects in crates/perry-runtime/src/timer.rs, fixed together (same subsystem, same file):

• timeout.refresh() re-refs an unref()'d timer: hasRef() flips to true and the process stays alive until it fires #10541 — timeout.refresh() re-refs an unref()'d Timeout/interval: hasRef() flips to true and the process stays alive for a callback that had been deliberately detached from the event loop.
• +setImmediate(...) is a number in Perry; Node's Immediate has no numeric conversion (NaN) #10542 — +setImmediate(...) coerces to a number in Perry; in Node an Immediate has no numeric conversion (+setImmediate(...) is NaN), unlike Timeout (+setTimeout(...) / +setInterval(...) is the internal id).

Root cause

• timeout.refresh() re-refs an unref()'d timer: hasRef() flips to true and the process stays alive until it fires #10541: js_timer_refresh (crates/perry-runtime/src/timer.rs:914) called set_timer_ref_state(timer_id, true) unconditionally after rescheduling both the CallbackTimer (Timeout) and IntervalTimer branches. Node's Timeout.refresh() only resets the start time / reschedules the deadline — it never touches ref state. The forced write meant any unref()'d timer that was later refresh()'d became ref'd again.
• +setImmediate(...) is a number in Perry; Node's Immediate has no numeric conversion (NaN) #10542: js_number_coerce's pointer branch (crates/perry-runtime/src/builtins/numbers.rs:562) has a fast path that converts any known timer handle straight to its numeric id, gated only on crate::timer::is_known_timer_id(id) — which is true for both Timeout and Immediate handles alike (register_scheduled_timer records kind: CallbackTimerKind::{Timeout,Immediate} but the coercion never consulted it).

Fix

• js_timer_refresh: removed both set_timer_ref_state(timer_id, true) calls. The id's ref-state entry is left exactly as it was — set at register_scheduled_timer time and updated by any ref()/unref() call since — and per #10447's pinning, it cannot have been evicted from the bounded ref-state registry while the timer is still queued (_scheduled: ScheduledTimerId holds the pin).
• builtins/numbers.rs: added crate::timer::is_immediate_timer_id(id) (new pub(crate) fn in timer.rs, thin wrapper over the existing timer_handle_kind) and gated the numeric shortcut on !is_immediate_timer_id(id). An Immediate now falls through to the existing generic (object-shaped) toPrimitive/toString path, which already produces "[object Object]" → NaN — no new code needed there, it was simply unreachable for timer handles before.

Both fixes are 2–10 line, surgical diffs; no architectural changes.

Tests

Added test-files/test_gap_10541_10542_timer_refresh_ref_immediate_primitive.ts, comparing byte-for-byte against node --experimental-strip-types (pinned Node 26.5.1). Covers:

• unref() then refresh() on a Timeout and on an Interval: both must keep hasRef() === false, and the process must actually exit without running the (BUG-labelled) detached callback — this is measured, not assumed: the unref'd 1.2 s timer is left un-cleared, so if the bug re-refs it, its callback fires and prints before exit, which is what diverges the diff.
• ref() (explicit) then refresh(): must stay ref'd (guards a naive alternative fix that unconditionally clears ref state instead of leaving it alone).
• A plain refresh()'d Timeout still reschedules and fires (proves the reschedule itself, not just ref state, still works).
• +setImmediate(...): typeof is "number", Number.isNaN(...) is true; Object.prototype.toString.call(immediate) is "[object Object]".
• +setTimeout(...) / +setInterval(...): verified against real Node 26.5.1 first (not assumed) — both remain numbers, Number.isNaN is false.
• clearTimeout/clearImmediate still work on all the handles produced above.

Proof the test fails on the baseline (/root/claude-fixbase-10476-main, unpatched, v0.5.1593, same-shape timer.rs as pre-fix): compiling and running the new test there reproduces both bugs exactly —

unref'd timeout after refresh: hasRef=true ctor=Timeout ...      (expected hasRef=false)
immediate: hasRef=true ctor=Immediate typeof(+t)=number isNaN(+t)=false   (expected isNaN(+t)=true)
...
BUG: unref'd refresh()'d timeout fired
exit

On the fixed build, output is byte-for-byte identical to the Node oracle.

Also added two perry-runtime unit tests in crates/perry-runtime/src/timer/tests_inline.rs:

• refresh_and_immediate_primitive_tests::refresh_preserves_ref_state — exercises js_timer_refresh/js_timer_ref/js_timer_unref/js_timer_has_ref directly for a Timeout, a ref'd Timeout, and an Interval.
• refresh_and_immediate_primitive_tests::immediate_kind_is_distinguished_from_timeout — exercises the new is_immediate_timer_id against Timeout/Interval/Immediate handles.

Validation

• RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --tests timer: 32/32 passed (includes the two new unit tests plus all pre-existing timer suites, including #10447's live_timers_keep_ref_state_across_timer_churn, unaffected). No #[no_mangle] signatures or emitted-IR symbols changed, so no perry/codegen test re-run was needed.

• Lint (SKIP_COMPILE_GATES=1 ./scripts/run_lint_gates.sh): 76/77 gates passed. The one red is Public benchmark evidence freshness, pre-existing/red on every PR in this repo (unrelated to this change).

• Gap suite (targeted, PERRY_SKIP_BUILD=1 ./run_parity_tests.sh --filter <name>): test_gap_10541_10542_timer_refresh_ref_immediate_primitive PASS, test_gap_10447_timer_ref_state_eviction PASS, test_gap_6287_timer_batch_order PASS, test_parity_timers PASS, test_parity_timers_promises PASS. Did not run the full local gap suite (this change is scoped to two FFI entry points in one file, not a hot lowering/runtime path used by most programs); CI's gap-suite shards are the full gate per the standard process.

• Performance: perf stat -e instructions,task-clock, 3 runs each, baseline (/root/claude-fixbase-10476-main, unpatched, same perry-dev profile) vs this fix, on a refresh()++timeout-coercion churn loop (2,000,000 iterations, exercising both touched functions every iteration):

  	instructions (avg of 3)	task-clock (avg of 3)
  baseline	10,842,480,795	~1,049,413,568 ns
  fixed	10,805,994,457	~1,023,680,881 ns
  Δ	−0.34%	−2.5%

  No regression (both deltas favor the fix slightly — js_timer_refresh now does one fewer ref-state write per call than before, which outweighs the one added is_immediate_timer_id check in js_number_coerce's already-cold timer branch). Node wall time for the same 2,000,000-iteration loop, for context only (different runtime, not a direct comparison): 0.228 s. Perry's absolute wall time for this specific synthetic micro-loop (~1.0 s) is a pre-existing characteristic of the mutex/Vec-scan-based timer queue (unrelated to and unmoved by this fix, per the A/B above) and out of scope for this PR.

Not verified

• Did not investigate Perry's absolute (non-comparative) timer-refresh throughput vs Node for the synthetic churn microbenchmark above — orthogonal to these two semantic bugs.
• No specific npm package was named in either issue (both describe the general idle-timeout/debounce/keep-alive-timer pattern), so no package-level repro was re-run.
• Did not run the full local gap suite (see Validation above); relying on CI's shards per the standard process.

Fixes #10541
Fixes #10542

Summary by CodeRabbit

• Bug Fixes
  • Timeout.refresh() and interval refreshes now preserve the timer’s existing ref()/unref() state.
  • Immediate handles no longer convert to their internal numeric IDs; numeric conversion now produces NaN, matching Node.js behavior.
  • Timeout and interval handles retain their numeric conversion behavior.
• Tests
  • Added coverage for timer reference state and handle conversion behavior.

[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: 62228213-dc2e-4f50-a37a-040c88afc75b

📥 Commits

Reviewing files that changed from the base of the PR and between 0058bab and 5f26f92.

📒 Files selected for processing (5)

• changelog.d/10620-timer-refresh-ref-and-immediate-primitive.md
• crates/perry-runtime/src/builtins/numbers.rs
• crates/perry-runtime/src/timer.rs
• crates/perry-runtime/src/timer/tests_inline.rs
• test-files/test_gap_10541_10542_timer_refresh_ref_immediate_primitive.ts

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

---

📝 Walkthrough

Walkthrough

The timer runtime now preserves ref and unref state across refresh(). Numeric coercion identifies Immediate handles separately, so +setImmediate(...) follows generic coercion while Timeout and interval handles retain numeric IDs. Unit and regression tests cover both changes.

Changes

Timer handle behavior

Layer / File(s)	Summary
Preserve timer ref state during refresh crates/perry-runtime/src/timer.rs, crates/perry-runtime/src/timer/tests_inline.rs, test-files/test_gap_10541_10542_timer_refresh_ref_immediate_primitive.ts, changelog.d/10620-timer-refresh-ref-and-immediate-primitive.md	js_timer_refresh no longer forces Timeout or interval handles into the ref'd state. Tests verify preserved ref and unref states for refreshed timers.
Distinguish Immediate numeric coercion crates/perry-runtime/src/timer.rs, crates/perry-runtime/src/builtins/numbers.rs, crates/perry-runtime/src/timer/tests_inline.rs, test-files/test_gap_10541_10542_timer_refresh_ref_immediate_primitive.ts	Immediate timer IDs are excluded from the numeric-conversion shortcut. Timeout and interval handles retain numeric conversion, while Immediate handles produce NaN through generic coercion. Tests verify handle classification, conversion, and clearing.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 5f26f

The timer behavior changes preserve the intended ref-state and numeric-coercion semantics with matching regression coverage. No actionable merge 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 8 functions across 4 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 both primary fixes: preserving timer ref state during refresh and removing numeric conversion for Immediate handles.
Description check	✅ Passed	The description provides a detailed summary, root causes, fixes, related issues, test coverage, validation results, performance findings, and known limitations. It does not reproduce every template he…
Linked Issues check	✅ Passed	The PR meets the coding requirements in [#10541] and [#10542]. js_timer_refresh now reschedules timers without forcing the ref state to true, so unref() and explicit ref() states remain unchan…
Out of Scope Changes check	✅ Passed	The changed runtime code, inline unit tests, parity test, and changelog entry directly support [#10541] and [#10542]. The changes preserve existing timer cancellation and ref-state behavior. No unrela…

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 8 functions across 4 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 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.}