bun:test: cover Bun.serve's Date header timer under fake timers - #37935
bun:test: cover Bun.serve's Date header timer under fake timers#37935robobun wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Status: ready for review, tests only. #37946 merged first and contains the source changes this PR originally made (plus the same treatment for every other runtime-internal timer), so this PR now only adds the two Reproduced on CI: the new tests pass on every lane that has run them, including Windows x64. The one red lane in the latest build is |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — focused fix that opts DateHeaderTimer out of the fake-timer heap and switches its two clock reads to ForceRealTime, mirroring the existing GcRepeating treatment.
What was reviewed:
allow_fake_timers()addition sits alongside the other internal pacing tags;All::insertroutes it to the real heap, whichAll::next/drain_timersalready drain againstForceRealTime, so the re-arm loop terminates.- Both
ForceRealTimesites (DateHeaderTimer::runandupdate_date_header_timer_if_necessary) agree on the clock thatenable()comparesnextagainst — checked that no other reader ofdate_header_timer.event_loop_timer.nextassumes mocked time. - Tests follow the sibling
test-timers-gc-spin-fixture.tspattern exactly: in-process test only assertsgetTimerCount()(can't hang), therunAllTimers()livelock case is spawned with a bounded timeout andsignalCode === nullguard; fixture is.tsnot.test.tsso CI's glob won't run it standalone.
Extended reasoning...
Overview
Three source changes plus tests:
src/event_loop/EventLoopTimer.rs: one line addingTag::DateHeaderTimerto theallow_fake_timers()opt-out list.src/runtime/timer/mod.rs(DateHeaderTimer::run) andsrc/runtime/timer/Timer.rs(update_date_header_timer_if_necessary): swapTimespecMockMode::AllowMockedTime→ForceRealTimefor the re-arm/enable clock reads, each with a comment naming why.src/runtime/timer/DateHeaderTimer.rs: doc-comment only.- Two new tests in
test/js/bun/test/test-timers.test.tsplus a spawned fixturetest-timers-date-header-fixture.ts.
Security risks
None. No user input handling, no auth/crypto, no memory-ownership changes. The diff only touches which intrusive heap an internal bookkeeping timer is inserted into and which monotonic-clock mode it reads.
Level of scrutiny
Low-to-medium. The mechanism is well explained in the PR body and matches the code: All::insert diverts to fake_timers.timers when tag.allow_fake_timers(), and All::next (real-heap drain) already reads ForceRealTime, so moving DateHeaderTimer to the real heap while keeping its own re-arm on the mocked clock would have reproduced the same spin in drain_timers — the two ForceRealTime changes are the necessary companion to the tag change, not incidental. enable() compares the passed now against event_loop_timer.next (written by run()), so both call sites reading the same clock is required for the "> 1s stale" check to be meaningful. The Date header string itself comes from loop_.update_date() (wall-clock), which this PR does not touch, so setSystemTime behavior for the header value is unchanged.
Other factors
- The change is a direct sibling of the existing
GcRepeating/DnsSdConnection/QuicEndpointopt-outs on the same match arm, and the new spawned-fixture test is a near-copy of thetest-timers-gc-spin-fixture.tstest immediately above it — sametimeout: 20_000, samesignalCode === nullhang guard, same stdout-sentinel assertion order. - The in-process test restores real timers in
finallybefore theusing serverdisposal runs, and only assertsgetTimerCount()(norunAllTimers()), so it cannot hang the outer test file even on a regressed build. - The fixture is named
*-fixture.ts(not.test.ts), matching the sibling, so it will not be double-run by CI's test glob. - The PR notes the same bug shape exists for
DNSResolverand explicitly scopes it out; that's a reasonable boundary since it's a different tag with its own re-arm path.
|
Heads up: #37946 turns |
…37946) ### Problem - `jest.useFakeTimers()` captures the runtime's own safety timeouts. With fake timers active, `Bun.spawn({ timeout })` never kills the child, `new SQL({ connectionTimeout })` / `new RedisClient(url, { connectionTimeout })` never time out, and `jest.getTimerCount()` counts these timers as if the test had created them. - `jest.useRealTimers()` (and `jest.clearAllTimers()`) then drains the fake heap and disarms them permanently: a child spawned with `timeout: 200` under fake timers runs to completion after the test is back on real time, and a Postgres connection attempt stays pending forever. The owner still believes it is armed, so nothing re-arms it. - `Bun.spawnSync({ timeout })` under fake timers has the same problem without the heap: its wait loop compared the deadline against the mocked clock, which cannot advance while the call blocks. - Cause: `Tag::allow_fake_timers()` in `src/event_loop/EventLoopTimer.rs` was an opt-out list, so every owner not on it (`SubprocessTimeout`, the Postgres/MySQL/Valkey connection, idle, lifetime and reconnect timers, `UpgradedDuplex`, `WindowsNamedPipe`, `DNSResolver`, `DateHeaderTimer`, the dev server timers) was routed into the fake heap by `All::insert` (`src/runtime/timer/mod.rs`), and each of them computed its deadline with `AllowMockedTime` to match. The fake timers PR left this list as a TODO ("should subprocess timeout? probably not"). - `FakeTimers::clear()` (`src/runtime/test_runner/timers/FakeTimers.rs`) only told `TimeoutObject` and `AbortSignal.timeout()` owners that their timer was gone; every other popped node was just marked `CANCELLED`. ### Fix - `allow_fake_timers()` becomes an allowlist of the timers a program schedules itself: `setTimeout`/`setInterval` (`TimeoutObject`), `AbortSignal.timeout()`, and `Bun.cron()` (documented as honoring fake timers). Everything else stays in the real heap and keeps running on the real clock, which is also what Jest does (its fake timers never reach Node's internal timeouts). - Every owner that moves to the real heap now arms with `ForceRealTime` (subprocess, Postgres, MySQL, Valkey, `UpgradedDuplex`, `WindowsNamedPipe`, c-ares poll timer, Date header timer, dev server sweep / visualizer). This is required, not optional: the real heap is drained against the real clock (`All::next`), so a mocked deadline there would be due immediately. It is the same convention the existing opt-outs (`BunTest`, `GcRepeating`, `StatWatcherScheduler`, `QuicEndpoint`, `DnsSdConnection`) already follow. - `spawnSync` compares its `timeout` against the real clock too (`js_bun_spawn_bindings.rs`, `SpawnSyncEventLoop.rs`), and only consults an `AbortSignal.timeout()` deadline that lives in the real heap; one in the fake heap is on the mocked clock and cannot fire during a blocking call anyway. Behavior without fake timers is unchanged. - `Bun.cron` is now the only non-JS owner the fake heap can drop, so `FakeTimers::clear()` collects dropped jobs and stops them the way `stop()` would (`CronJob::stop_dropped_from_fake_heap`), matching what happens to a dropped `setInterval`. Before, a job created under fake timers and then `useRealTimers()` kept the process alive forever with a timer that could never fire. The fallback arm is now a `debug_assert!`, so adding a tag to the allowlist without a release path fails loudly in debug builds. - Behavior change to be aware of: `advanceTimersByTime()` no longer fires runtime-internal timeouts (e.g. a SQL `connectionTimeout`); they elapse in real time. No test or doc relied on that; the cron tests that do rely on mocking are unaffected. Not changed here: the async `node:child_process` `timeout` option is implemented with a JS `setTimeout` in `src/js/node/child_process.ts`, so it is still faked like any other `setTimeout`. - Verified with `test/js/bun/test/fake-timers/fake-timers.test.ts` (new `runtime timeouts are not fake timers` and `Bun.cron() job dropped from the fake heap` blocks, 9 tests, including `node:child_process.spawnSync({ timeout })`, which hands its timeout to `Bun.spawnSync`). All 9 fail on the unfixed build (`getTimerCount()` is 1 or 2, spawn/spawnSync children exit normally, the cron child hangs until killed) and pass with the fix. - Also passing on the debug build: `test/js/bun/test/fake-timers/sinonjs`, `test-timers.test.ts`, `test/js/bun/cron/in-process-cron.test.ts` and `cron-local-time.test.ts`, `spawn-signal.test.ts` (covers `spawnSync` + `AbortSignal.timeout`), `spawnSync.test.ts`, `spawn-maxbuf.test.ts`, `spawnsync-isolated-event-loop.test.ts`, `spawnsync-no-microtask-drain.test.ts`, `test/js/sql/sql-connect-error-reporting.test.ts`, the fake-timer cases of `test/cli/test/isolation.test.ts`, `test/regression/issue/25869.test.ts`, `26284.test.ts`. `cargo clippy` on `bun_event_loop`, `bun_runtime`, `bun_sql_jsc` is clean, including `--target x86_64-pc-windows-msvc` for the `WindowsNamedPipe` change. - Overlap: #37935 (Date header timer) and #37940 (c-ares poll timer) each add one of these owners to the old opt-out list and flip its clock; this PR contains both of those source changes as part of the allowlist. Their tests still apply on top of this and are worth keeping; whichever lands second needs a small rebase of `allow_fake_timers()`. ### Background - `timer::All` keeps two intrusive heaps of `EventLoopTimer` nodes. The event loop drains `timers` against `CLOCK_MONOTONIC`. While `jest.useFakeTimers()` is active, `All::insert` diverts nodes whose `Tag` allows it into `fake_timers.timers`, which only the `jest.*` functions drain: each pop sets the mocked clock to the node's deadline and fires it; `useRealTimers()` / `clearAllTimers()` pop everything without firing. - `Tag` says which struct embeds a node (a `Subprocess`, a `PostgresSQLConnection`, a `TimeoutObject`, ...); `__bun_fire_timer` in `src/runtime/dispatch.rs` recovers the owner from it. Owners detect their own cancellation by checking `state == ACTIVE` before removing, which is why a foreign cancellation is silent rather than a crash. - `Timespec::now(AllowMockedTime)` returns the fake-timers clock when one is installed; `ForceRealTime` always reads the monotonic clock. A deadline must be computed with the clock of the heap it is inserted into. - `FakeTimers::clear()` returns the owners of the popped nodes so they can be released after the heap borrow ends (`TimeoutObject` drops its heap ref, `AbortSignal.timeout()` is cancelled through its signal, and now a `CronJob` is stopped); a node that is only unlinked leaves its owner holding event-loop refs for a timer that will never fire. - `spawnSync` runs the child on an isolated mini event loop and blocks the JS thread in a tick loop, so no `jest.advanceTimersByTime()` can run while it waits; the only clock that can make its timeout elapse is the real one. <details> <summary>Repro on the released binary (1.4.0)</summary> ```ts import { expect, jest, test } from "bun:test"; test("Bun.spawn timeout armed under fake timers, then useRealTimers()", async () => { jest.useFakeTimers(); const p = Bun.spawn(["sleep", "3"], { timeout: 200, killSignal: "SIGKILL" }); jest.useRealTimers(); await p.exited; expect(p.signalCode).toBe("SIGKILL"); // null: child ran for 3 s }); test("spawnSync timeout under fake timers", () => { jest.useFakeTimers(); const r = Bun.spawnSync(["sleep", "3"], { timeout: 200, killSignal: "SIGKILL" }); expect(r.exitedDueToTimeout).toBe(true); // undefined, and the call blocked for 3 s }); test("cron job dropped by useRealTimers", () => { // as a script: the process never exits, the job holds the loop open with a dead timer jest.useFakeTimers(); Bun.cron("* * * * *", () => {}); jest.useRealTimers(); }); ``` The SQL variant from the report (a `Bun.listen` that accepts and never answers, `connectionTimeout: 1`, let the connection get established, then `useRealTimers()`) stays pending past 4 s; with the fix it rejects with `ERR_POSTGRES_CONNECTION_TIMEOUT` after ~1 s. </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 16 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 9 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/test/fake-timers/fake-timers.test.ts bun test v1.4.0 (ada0dd1) test/js/bun/test/fake-timers/fake-timers.test.ts: (pass) fake timers [34.23ms] (pass) advanceTimersToNextTimer > one setTimeout [11.19ms] (pass) advanceTimersToNextTimer > setInterval [10.59ms] (pass) advanceTimersToNextTimer > sorted timeouts [18.70ms] (pass) advanceTimersToNextTimer > alternating intervals [12.93ms] (pass) advanceTimersByTime > setInterval [9.41ms] (pass) runOnlyPendingTimers > two setIntervals [12.40ms] (pass) runAllTimers > two setIntervals [12.89ms] (pass) getTimerCount > returns correct count of pending timers [10.69ms] (pass) getTimerCount > throws error if fake timers not active [5.09ms] (pass) clearAllTimers > clears all pending timers [6.90ms] (pass) clearAllTimers > throws error if fake timers not active [3.88ms] (pass) AbortSignal.timeout > pending signals stay alive while the fake heap holds their timer [653.89ms] (pass) AbortSignal.timeout > useRealTimers() releases the signals whose timers it dropped [696.70ms] (pass) AbortSign ... (truncated) release without fix: 11 FAILED bun test v1.4.0-canary.1 (da3851e) test/js/bun/test/fake-timers/fake-timers.test.ts: (pass) fake timers [10.73ms] (pass) advanceTimersToNextTimer > one setTimeout [0.29ms] (pass) advanceTimersToNextTimer > setInterval [0.17ms] (pass) advanceTimersToNextTimer > sorted timeouts [0.22ms] (pass) advanceTimersToNextTimer > alternating intervals [0.13ms] (pass) advanceTimersByTime > setInterval [0.15ms] (pass) runOnlyPendingTimers > two setIntervals [0.17ms] (pass) runAllTimers > two setIntervals [0.15ms] (pass) getTimerCount > returns correct count of pending timers [0.12ms] (pass) getTimerCount > throws error if fake timers not active [0.11ms] (pass) clearAllTimers > clears all pending timers [0.10ms] (pass) clearAllTimers > throws error if fake timers not active [0.04ms] (pass) AbortSignal.timeout > pending signals stay alive while the fake heap holds their timer [12.11ms] 238 | test("useRealTimers() releases the signals whose timers it dropped", () => { 239 | const before = liveAbortSignals(); 240 | vi.useFakeTimers(); 241 | leakObservedTimeouts(); 242 | vi.useRealTimers(); 243 | expect(liveAbortSignals() - before).toBeLessThan(N * 0.1); ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/test/fake-timers/fake-timers.test.ts bun test v1.4.0 (ada0dd1) test/js/bun/test/fake-timers/fake-timers.test.ts: (pass) fake timers [27.16ms] (pass) advanceTimersToNextTimer > one setTimeout [7.74ms] (pass) advanceTimersToNextTimer > setInterval [8.05ms] (pass) advanceTimersToNextTimer > sorted timeouts [13.95ms] (pass) advanceTimersToNextTimer > alternating intervals [8.95ms] (pass) advanceTimersByTime > setInterval [7.16ms] (pass) runOnlyPendingTimers > two setIntervals [7.82ms] (pass) runAllTimers > two setIntervals [8.66ms] (pass) getTimerCount > returns correct count of pending timers [7.35ms] (pass) getTimerCount > throws error if fake timers not active [4.26ms] (pass) clearAllTimers > clears all pending timers [4.93ms] (pass) clearAllTimers > throws error if fake timers not active [2.71ms] (pass) AbortSignal.timeout > pending signals stay alive while the fake heap holds their timer [619.24ms] (pass) AbortSignal.timeout > useRealTimers() releases the signals whose timers it dropped [609.59ms] (pass) AbortSignal.tim ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 827ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/1179] fetch hdrhistogram [hdrhistogram] up to date [2/1179] fetch lshpack [lshpack] up to date [3/1179] fetch lolhtml [lolhtml] up to date [4/1179] cc obj/vendor/zlib/deflate_slow.c.o [5/1179] cc obj/vendor/zlib/insert_string_roll.c.o [6/1179] cc obj/vendor/zlib/deflate_quick.c.o [7/1179] cc obj/vendor/zlib/insert_string.c.o [8/1179] cc obj/vendor/zlib/functable.c.o [9/1179] cc obj/vendor/zlib/deflate_rle.c.o [10/1179] cc obj/vendor/zlib/deflate_medium.c.o [11/1179] cc obj/vendor/zlib/uncompr.c.o [12/1179] cc obj/vendor/zlib/deflate_stored.c.o [13/1179] cc obj/vendor/zlib/inftrees.c.o [14/1179] cc obj/vendor/zlib/zutil.c.o [15/1179] cc obj/vendor/zlib/cpu_features.c.o [16/1179] gen generated_host_exports.rs generated_host_exports.rs: 93 exports (host=3, lazy=10, generic=80, rust=0); 239 extern-C blocks audited [16/1179] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Co ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/event_loop/EventLoopTimer.rs | 21 ++- src/event_loop/SpawnSyncEventLoop.rs | 6 +- src/runtime/api/bun/js_bun_spawn_bindings.rs | 13 +- src/runtime/api/cron.rs | 11 ++ src/runtime/bake/dev_server/hmr_socket.rs | 2 +- src/runtime/bake/dev_server/source_map_store.rs | 2 +- src/runtime/dns_jsc/dns.rs | 2 +- src/runtime/socket/UpgradedDuplex.rs | 2 +- src/runtime/socket/WindowsNamedPipe.rs | 2 +- src/runtime/test_runner/timers/FakeTimers.rs | 21 ++- src/runtime/timer/Timer.rs | 2 +- src/runtime/timer/mod.rs | 2 +- src/runtime/valkey_jsc/js_valkey.rs | 2 +- src/sql_jsc/mysql/JSMySQLConnection.rs | 4 +- src/sql_jsc/postgres/PostgresSQLConnection.rs | 4 +- test/js/bun/test/fake-timers/fake-timers.test.ts | 160 +++++++++++++++++++++++ 16 files changed, 225 insertions(+), 31 deletions(-) ``` </details> **gate history** · 1 passed · 1 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/event_loop/EventLoopTimer.rs 1 4 0 src/event_loop/SpawnSyncEventLoop.rs 1 4 0 src/runtime/api/bun/js_bun_spawn_bindings.rs 4 5 0 src/runtime/api/cron.rs 3 2 0 src/runtime/bake/dev_server/hmr_socket.rs 1 1 0 src/runtime/bake/dev_server/source_map_store.rs 2 1 0 src/runtime/dns_jsc/dns.rs 3 1 0 src/runtime/socket/UpgradedDuplex.rs 1 1 0 src/runtime/socket/WindowsNamedPipe.rs 1 1 0 src/runtime/test_runner/timers/FakeTimers.rs 2 7 0 src/runtime/timer/Timer.rs 3 1 0 src/runtime/timer/mod.rs 6 1 0 src/runtime/valkey_jsc/js_valkey.rs 1 1 0 src/sql_jsc/mysql/JSMySQLConnection.rs 1 2 0 src/sql_jsc/postgres/PostgresSQLConnection.rs 3 2 0 test/js/bun/test/fake-timers/fake-timers.test.ts 4 8 0 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Before #37946 the Date header timer was enrolled in jest.useFakeTimers(): accepting one connection under fake timers made jest.getTimerCount() report 1 with no user timers, and jest.runAllTimers() never returned because the timer re-armed itself into the fake heap on every pop while a keep-alive connection was open. #37946 fixed the enrollment; these tests pin the Bun.serve symptom, which its own tests do not exercise.
5f794b8 to
6e9b81e
Compare
|
Updated 8:05 PM PT - Aug 12th, 2026
❌ @robobun, your commit 547d939 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 37935That installs a local version of the PR into your bun-37935 --bun |
robobun
changed the title
There was a problem hiding this comment.
LGTM — test-only regression coverage for the Date header timer under fake timers, mirroring the neighbouring gc-spin fixture test.
What was reviewed
- Both new tests follow the exact shape of the existing
test-timers-gc-spin-fixture.tscase: child spawn withtimeout, concurrent pipe drain,signalCode === nullto prove the child exited on its own. - Checked the in-process test restores global state (
useRealTimers()infinally,using server), usesport: 0/127.0.0.1, and has no sleeps. - Confirmed the assertions are non-vacuous:
getTimerCount() === 0and theRUN_ALL_OKsentinel both fail on the pre-#37946 build per the PR evidence.
Extended reasoning...
Overview
This PR adds two regression tests (and one fixture file) to test/js/bun/test/test-timers.test.ts covering Bun.serve's internal Date header timer under jest.useFakeTimers(). The underlying source fix already landed in #37946; this PR was rebased down to tests only. No production code is touched.
Security risks
None. Test-only change that starts a local Bun.serve on 127.0.0.1 with port: 0 and fetches from it. No external network, no auth/crypto/permissions.
Level of scrutiny
Low. This is additive test coverage that mirrors an existing pattern in the same file line-for-line (test-timers-gc-spin-fixture.ts + the "real timer heap is ticked against the real clock" test). The main concerns for new tests — flakiness, hermeticity, vacuous assertions, resource leaks — are all addressed:
- No
sleep/setTimeoutwaits; the test awaits the actual observable condition (fetch response, child exit). port: 0, loopback-only,using serverandawait using procfor cleanup.jest.useRealTimers()infinallyso fake-timer state cannot leak to later tests in the file.- Child pipes drained concurrently via
Promise.all;signalCode === nullasserts the child was not killed by the 20s spawn timeout (the hang-guard pattern REVIEW.md asks for). - The PR description documents both tests failing on
1.4.0-canary.1and passing on current main, satisfying the "prove it fails for the right reason" bar.
Other factors
The fixture follows the *-fixture.ts naming convention and lives next to the other fake-timers fixture. Comments reference #37946 rather than restating the mechanism at length. The child test takes ~3s in the debug run shown, which is in line with the neighbouring 2.4s gc-spin test and within the file budget.
2 participants
Problem
1.4.0-canary.1, accepting oneBun.serveconnection underjest.useFakeTimers()put the server's internal Date header timer into the fake timer heap:jest.getTimerCount()reported1with no user timers, andjest.runAllTimers()never returned (the timer re-armed itself at mocked now + 1s on every pop while a keep-alive connection was open; a synchronous loop, so the test timeout could not fire).Tag::allow_fake_timers()(src/event_loop/EventLoopTimer.rs) into an allowlist and arming the Date header timer on the real clock, but its tests coverBun.spawn,spawnSync, SQL, Valkey andBun.cron; nothing exercisesBun.serveor therunAllTimers()livelock that was reported.Fix
test/js/bun/test/test-timers.test.ts, next to the existing real-heap-under-fake-timers test:Bun.serve's Date header timer is not enrolled in fake timers: serve,useFakeTimers(), onefetch, thengetTimerCount()must be0.runAllTimers returns while Bun.serve holds a keep-alive connection: runstest-timers-date-header-fixture.tsin a child with a spawn timeout and requires it to exit on its own withRUN_ALL_OK, the same shape as the neighbouringtest-timers-gc-spin-fixture.tstest. The livelock case has to be a child process because a regressed build spins synchronously and would hang the test file.1.4.0-canary.1both fail (getTimerCount()is1; the child spins until killed). On the current main debug build (74c245744, which includes bun:test: keep runtime-internal timeouts out of the fake timer heap #37946) the whole file passes; the child test takes about 2.6s there.Background
timer::Allkeeps two heaps ofEventLoopTimernodes. While fake timers are active,All::insertdiverts nodes whose tag passesallow_fake_timers()into the fake heap, which only thejest.*functions drain; each pop moves the mocked clock to that node's deadline and fires it, andrunAllTimers()pops until the heap is empty, so any node that re-arms itself on every fire makes it loop forever.src/runtime/timer/DateHeaderTimer.rs) refreshes the cachedDate:stringBun.servewrites into responses. usockets arms it when the first socket with an idle timeout appears, and it re-arms itself once per second for as long as such sockets exist, which is why one idle keep-alive connection is enough to trigger the old behaviour.Original version of this PR (superseded by #37946)
The first revision of this PR added
Tag::DateHeaderTimerto what was then an opt-out list inallow_fake_timers()and switched the two clock reads inDateHeaderTimer::runandAll::update_date_header_timer_if_necessaryfromAllowMockedTimetoForceRealTime, with the same two tests. #37946 landed the equivalent source changes for this timer and every other runtime-internal timer at once, so this PR was rebased down to the tests.[stamp-90s] gate passed · iteration 3 · 2 files touched
passes on PR (with fix)
diff hotspot
gate history · 3 passed · 0 rejected · iteration 3
evidence per changed file