[robobun]

Problem

• Timer ids come from a 32-bit signed counter that wraps (All.last_id, src/runtime/timer/mod.rs:585). After 2^31 timers an id is negative and clearTimeout("<id>") refuses it: the timer fires.
• After 2^32 a new timer gets a live timer's id: clearInterval(id) stops the new timer, the old one ticks on.
• A collected _repeat timeout then leaves a dangling map entry. ASAN: heap-use-after-free in <bun_runtime::timer::All>::remove_timer_by_id.

Fix

• All::next_id() is the one place that hands out an id, from a u64 that starts at 1. An id is positive and names one timer.
• The id maps, both clearTimeout arms, the inspector async id and the formatters take the u64. Node.js gives the same ids past 2^31.
• Verified: test/js/web/timers/setTimeout.test.js, test/cli/inspect/debugger-timer-async-stack.test.ts. The id cases fail on main plus the hook, the inspector case with a 32-bit key. Also fake timers, async_hooks, Node's 55 test-timers-*.
• Self-reviewed: 4 concerns raised, 4 addressed.

Background

• +timer gives the id. Its first read puts the timer in a map for clearTimeout(id). Unread timers have no entry.
• Found by fuzzing, no report. Reach: 2^31 timers on one thread, 25 days at 1,000 per second.
• Considered wrap to 1, skipping ids in the maps: an unread live timer is not there, so ids repeat.
• Overlaps socket, timer, sql, redis: replace unsafe with typed ownership #40139 and fix(node:async_hooks): report timer lifecycles #42621: see Notes.

Downsides

• An id kept in 32 bits (wasm i32, id | 0) clears nothing past 2^31. Main honoured it. Node.js does not.
• No release-mode uniqueness check: put_no_clobber asserts in debug builds.
• Release builds: id draw 3 instructions and timer 96 bytes as before, map entry 16 to 20 bytes.

Notes

Related open PRs

• socket, timer, sql, redis: replace unsafe with typed ownership #40139 (socket, timer, sql, redis: typed ownership) rewrites src/runtime/timer. Its All::next_id() has the same name and place as the one here, with a Cell<i32> and wrapping_add(1), and its IdMap is keyed by i32. Both PRs edit Timer.rs, mod.rs, timer_object_internals.rs and jsc_hooks.rs. Proposal: this PR first. The port onto socket, timer, sql, redis: replace unsafe with typed ownership #40139 is then a type change that the compiler guides (Cell<i32> to Cell<u64> in next_id(), the IdMap key, timer_id_from_number), and the two new tests pin it. If socket, timer, sql, redis: replace unsafe with typed ownership #40139 goes first, I redo the widening on top of it. The hook and the tests carry over.
• fix(node:async_hooks): report timer lifecycles #42621 (async_hooks timer lifecycles) adds a second 64-bit counter (last_async_hooks_id) and a per-timer async_hooks_id: Cell<u64>. In Node.js the timer id is the async id (Timeout.prototype[Symbol.toPrimitive] returns this[async_id_symbol]). With the id of this PR, fix(node:async_hooks): report timer lifecycles #42621 can draw from All::next_id() and add no field. As written, its field makes TimerObjectInternals 48 bytes and fails the == 40 assert here. That assert is on purpose: a timer then moves from the 96-byte to the 112-byte allocator class. One counter or two is a decision for the maintainers and for fix(node:async_hooks): report timer lifecycles #42621.
• Smaller overlaps with robobun PRs: timers: keep the async context on the Timeout, not in the callback slot #41194 and node:http: measure the keep-alive idle period on the monotonic clock #37974 (Timer.rs), sql: measure the connect-retry budget on the monotonic clock #37960 and bun:test: keep built-in modules' own timers out of fake timers (internal/timers) #37987 (internal-for-testing.ts), Make the allocation-only collection APIs infallible and drop the OOM wrappers around them #38888 (timer_object_internals.rs), Remove dead code from the inspector protocol package, bun_jsc, bun_install, and the Windows named pipe modules #40557 (also deletes Bun__Timer__getNextID).
• Read timer ids, the vm module timeout and the inspect depth by value, not by JSValue representation #39991 wrote "the int32 range is the whole id space" and matched the number arm by an i32 round trip. This PR retires that assumption and rewrites its timer_id_from_number.

Proof on unfixed builds

Real counts, release 1.4.3-canary.1+367d939d9, no hook:

2^31 timers: the key of a fresh timer is negative; cleared by its key (a string) it FIRED; by its number: did not fire; by the object: did not fire
2^32 timers: ids 1 and 1. the second interval has THE SAME id as the first; after clearInterval(id): the first STILL TICKS, the second STOPPED

855 s and 1611 s on this machine. The same two scripts on the release build of this PR (the wait for a negative id bounded, because it never comes):

2^31 timers: the key of a fresh timer is positive; cleared by its key (a string) it did not fire; by its number: did not fire; by the object: did not fire
2^32 timers: ids 1 and 4294967297. the second interval has another id; after clearInterval(id): the first stopped, the second still ticks

An earlier run of the unbounded scripts on this PR's build went on for 6 CPU hours each (about 4 x 10^10 timers, ids past 2^35) with RSS at 98 MB and 90 MB.

Main plus only the test hook (debug, ASAN), the cases that are now in timer-id-fixture.js:

ids past 2^31:   [2147483647, -2147483648, -2147483647, -2147483646, -2147483645], keys ["2147483647", "-2147483648"], fired ["byKeyPastInt32", "kept"], inspect "Timeout (#-2147483645)"
ids past 2^32:   [-1, 0], fires [1, 2]          (refresh() ignores the timer with id -1)
second interval: the same id, clearInterval(id) stops the second, the first ticks on
freed promoted timeout:
==15943==ERROR: AddressSanitizer: heap-use-after-free
READ of size 1
    #0 <bun_runtime::timer::All>::remove_timer_by_id src/runtime/timer/Timer.rs:313:32
    #1 <bun_runtime::timer::All>::clear_timer src/runtime/timer/Timer.rs:344:35
    #2 <bun_runtime::timer::All>::clear_timeout src/runtime/timer/Timer.rs:444:9
freed by: TimeoutClass__finalize -> RefCount::deref -> Box<TimeoutObject>::drop

The use after free needs three things: a timeout promoted by _repeat, a read of its id, and 2^32 timers before another timer reads its id.

The released build fails the new tests with TypeError: timerInternals.setNextTimerId is not a function, so the runs above are the before and after evidence.

With the inspector key cut back to 32 bits of the id (what main has), debugger-timer-async-stack.test.ts fails: the callback of the earlier timer gets the async stack trace of the timer 2^32 ids later (earlyTimeoutCb: "scheduleLateTimeout").

Node v26.3.0, with its async id counter moved the same way, and this PR print the same JSON:

{"2^31":{"ids":[2147483647,2147483648,2147483649],"clearedByKey":[true,true,true]},"2^32":{"ids":[4294967295,4294967296,4294967297],"clearedByKey":[true,true,true]}}

Reach

Found by fuzzing: an automated run made 2^31 and 2^32 timers. No issue reports a negative or repeated timer id. Bun.serve with fetch draws no timer id per request (0 in 300 requests, also with a node:http server). It takes 2^31 calls of setTimeout, setInterval, setImmediate or Bun.sleep on one thread: 14 minutes of a tight loop on this machine (855 s), 2.5 days at 10,000 per second, 25 days at 1,000 per second. The program must also clear by string key, test the sign of an id, or keep a numeric id long enough.

A JS number holds an id exactly up to 2^53. That is 114 years of the same tight loop.

What a script can see

• Ids below 2^31 do not change.
• From the 2^31-th id on, +timer is 2147483648, 2147483649 and so on, where main gave negative numbers.
• clearTimeout(id) and clearInterval(id) take an integer from 1 to 2^53 - 1, as a number or as a digit string. A larger value clears nothing in either form, because a number does not hold such an id exactly. Main took any int32, negative and zero too. A caller that carries the id in 32 bits (wasm glue that types the id as i32, id | 0) gets -2147483648 for id 2147483648 and clears nothing. Checked here on this PR and on Node v26.3.0: {"id":2147483648,"handle":-2147483648,"clearedByHandle":false} on both.
• refresh() works on the timer that main numbers -1.

Measurements

Release builds. Base: 367d939 (its timer sources differ from the merge base in visibility modifiers only). PR: the first commit of this branch (later changes are comments and tests). gdb single-step over the host function, 5 consecutive calls, the 5 counts identical each time. A call into libc or the vDSO counts as one.

	base	PR
id draw in setTimeout, setInterval, setImmediate, sleep	3 instructions, no compare	3 instructions, no compare
static instructions of the four create functions	104 / 104 / 143 / 115	104 / 104 / 136 / 115
their bytes	407 / 410 / 599 / 455	407 / 410 / 566 / 456
setTimeout(f, 1e9)	716	708
of which timer functions	478	477
clearTimeout(timer)	287	261
of which timer functions	183	183
setImmediate(f)	423	413
clearTimeout(+t), hit	381	357
clearTimeout(123456), miss	106	108
clearTimeout(2 ** 40), miss	64	111
clearTimeout("7"), miss	178	177
clearTimeout("1234567890"), miss	287	315
+t, first read	194	156
+t, repeat read	69	40
malloc size of a Timeout and of an Immediate	0x60	0x60
TimerObjectInternals::fire, static	509 instructions, 2035 B	500 instructions, 1970 B
__bun_run_immediate_task, static	282 instructions, 1183 B	287 instructions, 1186 B
timer symbols (63 and 66)	23,062 B	23,074 B
map index rebuilds in a GC of 20,000 / 40,000 id-read timers	3 / 3	3 / 3

The draw before: mov 0x198(%r15),%r13d; lea 0x1(%r13),%eax; mov %eax,0x198(%r15). After: mov 0x180(%r15),%r13; lea 0x1(%r13),%rax; mov %rax,0x180(%r15).

The differences outside timer functions (Bun::StrongRootBlock::acquire, JsRef::downgrade) come from changes on main between the two base commits. clearTimeout(2 ** 40) costs more because that number can now be an id, so the maps are read. A string id costs about 3 more instructions per digit. The second commit adds one compare of the parsed value with the bound to the string arm. The two string rows were measured before it. The +12 bytes of timer symbols include the 169-byte test hook.

An id map entry is key, value and a u32 hash: 16 bytes before, 20 after. Only a timer whose id was read has one. TimerObjectInternals is 40 bytes before and after (const assert). timer::All (one per thread) is 464 bytes in the debug build of this PR. The base was not measured. It is at most 8 bytes smaller.

Not in this PR

These stay as they are. None is the negative or repeated id.

1. clearImmediate(number | string) cancels the timeout or interval with that id, because the number and string arms of clear_timer do not test kind. Node.js v26.3.0 leaves the timer alone. socket, timer, sql, redis: replace unsafe with typed ownership #40139 fixes it.
2. Immediates register in maps.set_immediate, which no clear path reads. +immediate is NaN in Node.js. socket, timer, sql, redis: replace unsafe with typed ownership #40139 removes the map.
3. deinit and remove_timer_by_id remove by key and do not compare the stored pointer. Unique ids are the only guard.
4. fire() promotes a _repeat timeout that was cleared inside its own callback. Node.js fires once. Bun fires again, and a clear by id leaves an interval that never stops. A separate fix is in progress.
5. The id is drawn before the delay is coerced. Node.js draws after.

The same shape elsewhere, not changed here: the fetch request id is an AtomicU32 (src/http/lib.rs), and #37437 widens it. #44767 does the same for the AbortSignal algorithm identifier. The node:cluster primary seq: i32 is not affected, because one callback per worker is in flight. The ScriptExecutionContext identifier is an unsigned.

Tests

• One child runs timer-id-fixture.js for all id cases: timerInternals.setNextTimerId moves the counter of its own thread, and only forward. It takes about 3 s on a debug build, most of it to load bun:internal-for-testing (internal-for-testing: make exposedInternals lazy so loading the module stays cheap #37494 makes that load cheaper). About 0.1 s on a release build.
• Suites run on the release build of this PR: test/js/web/timers/ (all files), test/js/node/timers/, test/js/bun/util/inspect.test.js, test/js/bun/test/fake-timers/fake-timers.test.ts, test/js/bun/test/test-timers.test.ts, test/js/node/async_hooks/, and Node's test-timers-* (55 parallel, 2 sequential). All pass.
• On the debug ASAN build: the two new tests, the id tests in setTimeout.test.js and node-timers.test.ts, fake timers, inspect.test.js, and ten of Node's timer tests pass. Some older tests in the timer files time out or report RSS growth on a debug build in this container. They pass on the release build of this PR.
• Checked by hand: a Worker counts its own ids ([2147483647, 2147483648] in the worker, 1 and 2 on the main thread).
• The fixture does not assert that the promoted timeout was collected. On the macOS aarch64 and Ubuntu 25.04 aarch64 lanes it was not collected within 20 collections. The reason is not known. The case gives the same result either way. The ASAN proof above is from linux x64, where it was collected.
• The fixture has a case for the upper bound: with ids 2^53 - 1 and 2^53, neither the number nor the string of the second one clears it. With the bound removed from the string arm, that case fails (pastIsAlive: false).

---

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/timers/setTimeout.test.js

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 13 minutes.

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: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: f2cc2ff3-e7ff-4c5e-8afd-866425f079b8

📥 Commits

Reviewing files that changed from the base of the PR and between a407eae and 52097d5.

📒 Files selected for processing: 13

• src/event_loop/EventLoopTimer.rs
• src/js/internal-for-testing.ts
• src/jsc/bindings/headers.h
• src/runtime/jsc_hooks.rs
• src/runtime/test_runner/pretty_format.rs
• src/runtime/timer/ImmediateObject.rs
• src/runtime/timer/TimeoutObject.rs
• src/runtime/timer/Timer.rs
• src/runtime/timer/mod.rs
• src/runtime/timer/timer_object_internals.rs
• test/cli/inspect/debugger-timer-async-stack.test.ts
• test/js/web/timers/setTimeout.test.js
• test/js/web/timers/timer-id-fixture.js

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[robobun]

Status

How the bug was reproduced:

• Release build 1.4.3-canary.1+367d939d9, no test hook. A script made and cleared 2^31 timers, then ran for (const key in timers) clearTimeout(key). The cleared timer fired (855 s). After 2^32 timers two live intervals had id 1, and clearInterval(1) stopped the new one while the old one ticked on (1611 s).
• Main plus only the test hook, debug ASAN build. The id cases that are now in test/js/web/timers/timer-id-fixture.js failed, and a collected _repeat timeout gave heap-use-after-free in <bun_runtime::timer::All>::remove_timer_by_id.

With this PR the same two real runs print RESULT: ok (ids 1 and 4294967297), and the two new tests pass on the debug ASAN build and on the release build.

One question for a maintainer is open: the landing order against #40139. The description has the details under "Related open PRs".

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked two layout concerns and ruled them out: nothing in src/jsc, src/js or the C++/Zig side reads TimerObjectInternals or the ID struct by layout, so the field reorder and the dropped #[repr(C)] have no cross-language consumer; and the new (id << 2) | kind async-id key is sound because KindBig has only values 0..2, and every consumer of the key is the Rust inspector plumbing in timer_object_internals.rs that goes through ID::async_id().

Extended reasoning...

The change widens the per-thread timer id from i32 to u64 across src/runtime/timer (allocator, id maps, string/number id parsers, inspector async-id key, formatters) and adds a test-only setNextTimerId hook plus two new tests; it touches no auth, crypto or injection surface. Inline findings are already posted, and the one real one (number vs string arm diverging above 2^53) plus the breadth of the type change warrant a human look.

One part of this review's analysis that stopped early was run once more. It finished.

[robobun]

^{Updated 8:05 AM PT - Oct 15th, 2026}

✅ @robobun, your commit 52097d513e6b004d8ba0aa19e34ba3d192f58c0b passed in Build #125924! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 44771

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

bun-44771 --bun

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

The two follow-up commits cover the points from my earlier pass (one MAX_ID bound now applies to both the number and string arms of clearTimeout, and the per-test timeout is gone). Beyond the inline note, I also checked: no remaining callers of the deleted Bun__Timer__getNextID in C++ or Rust; every inspector async-id consumer goes through the single ID::async_id, so the key format change is consistent; All starts last_id at 1 and setNextTimerId is lower-bounded by the current counter, so the default id: 0 is never handed out and dropping the old -1 guard in refresh() loses no case.

Extended reasoning...

The change widens the JS-visible timer id from i32 to u64 across src/runtime/timer (mod.rs, Timer.rs, timer_object_internals.rs), the inspector async-id key, the debug formatter in jsc_hooks.rs, and adds a bun:internal-for-testing hook plus two tests; it touches no auth, crypto, or injection surface. It reorders a repr(C) struct and changes the inspector key format, and one verified finding is not being posted, so a human look is still worthwhile rather than an approval.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 src/runtime/timer/Timer.rs — nit, pre-existing: a program that calls clearImmediate with a number or digit string cancels an unrelated setTimeout or setInterval whose id matches, where Node.js clears no timer. In src/runtime/timer/Timer.rs:323-334 the number arm (and the string arm at 335-394) call remove_timer_by_id regardless of kind, so clearImmediate(5) removes timeout 5 from the id map and cancels it; only the object arm at 397-404 checks kind != Kind::SetImmediate. Fix: make every id form a no-op for clearImmediate, e.g. return Ok(()) at the top of both the number and string arms when kind == Kind::SetImmediate, while still letting clearTimeout and clearInterval clear either a timeout or an interval by id as Node does.

  Why this was flagged

  Trigger: a script does const t = setTimeout(cb, 1000); const n = +t; clearImmediate(n); (or clearImmediate(String(n))), reached through the C++ host function in src/jsc/bindings/node/NodeTimers.cpp:191 which forwards to Bun__Timer__clearImmediate and then All::clear_timer with kind = Kind::SetImmediate. In clear_timer, src/runtime/timer/Timer.rs:323-334, the number arm converts the value with timer_id_from_number and calls all.remove_timer_by_id at line 330, which searches maps.set_timeout and maps.set_interval (lines 291-296) and removes the entry; the string arm at lines 335-394 does the same at line 390. The found TimeoutObject is then cancelled at line 420, so the user's setTimeout never fires. Node.js never looks a numeric id up for clearImmediate, so cb runs there. The kind guard that prevents this for the object form exists only in the object arm at lines 397-404; the number and string arms have no such check. The base commit has the identical arms (base Timer.rs:337-348), so this is pre-existing; the diff rewrites these lines without adding the guard.

  Verification: pre-existing (nit). Trigger: a script calls clearImmediate(n) where n is the numeric id of a live setTimeout/setInterval. In clear_timer (src/runtime/timer/Timer.rs) the number arm (lines 323-334) calls remove_timer_by_id at line 330 and the string arm does the same at line 390; neither consults kind. cancel(vm) runs at line 420. Only the object arm (lines 397-404) has the guard. The base has the same structure.

[robobun]

The clearImmediate(number | string) finding is correct, and it is older than this PR. Node.js v26.3.0 leaves the timeout or interval alone. Bun cancels it.

It is item 1 under "Not in this PR" in the description. #40139 already returns early in clear_timer when kind == Kind::SetImmediate and the argument is not an object. So this PR does not add a second fix: it stays on the id change, and the two PRs get no extra conflict in that function.

If a maintainer wants the guard here, I can add it with a test.

[Jarred-Sumner]

This is Claude (an AI assistant) commenting on Jarred's behalf.

This fix went in with #46021, which is merged. Closing.