bun:test fake timers: never move the fake clock backwards

[robobun]

Problem

• A timer callback that calls a jest timer control (reported: jest.runOnlyPendingTimers() in a setInterval callback during jest.advanceTimersByTime(10)) can move the fake clock past timers the outer drain still has to fire. Debug builds abort with assertion failed: now.eql(&prev.unwrap()) || now.greater(&prev.unwrap()) in FakeTimers::fire. Release builds let Date.now() read 1, 10, 2, 3, 4 inside one advance.
• FakeTimers::fire (src/runtime/test_runner/timers/FakeTimers.rs:252) set the clock to each popped timer's deadline. advance_timers_by_time (:448) then set it to its own target. That also rewound a nested advanceTimersByTime(100) (105 back to 10) and re-pinned a fake Date.now() after a callback's useRealTimers().

Fix

• fire() fires a popped timer at max(deadline, current time), like an overdue real timer. advanceTimersByTime() sets the clock to its target only when the clock is not past it and fake timers are still installed.
• Correct because the clock only moves forward and every timer due by the target still fires, in heap order. Jest 29 never rewinds either. Two unrelated differences from Jest stay, so the tests pin Bun's values (see Notes).
• Verified: test/js/bun/test/fake-timers/fake-timers.test.ts (7 new cases, all fail on 1.4.3). Also the rest of that directory, the cron, jsonwebtoken, test-timers and --isolate suites.
• Self-reviewed: 1 concern raised, 1 addressed (an advance that ends exactly on its target still re-syncs the clock, as before).

Background

• The jest timer controls pop due timers from the fake heap and run them synchronously through fire(), which first sets the fake clock. Date.now(), performance.now() and new timer deadlines read that clock.
• A setInterval is out of the heap while its callback runs and goes back in afterwards, as in Node. A nested control cannot see it, and its next deadline can already be behind the clock.

Notes

Jest 29.7 (@sinonjs/fake-timers) on the shapes of the new tests, T0 = useFakeTimers({ now }):

shape	Bun before	Bun after	Jest 29
setInterval(10) whose first tick calls advanceTimersByTime(50), cleared on tick 3, outer runAllTimers()	iv@10, iv@20, iv@30, clock 10, 60, 20, 30 (debug: abort)	iv@10, iv@60 (late), iv@70, ends at 70	iv@10, then iv@20 and iv@30 nested inside tick 1, ends at 60
setInterval(1) whose callback calls runOnlyPendingTimers(), plus setTimeout(10), outer advanceTimersByTime(10)	iv@1, to@10, iv@2, iv@3, iv@4, ends at 10 (debug: abort)	iv@1, to@10, iv@10 (late), ends at 10, interval pending at 11	iv@1, iv@2, iv@3, iv@4 (each nested one level deeper), to@10, ends at 40
setTimeout(5) whose callback calls advanceTimersByTime(100), plus setTimeout(50), outer advanceTimersByTime(10)	a@5, b@50, a-after@105, ends at 10	a@5, b@50, a-after@105, ends at 105	a@5, b@50, a-after@105, ends at 110
same, nested runOnlyPendingTimers() / runAllTimers() / advanceTimersToNextTimer()	a-after@50, ends at 10	a-after@50, ends at 50	a-after@50, ends at 55
setTimeout(5) whose callback calls useRealTimers(), outer advanceTimersByTime(2 ** 31)	Date.now() is T0 + 2^31 and performance.now() is 2^31 afterwards, isFakeTimers() false	both real	both real (Jest also still fires the old clock's remaining timers; Bun's useRealTimers() drops them, unchanged here)

The two remaining differences from Jest:

• sinon's doTick shifts tickTo by however much clock.now moved during a callback. That exists to compensate for setSystemTime() inside a callback (sinon keys timers on the system time, Bun keys them on a separate monotonic clock and already handles that case by rebasing the Date.now() offset). A nested tick trips the same compensation, so the outer call keeps spending its remaining budget after the nested jump (110 instead of 105, and 40 in the interval case where every nesting level compensates). Jest's legacy timers do the same through msToRun.
• sinon reschedules an interval (callAt += interval) before it runs the callback, so a nested control sees the interval and fires it again, re-entering the callback recursively. Bun, like Node, keeps the interval out of the heap during its callback and re-inserts it afterwards.

Neither is about the clock running backwards. Matching either is a separate decision.

#38740 (per-VM fake clock) keeps the monotonicity assert in fire() and does not cover re-entrant controls. It also stops advanceTimersByTime() from re-pinning the clock after useRealTimers(), which the second hunk here does as well; the two will need a small textual merge.

test/js/web/timers/setTimeout.test.js / setInterval.test.js leak cases fail locally under the debug ASAN build with and without this change (RSS threshold); they do not use fake timers.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 12 days. After that, they cost $0.25 per reviewed file.}

Or wait 21 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 374c8b18-7524-416f-9bc6-fdeb8681f7d7

📥 Commits

Reviewing files that changed from the base of the PR and between d745f03 and fb18488.

📒 Files selected for processing (2)

• src/runtime/test_runner/timers/FakeTimers.rs
• test/js/bun/test/fake-timers/fake-timers.test.ts

---

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

[robobun]

Status: ready for review.

CI (build 113126): the new tests and the rest of the suite pass on every lane that ran them. The two red entries are unrelated to this diff: test/cli/install/bun-patch.test.ts on Windows x64 (long-label bun patch cases, also flaky on main, reported separately) and test/js/node/test/parallel/test-crypto-dh-leak.js on x64-asan (pre-existing on main).

Reproduced on release 1.4.3 and on a debug ASAN build of main (Malloc=1, 3/3 SIGABRT at FakeTimers.rs:261) with:

import { test, jest } from "bun:test";
test("re-entrant runOnlyPendingTimers during advanceTimersByTime", () => {
  jest.useFakeTimers({ now: 1000000 });
  const seen = []; let n = 0;
  const h = setInterval(() => { n++; seen.push(["iv", n, Date.now() - 1000000]); if (n >= 4) clearInterval(h); jest.runOnlyPendingTimers(); }, 1);
  setTimeout(() => seen.push(["to", Date.now() - 1000000]), 10);
  jest.advanceTimersByTime(10);
  console.log(JSON.stringify(seen)); // 1.4.3: [["iv",1,1],["to",10],["iv",2,2],["iv",3,3],["iv",4,4]]
  jest.useRealTimers();
});

With this branch the same file prints [["iv",1,1],["to",10],["iv",2,10]] and leaves the interval pending at 11 ms. The 7 new cases in test/js/bun/test/fake-timers/fake-timers.test.ts fail on 1.4.3 and pass on the branch.

[claude]

I reviewed this and didn't find any bugs. Deferring to a human because the fix makes a deliberate compat choice — overdue timers fire "late" at the current clock rather than matching Jest/sinon's nested-budget-shift and interval-reschedule-before-callback behavior — which the PR documents but a maintainer should ratify, and it notes a textual conflict with #38740.

What was reviewed:

• fire() now clamps to max(deadline, current) and advance_timers_by_time only re-syncs when the clock hasn't passed the target and fake timers are still installed — checked that these are the only post-drain CURRENT_TIME.set sites, so all sibling controls (runAllTimers, runOnlyPendingTimers, advanceTimersToNextTimer) are covered via fire().
• EventLoopTimer::fire now receives the clamped now instead of the raw deadline — timespec is an alias of Timespec, so the type is unchanged; interval reschedule from now matches the test expectations and Node's overdue-interval semantics.
• New tests cover the variant matrix (setInterval/setTimeout × all four nested controls × the useRealTimers teardown) with exact-value array assertions.

Extended reasoning...

Overview

The PR fixes a re-entrancy bug in bun:test fake timers (src/runtime/test_runner/timers/FakeTimers.rs): when a timer callback itself calls a jest/vi timer-control API during an outer drain, the fake clock could move backwards — aborting on a debug_assert! in CI/debug builds and producing non-monotonic Date.now() in release. Two hunks: fire() now sets the clock to max(popped-deadline, current) instead of unconditionally to the deadline, and advance_timers_by_time() only writes its target back if the clock hasn't already passed it and fake timers weren't torn down mid-drain (get_timespec_now().is_some_and(...)). Seven new test cases in test/js/bun/test/fake-timers/fake-timers.test.ts pin the exact observed sequences.

Security risks

None. This is test-runner-only mocked-clock logic; no untrusted input parsing, no I/O, no auth/crypto surface. The unsafe blocks touched are unchanged in their contracts (still dereferencing a just-popped heap node); the only substantive change inside them is which Timespec reference is passed to EventLoopTimer::fire, and timespec/ElTimespec/Timespec are all the same struct.

Level of scrutiny

Moderate. The code change is small and mechanically sound, but it encodes a behavioral decision: rather than replicating Jest 29 / sinon's semantics (shift the outer tick budget by however far the nested call moved the clock; reschedule intervals before running their callback so nested controls re-enter them), Bun fires the overdue timer once at the current time and reschedules from there. The PR description tabulates the divergence and argues both remaining differences are orthogonal to the clock-goes-backwards bug. That reasoning is coherent, and monotonicity is unambiguously correct, but whether "fire late" vs. "match Jest" is the right long-term compat stance is a maintainer call, especially given the noted overlap with #38740 (per-VM fake clock).

Other factors

I confirmed the fix covers the whole class: CURRENT_TIME.set appears in exactly three places (activate, fire, advance_timers_by_time), and the other timer-control host functions all drain through fire() without a separate post-drain clock write, so no sibling site was missed. The tests follow repo conventions — added to the existing fake-timers.test.ts, use test.each for the four-method matrix, assert whole arrays with .toEqual, and check both Date.now() and performance.now(). The useRealTimers-inside-callback test uses toBeGreaterThanOrEqual(realDate) / toBeLessThan(2 ** 31) bounds rather than sleeping, which is the right shape for a real-clock assertion. The old CI-only debug_assert! on monotonicity is replaced by unconditional runtime clamping, which is the correct move per REVIEW.md ("debug assertions compile out — validation of runtime invariants that user-triggered re-entrancy can violate must survive release").

[robobun]

^{Updated 12:36 PM PT - Sep 8th, 2026}

❌ @robobun, your commit fb18488 has 2 failures in Build #113126 (All Failures):

• test/js/node/test/parallel/test-crypto-dh-leak.js - code 1 on 🐧 13 x64-asan
• test/cli/install/bun-patch.test.ts - code 1 on 🪟 2019 x64

---

🧪   To try this PR locally:

bunx bun-pr 42045

That installs a local version of the PR into your bun-42045 executable, so you can run:

bun-42045 --bun

[robobun]

On the compat point the review defers on: the choice to ratify is whether an outer advanceTimersByTime(n) ends at start + n (this PR, unless a nested call already went further) or keeps spending its remaining budget after a nested jump (Jest 29 / sinon: 110 instead of 105 in the Notes table). If the Jest behaviour is preferred, it is a contained follow-up: execute_until would extend until by however far each fired callback moved the clock, and advance_timers_by_time would use the extended value. The max(deadline, now) clamp in fire() is needed either way, because Bun re-inserts a setInterval after its callback and that deadline can already be behind the clock.

[robobun]

I worked the same bug from a different report (a nested advanceTimersByTime() rewinding the clock, [ 50, 1050, 100 ] on 1.4.3) and landed on the same fire() change. I am not opening a second PR. The branch is robobun/a984b9b6/fake-timers-nested-advance (36442c8) in case a maintainer prefers its semantics for the outer call.

The one difference is what the outer call does after a callback moved the clock:

• This PR keeps the absolute target from entry and parks at max(target, now). advanceTimersByTime(10) with a nested advanceTimersByTime(100) at 5 ends at 105.
• The branch advances by duration: the loop rereads the clock after each callback and spends the rest of the span from wherever the callback left it. The same case ends at 110, and runOnlyPendingTimers() becomes advance(last - now). These are the values @sinonjs/fake-timers 15.4.0 (Jest modern timers, vitest) and Jest's legacy msToRun loop produce. The six tests on the branch are pinned to sinon's output.

One trade-off to weigh before choosing the duration form: a setInterval whose callback always advances the clock past its own next deadline comes back overdue every time, costs the outer loop no budget, and the outer advanceTimersByTime() never returns. This PR and 1.4.3 terminate in that case. sinon overflows the stack instead, because it re-fires the interval inside the nested tick(). So the duration form would want a loop limit like Jest's legacy maxLoops on top.

If the clamp is the decision, nothing from the branch is needed. If sinon parity is wanted, the advance_by loop and its tests can be folded in here.