[robobun]

Problem

• bun test --parallel (2+ files, so the process-pool coordinator is in use) takes up to a second to act on Ctrl+C / SIGTERM once every worker is in the middle of a file: measured 950-970 ms from kill to exit, 5/5 runs, with the current release build.
• The handler installed by abort_handler::install (src/runtime/cli/test/parallel/Coordinator.rs) only stores SHOULD_ABORT. Coordinator::drive reads the flag between polls, and while it is parked in auto_tick() nothing reads it.
• The signal itself does not end the poll either: bun_epoll_pwait2 / bun_kevent64_wait (packages/bun-usockets/src/eventing/epoll_kqueue.c) retry on EINTR for the remaining timeout.
• So the flag is only seen when something else wakes the loop. With workers mid-file there is no I/O; what actually wakes it is the coordinator's housekeeping: the GC repeating timer and the Date header timer (armed because the worker IPC channels are usockets sockets), both on a 1 s period. Hence 0-1 s of latency depending on where in that period the signal lands. The Windows console control handler has the same shape (flag only).

Fix

• abort_handler::install now takes the coordinator's uws loop and publishes it in an AtomicPtr; the POSIX signal handler and the Windows control handler set the flag and then call us_wakeup_loop on it, so the poll drive() is parked in returns at once and the existing flag check runs. uninstall clears the pointer.
• us_wakeup_loop is an atomic increment plus an eventfd write (Linux) / mach_msg (macOS) / kevent NOTE_TRIGGER (FreeBSD) / uv_async_send (Windows): no allocation, no locks, so it is fine in signal context. It is the same primitive process.on(<signal>) delivery (PosixSignalHandle::enqueue) and every other cross-thread wakeup of this loop already use, called through the raw extern as HTTPThread::wakeup and MiniEventLoop::wakeup do, so no &mut Loop is formed while the main thread is inside the tick.
• No lost-wakeup window: a signal that lands between drive()'s flag check and the poll still wakes the poll, because the eventfd / mach port / async is already signalled when the poll is entered (and pending_wakeups makes the kqueue path poll non-blocking).
• ensure_waker() moves ahead of the handler install in runner.rs, since it is what sets up the loop pointer the handler needs; it used to run a few lines later, before drive().
• Verified: test/cli/test/parallel.test.ts, "coordinator acts on SIGTERM/SIGINT immediately while every worker is mid-file". Without the src change it fails with ~901 ms (release) / ~935 ms (debug ASAN); with it the coordinator reports the interruption within a few ms (about 1 ms measured, ~35-65 ms to full exit under ASAN). The existing "SIGTERM on coordinator kills workers and their grandchildren" test still passes.
• Windows, verified by hand since CI cannot synthesize console control events (see the details block): CTRL_BREAK_EVENT aimed at the coordinator's process group, workers keeping themselves alive with a SIGBREAK listener. Current canary reports the interruption after 929 / 931 / 530 ms depending on when the event is sent; this branch's debug build after 1 ms in every run.
• The test measures the time until the coordinator's "Interrupted" report rather than until exit so ASAN process teardown stays out of the number, and on Linux it first waits for one housekeeping burst to pass (visible as the coordinator main thread's voluntary_ctxt_switches moving, then 100 ms of quiet) so the signal is sent at the start of the quiet period. That turns the unfixed 0-1 s into a reliable ~900 ms instead of a value that passes the assertion some of the time; the fixed coordinator reacts immediately regardless of phase, so the alignment only matters for fail-before. Skipped on Windows, where proc.kill is TerminateProcess and never reaches the control handler.
• Scope: bun run --filter (src/runtime/cli/filter_run.rs) and bun run --parallel/--sequential (src/runtime/cli/multi_run.rs) carry their own copies of this flag-only handler and have the same defect in a worse form (their loop has no timers at all, so a SIGINT sent to the parent alone is ignored until a child exits; a terminal Ctrl+C still works there because it also hits the children). Their installers differ on purpose (SIGINT only, SA_RESETHAND so a second Ctrl+C kills immediately during the graceful abort), so this PR stays with the coordinator and the two bun run sites are tracked separately rather than folded into a shared module.

Background

• Coordinator: with --parallel and 2+ files, the bun test process does not run tests itself; it spawns worker processes, hands them files over an IPC socketpair and streams their results. Its main loop (Coordinator::drive) is: check SHOULD_ABORT, then block in the event loop (auto_tick, i.e. epoll_pwait2 / kevent64 / uv_run with the next timer deadline as the timeout) until I/O or a timer.
• us_wakeup_loop: usockets' cross-thread "make the loop's poll return" primitive. Every loop owns a wakeup async (an eventfd on Linux, a mach port on macOS, an EVFILT_USER on FreeBSD, a uv_async_t on Windows) registered in its poll set; signalling it makes the blocked poll return, after which the caller's own loop logic runs again.
• Housekeeping timers: the GC controller re-arms a timer every 1 s (30 s once the heap has been idle for 30 ticks), and the Date header timer re-arms every 1 s while any usockets socket is linked to the loop. Neither has anything to do with signals; they are just what happened to bound the latency before.

Measurements

Release 1.4.0 canary, coordinator + 2 workers parked in Bun.sleep(60000), time from proc.kill to exit:

SIGTERM: exited 130 after 966ms / 950ms / 972ms
SIGINT:  exited 130 after 966ms / 968ms
BUN_GC_TIMER_INTERVAL=100:            59ms   (GC timer period now bounds it)
BUN_GC_TIMER_INTERVAL=4000:          962ms   (Date header timer still does)
BUN_GC_TIMER_DISABLE=1, 4s in:       937ms

Coordinator main-thread wakeups sampled via /proc/<pid>/status while parked (release build): one burst at +974 ms, +1975 ms, +2976 ms, nothing in between. gdb on the debug build shows the timers firing in those bursts are DateHeaderTimer and GcRepeating.

Fixed debug ASAN build, time to first stderr byte ("Interrupted ...") vs exit: 0.4 / 1.0 / 0.8 ms and 40 / 35 / 65 ms. Under a 2x CPU oversubscription the exit figure went up to 350 ms, which is why the test measures the report and not the exit.

Windows (Server 2019 x64). A Python driver starts the coordinator with CREATE_NEW_PROCESS_GROUP, waits for both workers to be inside their test (each test file does process.on("SIGBREAK", () => {}) so the workers, which share the group, survive the event), sends CTRL_BREAK_EVENT to the group and times the first stderr byte and the exit:

canary bun 1.4.0:          report after 929ms, exit 130 after 935ms
                           report after 931ms, exit 130 after 936ms
  (sent 400ms later)       report after 530ms, exit 130 after 535ms
this branch, debug build:  report after 1ms, exit 130 after 14ms   (x4, including runs sent 400ms and 700ms later)

[robobun]

^{Updated 12:05 PM PT - Aug 15th, 2026}

❌ @robobun, your commit 7988c6b has some failures in Build #98114 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 39039

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

bun-39039 --bun

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 25 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: adaf6e42-8a75-4474-b6ea-ecdc246c6c55

📥 Commits

Reviewing files that changed from the base of the PR and between 88a6398 and 7988c6b.

📒 Files selected for processing (3)

• src/runtime/cli/test/parallel/Coordinator.rs
• src/runtime/cli/test/parallel/runner.rs
• test/cli/test/parallel.test.ts

---

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

[robobun]

Status: reproduced and fixed, waiting on CI and a maintainer.

• Reproduced with the release build: coordinator + 2 workers parked in Bun.sleep, kill -TERM / kill -INT on the coordinator, exit came 950-970 ms later in every run; sampling the coordinator's wakeups showed exactly one housekeeping burst per second and nothing else, so the latency is "time until the next burst".
• Fix in this PR: the abort handler wakes the coordinator's loop (us_wakeup_loop) after setting the flag, on POSIX and in the Windows control handler.
• Fail-before / pass-after with test/cli/test/parallel.test.ts ("coordinator acts on SIGTERM/SIGINT immediately ..."): ~900 ms without the src change, ~1 ms with it.
• Windows checked by hand (CTRL_BREAK_EVENT at the coordinator's process group): canary reacts after 530-931 ms depending on timing, this branch's build after 1 ms; numbers in the PR body.
• Review feedback so far is addressed (explicit test timeout matching the sibling test, shorter comments, errno question answered below). The same flag-only handler also exists in bun run --filter and bun run --parallel/--sequential, where a SIGINT aimed at the parent alone is ignored until a child exits; that is being handled separately, see the Scope bullet in the description.
• Four unrelated tests in that file (lazily scales workers, partitions by directory, work-stealing balances, forwards --experimental-http2-fetch) time out locally under the debug+ASAN build both with and without this change; they pass in CI and are not touched by it.

[claude]

Beyond the inline nit, I checked async-signal-safety of us_wakeup_loop — on the epoll path it's an atomic add plus write(eventfd), and the same primitive is already called from signal context by PosixSignalHandle::enqueue → EventLoop::wakeup, so this follows established precedent. Also checked: no lost-wakeup window (flag is stored before the wakeup, eventfd/mach-port stays signalled if the poll hasn't been entered yet), and the ensure_waker() reordering is required on Windows since usockets_loop() panics before it runs.

Extended reasoning...

Verified the signal-context call path against the existing PosixSignalHandle::enqueue pattern (src/jsc/PosixSignalHandle.rs:67 → EventLoop::wakeup → us_wakeup_loop), confirmed us_internal_async_wakeup on Linux/macOS/FreeBSD is a raw syscall with no allocation or locks, and confirmed usockets_loop() requires ensure_waker() to have run first on Windows (src/jsc/event_loop.rs:642). The uninstall path clears LOOP after restoring the previous sigaction, so a late signal either reaches the restored handler or finds a still-valid loop pointer. Nothing beyond the timeout nit surfaced.

[claude]

I reviewed this PR and didn't find any bugs. The change is small and follows the existing AtomicPtr + raw us_wakeup_loop pattern used elsewhere (e.g. js_bundle_completion_task.rs, MiniEventLoop::wakeup), and the earlier timeout feedback has been addressed. Because it modifies a signal handler with async-signal-safety reasoning across three platform backends (eventfd write / mach_msg / kevent64 / uv_async_send), and the Windows path was only hand-verified, a human look would still be worthwhile.

What was reviewed:

• LOOP store/clear ordering vs. sigaction install/restore — pointer is published before the handler is installed and cleared after it is removed, so no window where the handler runs with a stale/null pointer that matters.
• us_wakeup_loop implementation on each backend — atomic increment + write() to eventfd (Linux), mach_msg (macOS), kevent64 NOTE_TRIGGER (FreeBSD), uv_async_send (Windows, from the console-control thread, not signal context); no allocation or locks.
• ensure_waker() reorder in runner.rs — needed so usockets_loop() returns non-null before install(); the old call site is removed so it still runs exactly once.
• New test — fail-before condition made deterministic on Linux via /proc/<pid>/status context-switch polling; 15000 ms timeout now matches the sibling signal test.

Extended reasoning...

Overview

This PR makes bun test --parallel's coordinator react to SIGINT/SIGTERM immediately instead of waiting up to ~1s for the next housekeeping timer. Three files touched: Coordinator.rs (the abort_handler module now stores the uws loop pointer in an AtomicPtr and calls the raw us_wakeup_loop extern from the signal handler / Windows console-control handler after setting the flag), runner.rs (moves ensure_waker() a few lines earlier and passes the loop pointer to install()), and parallel.test.ts (a new .concurrent.each test over SIGTERM/SIGINT that times how long the coordinator takes to print its "Interrupted" line).

Security risks

None. This is test-runner shutdown handling; no untrusted input, no auth/crypto/permissions surface. The only correctness-sensitive aspect is async-signal-safety.

Level of scrutiny

Medium-high. The diff is small (~30 lines of Rust plus a test), but it changes what runs in POSIX signal context. I verified: (1) us_wakeup_loop on Linux is an atomic add plus write() to an eventfd — write() is async-signal-safe, and the eventfd write essentially always succeeds so the errno-checking retry loop is not reached in practice; on macOS it is mach_msg; on FreeBSD it is a kevent64 NOTE_TRIGGER submit; on Windows the handler runs on a dedicated console-control thread so signal-safety does not apply and uv_async_send is the documented thread-safe primitive. (2) The AtomicPtr<Loop> + raw-extern pattern is exactly what js_bundle_completion_task.rs, HTTPThread::wakeup, and MiniEventLoop::wakeup already use, and bun_uws::us_wakeup_loop is publicly re-exported for exactly this reason (to avoid forming &mut Loop off the owning thread). (3) The LOOP.store in install() happens before sigaction(), and uninstall() restores sigaction() before clearing LOOP, so the handler never observes a torn state; the null check makes a race benign anyway. (4) The ensure_waker() reorder is required because usockets_loop() panics on null; the old call site is removed so it still runs once.

One minor point I considered but did not flag: the Linux eventfd path in us_internal_async_wakeup reads errno on the (essentially unreachable) EINTR/EAGAIN branch, and the handler does not save/restore errno. Since this is a terminal shutdown path (the process exits with 130 on the very next tick) and the eventfd write of a single 8-byte value never fails in practice, this is not a real concern here.

Other factors

• The earlier inline comment about the missing per-test timeout was addressed in 4a6fc0f (15000 ms, matching the sibling test).
• The comment-cop bot's complaints about the module doc were addressed in 7988c6b (trimmed to four lines, shorter than what it replaces).
• The test uses the file's established conventions (tempDir, bunEnv spread, PID file polling like the sibling test, test.concurrent.each, skipIf(isWindows)). It measures time-to-first-stderr-byte rather than time-to-exit so ASAN teardown stays out of the number, and the waitForCoordinatorToPark helper makes fail-before deterministic on Linux by aligning the signal to the start of a quiet period.
• The Windows path is exercised only by hand (CTRL_BREAK_EVENT via a Python driver, per the PR body); CI cannot synthesize console control events. Given that the Windows change is one line (SHOULD_ABORT.store → request_abort()) and uv_async_send from a non-main thread is the standard libuv wakeup, this seems low-risk, but a maintainer familiar with the Windows CI should confirm.

Deferring rather than approving because signal-handler changes across three eventing backends are the kind of thing a maintainer who knows usockets internals should sign off on, even when the diff is small and the reasoning checks out.

[robobun]

On the errno question from the review: the handler leaves errno alone on purpose. None of the wakeup backends touch it on the path that actually runs (the eventfd write cannot block or be interrupted, so its retry branch is unreachable; mach_msg does not use errno; kevent only sets it on failure), and the existing signal handlers in the tree (Bun__onPosixSignal, the SIGCHLD waiter wakeup) follow the same convention, so adding per-platform errno save/restore here would be code for a path that cannot be reached.