timer: remove unsafe from the timer module

Closed
JA
1mo ago
+2038-2594

What

Same programme as #40055 / #40135 / #40136 / #40139, applied to timers. src/runtime/timer/: mod.rs 89 → 0, timer_object_internals.rs 54 → 0, WTFTimer.rs 28 → 0, Timer.rs 15 → 0, ImmediateObject.rs 4 → 0, EventLoopDelayMonitor.rs 5 → 0, DateHeaderTimer.rs 3 → 0 (each of the first three keeps one unsafe extern "C" { safe fn … } decl for a Bun C++ symbol). The unsafe that remains for timers is the tag→container recovery in src/runtime/dispatch.rs and the intrusive heap internals in bun_io/bun_event_loop, which is where it belongs.

How:

  • The heap owns membership. bun_io::heap: links are private Cells, HeapNode::heap(&self), Intrusive is &self-based; only insert/remove stay unsafe fn (liveness). bun_event_loop::TimerHeap wraps it for EventLoopTimer: it is the sole writer of a slot's links and in_heap, so double-insert / remove-from-the-wrong-heap are refused (debug-asserted) instead of corrupting the heap; insert/remove/delete_min/peek/find_max/count/to_vec are safe.
  • EventLoopTimer: tag/heap/in_heap are private (new(tag, state, next), tag(), in_heap()); pub unsafe trait TimerOwner is emitted by impl_timer_owner! (the invocation asserts: slot tagged for its dispatch arm, unlinked before drop/move); TimerRef is the handle timer::All traffics in — built from (&Owner, field accessor) with whole-owner provenance (asserts the slot lies inside the owner), exposes deadline/state only, no Deref, no access to tag or links. EventLoopTimer::fire and the __bun_fire_timer extern are gone; dispatch::fire_timer(TimerRef, ..) is the one container-of site.
  • TimeoutObject / ImmediateObject derive RefCounted, hold the armed-timer ref in heap_ref: Cell<Option<RefPtr<Self>>> (set where ref_() was, released where deref() was), share trait TimerObject, and their host fns that may release refs take this: ThisPtr<Self> (codegen support cherry-picked byte-for-byte from #40139). Maps is typed (IdMap<T> = ArrayHashMap<i32, BackRef<T, Mut>>).
  • timer::All is &self throughout (jsc_hooks::timer_all(); timer_all_mut removed); WTFTimer externs, Bun__Timer__getNextID, Bun__internal_ensureDateHeaderTimerIsEnabled, Timer_{enable,disable}EventLoopDelayMonitoring are HOST_EXPORTs; FakeTimers is &self; bun_libuv_sys::Timer::{get_due_in, update_loop_time}; JsCell::from_mut.

Two pre-existing bugs surfaced by the stricter heap: DevServerMemoryVisualizerTick's fire arm never cleared its slot's state, so the next update() called remove() on an unlinked node (popped an unrelated root in release / asserted in debug) — now a refused no-op; WTFTimer::is_active/seconds_until_timer read the slot unlocked while GC threads write it under the lock — now locked.

Testing

Debug+ASAN: web timers (setTimeout/setInterval/setImmediate/setImmediate2/microtask/clearImmediate-gc/timer-gc-roots/timer-heap-race/performance-entries), js/node/timers, 36 test-timers-*.js node parallel tests, fake-timers (516), in-process-cron, sleep, bun-serve-date, perf_hooks + test-performance-eventloopdelay.js, abort, cli/test/isolation, socket, node-http, fetch, worker, serve — all pass except cases that fail identically on a main debug build (the four RSS-threshold timer "leak" fixtures whose isASAN check keys on the executable name — native TimeoutObject counts verified at 0 after GC; serve uid-0 port case; fetch root-permission cases; the LSAN dispatchAfter suppression that needs a symbolizer; the worker message-flood test whose 30 ms budget is shorter than debug worker startup). Hand-driven: clear/refresh/_repeat-inside-callback, 1e5 immediates, unref exit, Bun.sleep, AbortSignal.timeout, numeric-id clear, Symbol.dispose. clippy clean on every touched crate; rust-check-all windows-msvc + apple-darwin clean.