Skip to content

spawnSync: release polls and keep-alives on the loop they counted on - #40078

Open
robobun wants to merge 17 commits into
mainfrom
farm/7d1c0396/spawnsync-isolated-loop-poll-count
Open

robobun wants to merge 17 commits into
mainfrom
farm/7d1c0396/spawnsync-isolated-loop-poll-count

Conversation

@robobun

@robobun robobun commented Aug 22, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #34069

Problem

Fix

  • Each owner records one bit when it takes a counter: the isolated loop was current (Flags::SpawnSyncLoop, KeepAlive::spawn_sync_loop, two bools in timer::All). EventLoopCtx::loop_for resolves the loop from that bit at release.
  • FilePoll::register/unregister and the timer ref functions take the EventLoopCtx, not a loop pointer.
  • Verified: one new case each in spawnsync-isolated-event-loop.test.ts and spawnSync.test.ts, on every Linux lane. Both fail on main dc3660d (spin, EEXIST).

Background

Downsides

  • KeepAlive grows from 1 to 2 bytes. FilePoll stays 40 bytes.
  • FilePoll::init and KeepAlive::ref_ do 2 more loop-pointer lookups each. ref_/unref make one call that main inlined. No allocation or syscall.
  • Release .text on linux-x64: 6,656 bytes smaller than main.
Notes

Merge with main (ba3f27d, 2026-09-30), in two merge commits (bb97ecd, 0dedb9f), followed by 87f1190 (see Costs). Four files conflicted. ParentDeathWatchdog.rs: main renamed file_poll::Pollable to file_poll::Flags, and the register call keeps this branch's EventLoopCtx argument. WindowsNamedPipe.rs: main replaced the impl_streaming_writer_parent! invocation with hand-written Windows impls, so main's file is taken whole and this PR no longer touches it. timer/mod.rs: main removed a clippy allow above increment_timer_ref and increment_immediate_ref, and this branch's signatures stay. posix_event_loop.rs: #43868 added Flags::Tty at the end of the enum where this branch added Flags::SpawnSyncLoop. Both stay, and the flag set (25 variants) is still 4 bytes. main's other changes to PipeReader.rs (#43868, #43900, #44086), PipeWriter.rs and FileSink.rs merged without conflict.

Fail-before on current main: a debug build with src/ and packages/ from main dc3660d and this branch's tests. finalizers that run inside spawnSync do not stall the next spawnSync fails with {"collectedDuringCall":3,"stdout":"","exitedDueToTimeout":true,"exitCode":0}. a writer finalized during spawnSync does not break the next writer on the same fd fails with EEXIST: file already exists, epoll_ctl. The other six cases of the first file pass there. On this branch the two files pass (7 pass with --timeout 120000, and 18 pass with 2 skip).

build-toolchain-identity.test.ts: the flaky annotation for the case a file changes when its tool's path resolves to another tool, and only then, with the message Cannot tell which /…, is on 47 of the 60 most recent finished builds (121638 to 121728). The case fails in the parallel batch and passes alone. toolIdentity (scripts/build/tools.ts:217) runs child_process.spawnSync(exe, ["--version"], { timeout: 30_000, stdio: ["ignore", "pipe", "pipe"] }) on a two-line /bin/sh script, and the call ends at its own timeout. Stand-in: three Bun.stderr.writer() sinks held on globalThis with stderr on a pipe, a warm-up Bun.spawnSync, the drop, one more Bun.spawnSync under BUN_JSC_slowPathAllocsBetweenGCs=5, then that child_process.spawnSync with a 3s timeout. Debug build of main dc3660d: ETIMEDOUT after 3.4s to 3.7s in 3 of 3 runs. Release 1.4.3-canary 367d939: ETIMEDOUT in 3 of 3. This branch: clang version 21.1.8 with no error in 0.3s to 0.7s in 9 of 9. With zero writers main returns normally. The test file alone passes 15 of 15 on this branch.

Costs, how they were measured. Struct sizes: a compile-time size_of probe on both trees (KeepAlive 1 and 2 bytes, FilePoll 40 and 40, FlagsSet 4 and 4). timer::All gains two bools, one instance per VM. Lookups, counted from the diff: FilePoll::init reads the ctx's loop pointer and uws_get_loop() (a thread-local read) once each to set the bit, where main did no lookup. register_with_fd and unregister_with_fd resolve the loop once, as their callers did on main. EventLoopCtx::loop_ref does the same two reads as init before the ref, and loop_unref_for does one lookup, as loop_unref did. Binary size: size and a per-symbol nm -S diff on linux-x64 release builds of the merge base ba3f27d and of this branch, same checkout, only src/ swapped. The build is reproducible (two builds of 0dedb9f are byte-identical). The first merged revision, 0dedb9f, had 66,560 more bytes of .text than the base (58,217,461 against 58,150,901). The lookups that KeepAlive::ref_ and unref gained were inlined with them into 572 symbols: the node:fs async tasks, work-pool jobs, dns, sockets and the other users. CI's binary-size annotation on build 121730 agrees (+64.0 KB on linux-x64 and on linux-x64-musl). In 87f1190 EventLoopCtx::loop_ref returns the bit itself, and it and loop_unref_for are #[inline(never)], so a caller keeps a status check and one call. FilePoll::register_with_fd and unregister_with_fd resolve the loop once and pass &mut Loop to the private helpers, as on main. Result for 2d3f147: .text 58,144,245 bytes, 6,656 fewer than the base, and the file is 8,192 bytes smaller. CI's binary-size annotation on build 121775 compares against main #121761. No target is larger. linux-x64 is 12.0 KB smaller and linux-aarch64 is 64.0 KB smaller. strace, perf, valgrind and bloaty are not installed in this container, so there is no instruction or syscall count from a tool.

Suites on the merged debug build (0dedb9f), on a host under heavy load: spawnsync-isolated-event-loop.test.ts (7 pass, where the two #40508 cases take 10s to 13s under collectContinuously on this build and on main, above the 5s default), spawnSync.test.ts (18 pass, 2 skip), process-stdio.test.ts (18 pass), then spawn.test.ts, 10887.test.ts, filesink.test.ts, node-timers.test.ts, spawnsync-no-microtask-drain.test.ts and build-toolchain-identity.test.ts in one run with --timeout 120000 (278 pass, 8 skip, 1 fail). The one failure is the with BUN_FEATURE_FLAG_FORCE_WAITER_THREAD wrapper in spawn.test.ts, which hit its own 192s cap. The same inner run, started directly with a 60s per-test timeout, passes (177 pass, 9 skip, 210s). The time goes to the 200 debug bun -e children that several of its cases start. cargo check passes for x86_64-pc-windows-msvc, x86_64-apple-darwin and aarch64-unknown-linux-musl. Again on 87f1190: the two test files, the stand-in above (3 of 3 return normally), 10887.test.ts, filesink.test.ts, node-timers.test.ts, spawnsync-no-microtask-drain.test.ts, build-toolchain-identity.test.ts, process-stdio.test.ts and spawn-streaming-stdin.test.ts (120 pass in one run), spawn.test.ts in waiter-thread mode (177 pass, 9 skip) and in the default mode (177 pass, 8 skip, and the same wrapper at its 192s cap), and the same three cargo check targets. One run of spawnSync.test.ts failed should not set a timeout if timeout is 0 and inherit is used for stdout on its toBeLessThan(1000) wall-clock check (1190ms for a debug child). A spawnSync of sleep 0.3 takes 300ms to 1600ms on this host with the release builds of main and of this branch alike.

Earlier summary, moved here from the visible part. The user-visible face (#34069): under bun test --isolate, a file that touches process.stderr and calls Bun.spawnSync times out after 5000ms, or the run hangs until it is killed. spawnSync never returns, the child stays a zombie, and the parent spins at 100% CPU. Other signatures of the spin and of the stale epoll entry: ✗ does not segfault [90001.51ms] (10887.test.ts), and in the shell ShellError: Failed with exit code 17 (#40611). An epoll entry is keyed by the open file and the fd number. The kernel drops it only when the last fd for that open file closes. A dup of stdio never reaches that point while the process runs, so a missed EPOLL_CTL_DEL leaves the entry behind for the next dup that lands on the same number. FilePoll is one registration of an fd on a loop. KeepAlive is the on/off loop ref used by servers, sockets, fetch, fs and dns. FilePoll::activate and deactivate take the EventLoopCtx too, and SpawnSyncEventLoop::prepare asserts in debug builds that the isolated loop's counters are zero before each spawn. This PR supersedes #37754 (same cause, predates #38883) and #40611 (same FilePoll cause, stored a loop pointer instead of the bit, and its test is folded in here). main fixed the double close that #40611 also reported in #38354. This PR's tests still fail on main with #40508, because FilePoll, KeepAlive and the timer counters resolve their loop through EventLoopCtx, not through the EventLoop.

The notes below predate the merge of 2026-09-30.

How the bucket was identified: every flaking build ran the same 65-file batch, and in that batch 10887.test.ts was the tenth file of the third worker, right after web-globals.test.js (four stdin: "pipe" children), the napi test_object suite (four pipeless spawnSyncs), 013880, 04893 and 05828. 11 of the last ~110 PR builds flaked on at least one of debian 13 x64/aarch64 and ubuntu 25.04 x64/aarch64, no flake on any build with a different bucket. #38883 saw the same state on 07261.

Repro of the mechanism: with a debug build instrumented to record, per FilePoll, the loop it was activated on, a forced collectNow(Sync) at the end of spawnSync logged FileSink__finalize → PosixStreamingWriter::drop → FilePoll::deinit_force_unregister → deactivate against the isolated loop for polls activated on the main loop, then prepare: num_polls=-1 on the next spawnSync and a pipeless napi spawnSync spinning with num_polls=0. Without the forced GC the drift did not reproduce locally in ~100 batch runs (release canary and debug, with CI's BUN_GARBAGE_COLLECTOR_LEVEL=1), so the CI trigger is GC timing. The isolated loop is reused for every spawnSync in the process, which is why the drift accumulates.

The asan abort (#40611): 15 of 16 asan builds, always the worker that ran test/regression/issue/20753.test.js right after 19652.test.ts (a Bun.spawnSync). The GC epilogue runs inside spawnSync on the first JS allocation after the wait (create_resource_usage_object) and finalizes the previous test file's process.stderr FileSink. Timeline from an instrumented asan build: epoll ADD ok (default loop), epoll DEL ENOENT (other loop) from FileSink::finalize, Closer::close on the work pool, epoll ADD EEXIST (default loop) for the next Bun.file(2).writer(), FileSink::setup closes the fd, then the writer's Drop schedules a second close that fails with EBADF. The panic was visible in the worker log but not in annotations: the runner re-ran the crashed files alone and marked the job flaky. On macOS kqueue knotes are per fd, so the EEXIST symptom cannot appear there; the spawnSync.test.ts case is Linux-only for that reason. The wrong-loop EV_DELETE and the counter drift applied on macOS too and are fixed by the same change.

Merge with main (367d939): main's #38354 fixed the double close with the opposite fd contract. A failed POSIX start() now leaves the fd with the caller, and FileSink::setup closes it. This branch first made the writer own the fd. The merge takes main's contract: FileSink::setup and PosixStreamingWriter::start keep main's bodies, and this branch's filesink.test.ts case is gone because main's "whose registration fails closes the dup exactly once" covers it. The EEXIST half stays here, with its spawnSync.test.ts case. unregister_with_fd_impl takes main's disarmed_only logic and resolves the watcher fd through loop_mut(vm). On the merged debug build: spawnsync-isolated-event-loop.test.ts (the new case passes; the two #40508 cases still need more than the 5s default on a local debug build), spawnSync.test.ts (18 pass, 2 skip), filesink.test.ts (68 pass), spawn-pipe-start-error.test.ts (9 pass, 1 skip), spawn.test.ts (144 pass, 8 skip), isolation.test.ts (41 pass), node-timers.test.ts (24 pass), 10887.test.ts, spawn-streaming-stdin.test.ts and spawnsync-no-microtask-drain.test.ts. child_process.test.ts has 65 pass and 2 fail: extra stdio pipes are not double-closed on GC needs 6.4s on this debug build and passes with a longer timeout, and should allow us to spawn in the default shell fails because $SHELL is empty in the test environment of this container. cargo check --workspace passes for x86_64-pc-windows-msvc and aarch64-apple-darwin.

Stand-in for the user-visible face: 30 files under bun test --isolate --smol with stderr on a pipe. Each file touches process.stderr and process.stdout, allocates 20000 small objects, then calls Bun.spawnSync(["/bin/true"]) 30 times and counts the fds on the stderr pipe around each call. Release build of main b52d513: 11 of 20 runs were killed at the 25s limit after (fail) t04 [5019.48ms] or (fail) t08. Release build of this branch merged with main: 0 of 20 hung and 0 failed. In 4 of those 20 runs, 13 sinks of earlier files were finalized inside a Bun.spawnSync call: the fd count and the main loop's numPolls both dropped across the call. A second variant reads, at the end of every file, the main loop's numPolls minus the fds on the stderr pipe. On main the value moves from -2 to 0 and 1 in the runs that hang. On this branch it is -2 in all 600 files of 20 runs, and in 8 of those runs sinks were finalized inside a call. The same stand-in on the debug build never finalizes a sink inside a call, so it shows nothing there.

The script from #34069: spawnsync-spin.test.mjs from the comment of 2026-09-19, unchanged, in the shape its runner uses (8 copies of the file, bun test --isolate --timeout 0, stdout on a pipe, 3 such processes at once, a process that is silent for 40s counts as wedged). Release builds, blocks of 6 process-runs that alternate between the two binaries, Linux x64. main 367d939: 15 of 72 wedged, each in state R with its CPU time rising 5s per 5s and one sh child a zombie. This branch (2cd5c71): 0 of 72. bun 1.4.2: 5 of 6.

Why BUN_JSC_slowPathAllocsBetweenGCs: a finalizer runs inside spawnSync only when JSC sweeps at one of the allocations spawnSync makes after the wait. LocalAllocator::doTestCollectionsIfNeeded runs collectNow(Sync, Full), which sweeps synchronously, every N slow-path allocations. Two details make the window deterministic. The first spawnSync call of a process creates lazy structures and Bun.spawnSync itself, about 11 slow-path allocations before the loop swap, so the fixture warms spawnSync up while the writers are still reachable; the next call then makes no allocation before the swap (measured with markers in prepare/cleanup and BUN_JSC_logGC=1: 0 GCs before the swap, 3 inside at N=5) and the first forced GC after the drop lands inside it. The probe child sleeps 300ms so the parent has to poll for its exit; with echo the child could exit before the polls were registered and the unfixed build returned without spinning. N=5 spins on the unfixed release, canary and main (with #40508) builds in 105 of 105 runs with the allocation phase shifted by 0 to 150 objects of five kinds. N=1 also works but makes a debug build spend 12s in startup GCs; N=5 costs about 1s. An earlier revision used a debug-only run_gc(true) hook behind BUN_INTERNAL_SPAWN_SYNC_GC, which no CI lane exercised; the hook is gone.

Why a bit and not a pointer: an earlier revision stored the loop pointer on every FilePoll, KeepAlive and timer counter and dropped the ctx argument from unref. Review asked for a 1-bit identifier instead, since only two loops exist per thread. The bit also leaves the existing ref_(ctx)/unref(ctx) call sites unchanged.

Isolated-loop lifetime: every poll that records SpawnSyncLoop is created and freed inside the same spawnSync call. The stdio readers and the process poller start after prepare() swaps the handle in, SubprocessT::finalize runs before the cleanup() scopeguard swaps it back, and the sync Subprocess never gets a JS wrapper. loop_for asserts this in debug builds, and KeepAlive::unref_on_next_tick (deferred through pending_unref_counter, drained on the thread's loop) asserts its ref was not taken on the isolated loop. Timer refs are never taken during spawnSync, since no JS runs there, but a finalizer can release one, so the timer counters record the bit too.

Windows: VirtualMachine::uws_loop() returns Loop::get() unconditionally there, so is_spawn_sync_loop() is always false and every release resolves to the same loop as before. The Windows FilePoll is unchanged.

Why the timer takes the EventLoopCtx too: timer::All first carried its own copy of the spawnSync predicate and release path over a raw *mut Loop, without the release-after-return debug assert. It now records the bit from EventLoopCtx::is_spawn_sync_loop and releases through loop_unref_for, like FilePoll and KeepAlive. The four callers (timer_object_internals.rs, subprocess.rs, two in dns.rs) already had the per-thread ctx in reach.

Why the fixture uses WeakRefs and not bun:internal-for-testing or node:fs: fileSinkInternals.liveCount() or an fd listing would give exact counts, but loading either module under slowPathAllocsBetweenGCs=5 costs a debug build 5s to 15s (1s without). A WeakRef made before a job boundary (await Bun.sleep(0)) does not keep its target alive afterwards (JSC currentWeakRefVersion), and deref() after the probe call returns undefined exactly when the writer was collected during the call. The writers hang off globalThis until the drop: a module binding that is only written after the await is dead at the suspension point, and the writers were collected in the warm-up call instead (measured: 0 of 3 alive after the warm-up). With the global, an unfixed build spins with collectedDuringCall: 3 for every padding tried before the warm-up (0, 2, 5 and 40 objects). What the check cannot see is a GC that runs after the drop but before the loop swap: an allocation between the drop and the call (the comment forbids code there), or a slow-path allocation in spawnSync's argument parsing, of which there is none today (N=1 also spins).

Scope: the drift requires a teardown under the isolated loop. GC finalizers of FileSink, Subprocess, FetchTasklet, MessagePort and similar are the only code that runs there since #38883 moved the bun:test timeout callback out of the wait. EventLoopCtx::loop_unref, loop_add_active and loop_sub_active are gone on POSIX (the last two remain for the Windows FilePoll). EventLoopCtx::platform_event_loop and PosixStreamingWriterParent::loop_ (with the uws_loop arm of impl_streaming_writer_parent!) lost their last callers and are removed.

Fail-before was checked against a debug build of the merge base (6d13a3a) in a second worktree, the same shape as the gate's stash run: all three tests this branch had then fail there (spin, EEXIST, err.is_none() abort) and pass on this branch. The third one left with the merge above.

Suites run on the debug build: spawn.test.ts (140 pass, 8 skip), spawnSync.test.ts (17 pass, 2 skip), filesink.test.ts, spawn-streaming-stdin.test.ts, spawn-stdin-readable-stream-sync.test.ts, spawn-stdin-pipe-fd-leak.test.ts, node-timers.test.ts, dns-resolver-concurrent-timeout.test.ts, dns-lookup-keepalive.test.ts, dns-interleave.test.ts, spawn-kill-signal.test.ts, spawn-maxbuf.test.ts, spawn-signal.test.ts, spawnsync-isolated-event-loop.test.ts, spawnsync-no-microtask-drain.test.ts, spawn-streaming-stdout.test.ts, spawn-streaming-stdin.test.ts, spawn-stdin-readable-stream-sync.test.ts, exit-code.test.ts, spawn-kill-signal.test.ts, spawn-many-teardown.test.ts, 10887.test.ts, filesink.test.ts and bun-write.test.js (123 pass), child_process.test.ts and child_process-node.test.js, node-timers.test.ts. resolve-dns.test.ts needs network access this container lacks (getaddrinfo ETIMEOUT). cargo check passes for linux-gnu x64, darwin x64 and windows x64. Rebased onto main after #40508: the only conflicts were the test file's imports. #40508's two new tests in the same file pass on this branch (OK, exit 0) but need more than the 5s default timeout on a local debug build (8s and 11s under collectContinuously). The same applies to extra stdio pipes are not double-closed on GC in child_process.test.ts (passes alone in 9s).


no test proof · iteration 7 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts, test/js/bun/spawn/spawnSync.test.ts

@coderabbitai

coderabbitai Bot commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f00c6462-31a5-4e89-ba06-6e1cab740329

📥 Commits

Reviewing files that changed from the base of the PR and between 5997b22 and c51a27b.

📒 Files selected for processing (14)
  • src/io/ParentDeathWatchdog.rs
  • src/io/PipeReader.rs
  • src/io/PipeWriter.rs
  • src/io/keep_alive.rs
  • src/io/lib.rs
  • src/io/posix_event_loop.rs
  • src/runtime/api/bun/Terminal.rs
  • src/runtime/dns_jsc/dns.rs
  • src/runtime/dns_jsc/dns_sd.rs
  • src/runtime/node/memory_pressure.rs
  • src/runtime/timer/mod.rs
  • src/runtime/webcore/FileSink.rs
  • src/spawn/process.rs
  • test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts
💤 Files with no reviewable changes (2)
  • src/runtime/api/bun/Terminal.rs
  • src/runtime/webcore/FileSink.rs

Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review.


Walkthrough

Changes

The change routes poll lifecycle operations through EventLoopCtx, preserves ownership for isolated spawnSync loops, updates keep-alive and timer references, removes obsolete raw loop accessors, and adds debug-only GC regression coverage.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: releasing polls and keep-alives on the loop where they were acquired during spawnSync.
Description check ✅ Passed The description explains the problem, fix, scope, risks, and verification results in detail. It does not use the template headings exactly, but it provides the required information, including how the …

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

@robobun

robobun commented Aug 22, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 9:31 PM PT - Sep 29th, 2026

✅ @robobun, your commit 2d3f147ea0f7afcd1bf6e61a8a140d2d7d1d4bd6 passed in Build #121775! 🎉


🧪   To try this PR locally:

bunx bun-pr 40078

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

bun-40078 --bun

@robobun

robobun commented Aug 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. 1-bit main/isolated flag per review. The tests run in CI on release builds. Head 2d3f147, merged with main ba3f27d. CI build 121775 passes (181 of 181 jobs).

What changed on 2026-09-30:

  • main is merged into the branch, which was in conflict. Four files conflicted and all four are mechanical (the PR body lists them). The design did not change.
  • 87f1190 keeps the loop bookkeeping of KeepAlive::ref_ and unref out of line. The first merged revision inlined it into 572 symbols, and release .text on linux-x64 was 66,560 bytes larger than main. It is now 6,656 bytes smaller than main.
  • The branch still merges with current main (d115f54) without a conflict.

How it reproduces on current main:

  • Debug build of main dc3660d with this branch's tests. bun bd test test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts -t "finalizers that run inside" fails: the probe spawnSync hits its 2s timeout with empty stdout. bun bd test test/js/bun/spawn/spawnSync.test.ts -t "finalized during spawnSync" fails with EEXIST: file already exists, epoll_ctl. Both pass on this branch.
  • The build-toolchain-identity.test.ts flake (Cannot tell which /…). A stand-in for its child_process.spawnSync(exe, ["--version"], { timeout, stdio: ["ignore", "pipe", "pipe"] }), run after a GC inside an earlier Bun.spawnSync finalized three Bun.stderr.writer() sinks: ETIMEDOUT in 3 of 3 runs on main, and a normal return in 12 of 12 runs on this branch.
  • Release builds, bun test --isolate stand-in: 30 files, each touches process.stderr and calls Bun.spawnSync(["/bin/true"]) 30 times, stderr on a pipe. main b52d513: 11 of 20 runs were killed at the 25s limit after (fail) t04 [5019.48ms]. This branch: 0 of 20 hung and 0 failed.

In CI: the two builds of this branch since the merge (121730, 121775) have no annotation for build-toolchain-identity.test.ts or 10887.test.ts. Of the 118 other finished builds from 121608 to 121781, 92 have one for the first and 8 have one for the second.

Supersedes #37754. Complements #40508, which fixed the jsc::EventLoop keep-alive path.

Comment thread src/runtime/napi/napi_body.rs Outdated
@robobun
robobun force-pushed the farm/7d1c0396/spawnsync-isolated-loop-poll-count branch from 1967316 to a9e1382 Compare August 22, 2026 10:39
Comment thread src/bun_core/env_var.rs Outdated
Comment thread src/event_loop/SpawnSyncEventLoop.rs Outdated
Comment thread src/io/keep_alive.rs Outdated
Comment thread src/io/lib.rs Outdated
Comment thread src/io/posix_event_loop.rs Outdated
Comment thread src/io/posix_event_loop.rs Outdated
Comment thread src/io/windows_event_loop.rs Outdated
Comment thread src/io/windows_event_loop.rs Outdated
Comment thread src/jsc/event_loop.rs Outdated
Comment thread src/jsc/event_loop.rs Outdated
Comment thread src/jsc/event_loop.rs Outdated
Comment thread src/runtime/api/bun/js_bun_spawn_bindings.rs Outdated
Comment thread src/runtime/timer/mod.rs Outdated
Comment thread src/runtime/timer/mod.rs Outdated
Comment thread src/event_loop/SpawnSyncEventLoop.rs Outdated
Comment thread src/io/keep_alive.rs Outdated
Comment thread src/io/lib.rs Outdated
Comment thread src/io/posix_event_loop.rs Outdated
Comment thread src/io/windows_event_loop.rs Outdated
Comment thread src/jsc/event_loop.rs Outdated
Comment thread src/runtime/api/bun/js_bun_spawn_bindings.rs Outdated
@robobun

robobun commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

Review sweep:

  • The stale comments that described the removed unref ctx argument (napi_body.rs, dns.rs, the three s3 files, WebSocketUpgradeClient.rs) are gone in a9e1382. That commit also drops the dead MultiPartUpload.vm field.
  • The doc comments on the new loop-tracking fields (FilePoll::loop_, KeepAlive::loop_, EventLoop::uws_loop, the timer ref loops) are down to one to three lines each in f181aeb. The comment above the prepare debug assert is removed in ab9ea4a. The assert message states the fact.

No code change since a9e1382. The test file still passes on the debug build (5 tests).

@robobun

robobun commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator Author

This bug now also fails test/js/web/workers/structuredClone-classes.test.ts in the --parallel batch on both Alpine lanes. The flaky annotation names it on 14 of the 27 builds from 116000 to 116026. The file passes alone.

Signature (build 116040, alpine 3.23 x64, shard 16):

killed 1 dangling process
✗ serialize & deserialize > net.BlockList (cloneable, non-transferable) [90000.49ms]
  ^ this test timed out after 90000ms.

Other runs print Interrupted while still running: test/js/web/workers/structuredClone-classes.test.ts (244s) and the runner reports (worker crashed).

Why this file

  • Each Bun.spawnSync in it has two polls: the pidfd and the stdout pipe.
  • On the Alpine shards one worker runs fetch-args, message-port-context-destroy-leak and message-port-pipe directly before it. --parallel implies --isolate, so the globals of those files are garbage by then.
  • Two FileSinks from those globals still hold a poll on the worker's stderr pipe. When they die inside a spawnSync wait, the isolated loop's num_polls goes from 0 to -2. The next spawnSync adds its two polls, num_polls is 0, and us_loop_run_bun_tick returns without a poll until the test timeout closes the stdout reader.
  • musl is not the cause. The Debian and Ubuntu lanes shard the files differently.

The sinks die in the GC epilogue, so no synchronous GC is necessary
The JSFileSink cells are precise allocations, and Heap::runCollectionEpilogue sweeps those eagerly. On canary 1.4.3-canary.1+37c28924e (linux-x64-musl) an ordinary run enters collectInMutatorThread from the JSResourceUsage allocation in create_resource_usage_object, while the isolated loop is still the current loop. When I force a collection at that point from gdb, the two unregisters have this stack:

FilePoll::unregister
FilePoll::deinit_force_unregister
PosixStreamingWriter<FileSink>::drop
JSC::MarkedSpace::sweepPreciseAllocations
JSC::Heap::sweepEagerlyInEpilogue
JSC::Heap::runCollectionEpilogue
JSC::Heap::collectInMutatorThread
JSC::Heap::collectNow
JSC__VM__runGC            (called from gdb)
Subprocess::create_resource_usage_object
spawn_maybe_sync

Reproduction on the canary, without CI

  1. Run the six files in one process: bun test --isolate <fetch-args> <fetch-url-after-redirect> <fetch.brotli> <message-port-context-destroy-leak> <message-port-pipe> <structuredClone-classes> 2>&1 | cat. The pipe on stderr is necessary. With stderr in a file the sinks have no poll and nothing drifts.
  2. In gdb, stop in Subprocess::create_resource_usage_object of the first spawnSync and call JSC__VM__runGC(vm, 1).
  3. A watchpoint on the isolated loop's num_polls shows 0, then -2. The BunFile test then fails with the CI output, at the same structuredClone-classes.test.ts:39:24 frame as build 116014.

Both CI variants follow from the drift. I set it by hand at the first isolated tick, with --timeout=5000:

drift result
-2 each of the three tests prints killed 1 dangling process and times out at 5000 ms, then the file continues
-1 killed 1 dangling process once, then the process spins and does not end (the 244 s variant)

A spawnSync without pipes has one poll, so -1 stops it the same way. That is consistent with the napi do.test.ts files that hang for about 242 s on the same lanes.

This branch conflicts with main at the moment.

@robobun

robobun commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator Author

The wrong-epoll EPOLL_CTL_DEL that this PR fixes is also the source of the exit code 17 flake in the parallel batch. I traced it on the release canary. The details are below.

Symptom in CI. On the glibc Linux lanes, the 78-file bun test --parallel=3 bucket fails test/napi/uv.test.ts with ShellError: Failed with exit code 17. Less often it fails test/js/bun/shell/exec.test.ts and test/js/bun/test/expect-assertions.test.ts with Expected: 1, Received: 17. All three pass on the solo retry. A count over builds 116415 to 117259 found uv.test.ts in 131 of 400 builds. 17 is EEXIST. The shell dups fd 2 for its stderr IOWriter. EPOLL_CTL_ADD for the dup fails. Cmd::buffered_output_close_stderr (src/runtime/shell/states/Cmd.rs:1068) stores the errno as the exit code. The command's stderr is lost too.

Local repro. 1.4.3-canary.1+b52d51348, the same 78 files, the environment that scripts/runner.node.mjs sets. BUN_GARBAGE_COLLECTOR_LEVEL=1 is the part that matters. 23 of 147 bucket runs failed uv.test.ts with exit code 17.

Trace. An LD_PRELOAD tracer on epoll_ctl and close recorded the registrations per fd in the test workers (Bun makes both calls through syscall(2)):

  • 118 dups of fd 2 were closed while an epoll entry for them still existed. In 118 of 118 cases the operation before the close was an EPOLL_CTL_DEL that returned ENOENT. It went to a different epoll fd than the EPOLL_CTL_ADD. No other source of a stale entry showed up.
  • The epoll that received the DEL held exactly one entry at that moment, an eventfd (20 of 20 samples). So it is an idle second loop on the JS thread. The same pattern appears when I release three Bun.stderr.writer() sinks inside Bun.spawnSync on purpose (BUN_JSC_slowPathAllocsBetweenGCs=5, as in this PR's test). There the DEL goes to the epoll fd that the first spawnSync call created.
  • The close ran on a thread pool thread, so the owner is a FileSink (PollOrFd::close, then Closer). The registrations were writable one-shot polls. They match the process.stderr sink that each isolated test file leaves behind. On the canary each file adds one more dup of fd 2, which is the leak that test --isolate: end the outgoing file's process stdio sinks at the global swap #38008 describes. Up to five of them were released in one sweep.
  • After that, each EPOLL_CTL_ADD for a new dup of fd 2 that got one of those numbers returned EEXIST (788 times).

One instance, worker pid 17509:

ADD   epfd=4   fd=10  EPOLLOUT|EPOLLERR|EPOLLHUP|EPOLLONESHOT   ok       JS thread
DEL   epfd=16  fd=10                                            ENOENT   JS thread
close          fd=10                                                     thread pool thread, 1 ms later
ADD   epfd=4   fd=10                                            EEXIST   JS thread, 70 ms later

So the mechanism in this PR explains that flake in full. #38008 removes the same path for the test runner from the other side. It ends each file's stdio sinks at the global swap. Then no sink is left for a finalizer to release inside spawnSync.

A fallback at registration is not a substitute. I tried the libuv approach: when EPOLL_CTL_ADD fails with EEXIST, retry as EPOLL_CTL_MOD (uv__io_poll in src/unix/linux.c). The symptom goes away, but the change is not safe in FilePoll. libuv allows one watcher per fd. FilePoll has no such table, so EEXIST can also mean that a second live poll exists for the fd. Example with stdin on a pipe:

const a = Bun.file(0).stream().getReader();
const b = Bun.file(0).stream().getReader();
console.log(await Promise.allSettled([a.read(), b.read()]));

On main a gets the data and b rejects with EEXIST. With the fallback the second poll takes over the entry of the first, a.read() never settles, and the process hangs. I did not open a PR for it.

Repro recipe and tracer

File list: the [32-109/300] 78 files in parallel (3×) group in a debian 13 x64 test-bun job log (for example build 117286).

T=$(mktemp -d)
env -i PATH="$PATH" HOME="$HOME" TMPDIR="$T" BUN_TMPDIR="$T" TEST_TMPDIR="$T" BUN_INSTALL_CACHE_DIR="$T" \
  FORCE_COLOR=1 CI=true BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING=1 BUN_DEBUG_QUIET_LOGS=1 \
  BUN_GARBAGE_COLLECTOR_LEVEL=1 BUN_JSC_randomIntegrityAuditRate=1.0 BUN_RUNTIME_TRANSPILER_CACHE_PATH=0 \
  BUN_ENABLE_CRASH_REPORTING=0 LD_PRELOAD=$PWD/eptrace.so EPTRACE_OUT=/tmp/eptrace.log \
  taskset -c 0-2 bun test --parallel=3 --timeout=45000 --dots $FILES
grep -v dup_of=-1 /tmp/eptrace.log

Node child processes also report EEXIST on ADD (dup_of=-1). That is libuv's normal path, and the grep removes it. 105 of the 147 runs used a longer version of the tracer that also keeps a per-fd history (95 of 95 closes). The other 42 runs used this short version, which reports the same three events (23 of 23 closes).

// cc -O1 -fPIC -shared -o eptrace.so eptrace.c -ldl -lpthread
// LD_PRELOAD=./eptrace.so EPTRACE_OUT=/tmp/eptrace.log bun test --parallel=3 ...
// Reports, per process: an EPOLL_CTL_DEL that misses while another epoll still
// holds the fd, a close() of a dup of fd 0/1/2 that still has an epoll entry,
// and an EPOLL_CTL_ADD that fails with EEXIST.
#define _GNU_SOURCE
#include <dlfcn.h>
#include <errno.h>
#include <fcntl.h>
#include <pthread.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/epoll.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <unistd.h>

#define MAXFD 8192
static int registered_in[MAXFD]; // epoll fd that holds an entry for this fd, 0 = none
static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
static long (*real_syscall)(long, ...);
static int (*real_close)(int);
static int (*real_epoll_ctl)(int, int, int, struct epoll_event *);
static int self_pid;

static void init(void) {
  if (real_syscall) return;
  real_syscall = dlsym(RTLD_NEXT, "syscall");
  real_close = dlsym(RTLD_NEXT, "close");
  real_epoll_ctl = dlsym(RTLD_NEXT, "epoll_ctl");
}

// A vfork child shares this table with its parent. Ignore it.
static int in_vfork_child(void) {
  int pid = (int)real_syscall(SYS_getpid);
  if (!self_pid) self_pid = pid;
  return pid != self_pid;
}
static void at_fork_child(void) { self_pid = 0; }
__attribute__((constructor)) static void setup(void) { pthread_atfork(NULL, NULL, at_fork_child); }

static int stdio_dup(int fd) {
  struct stat a, b;
  if (fd <= 2 || fstat(fd, &a)) return -1;
  for (int i = 0; i <= 2; i++)
    if (!fstat(i, &b) && a.st_dev == b.st_dev && a.st_ino == b.st_ino) return i;
  return -1;
}

static void report(const char *what, int fd, int epfd) {
  const char *path = getenv("EPTRACE_OUT");
  if (!path) return;
  char line[256];
  int n = snprintf(line, sizeof line, "%s pid=%d tid=%d fd=%d dup_of=%d epfd=%d entry_in_epfd=%d\n", what, self_pid,
                   (int)real_syscall(SYS_gettid), fd, stdio_dup(fd), epfd, registered_in[fd]);
  int out = (int)real_syscall(SYS_openat, AT_FDCWD, path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0644);
  if (out < 0) return;
  if (write(out, line, n) < 0) {}
  real_syscall(SYS_close, out);
}

static void on_close(int fd) {
  if (fd < 0 || fd >= MAXFD || in_vfork_child()) return;
  int saved = errno;
  pthread_mutex_lock(&lock);
  if (registered_in[fd] && stdio_dup(fd) >= 0) report("CLOSE_WITH_ENTRY", fd, -1);
  registered_in[fd] = 0;
  pthread_mutex_unlock(&lock);
  errno = saved;
}

static int on_epoll_ctl(int epfd, int op, int fd, struct epoll_event *ev, int raw) {
  int rc = raw ? (int)real_syscall(SYS_epoll_ctl, epfd, op, fd, ev) : real_epoll_ctl(epfd, op, fd, ev);
  int err = rc ? errno : 0;
  if (fd >= 0 && fd < MAXFD && !in_vfork_child()) {
    pthread_mutex_lock(&lock);
    if (!rc && op != EPOLL_CTL_DEL) registered_in[fd] = epfd;
    else if (!rc) registered_in[fd] = 0;
    else if (op == EPOLL_CTL_DEL && err == ENOENT && registered_in[fd] && registered_in[fd] != epfd)
      report("DEL_ON_OTHER_EPOLL", fd, epfd);
    else if (op == EPOLL_CTL_ADD && err == EEXIST) report("ADD_EEXIST", fd, epfd);
    pthread_mutex_unlock(&lock);
  }
  errno = err;
  return rc;
}

int close(int fd) {
  init();
  on_close(fd);
  return real_close(fd);
}

int epoll_ctl(int epfd, int op, int fd, struct epoll_event *ev) {
  init();
  return on_epoll_ctl(epfd, op, fd, ev, 0);
}

// Bun calls close(2) and epoll_ctl(2) through syscall(2).
long syscall(long number, ...) {
  init();
  va_list ap;
  va_start(ap, number);
  long a = va_arg(ap, long), b = va_arg(ap, long), c = va_arg(ap, long);
  long d = va_arg(ap, long), e = va_arg(ap, long), f = va_arg(ap, long);
  va_end(ap);
  if (number == SYS_close) on_close((int)a);
  if (number == SYS_epoll_ctl) return on_epoll_ctl((int)a, (int)b, (int)c, (struct epoll_event *)d, 1);
  return real_syscall(number, a, b, c, d, e, f);
}

main's #38354 made a failed POSIX `start()` leave the fd with the caller.
Take that contract here:

- `FileSink::setup` and `PosixStreamingWriter::start` keep main's bodies.
  This branch's fd-ownership change is gone. `start()` still passes the
  `EventLoopCtx` to `register_with_fd`.
- Drop this branch's filesink.test.ts case. main's "whose registration
  fails closes the dup exactly once" covers the double close.
- `unregister_with_fd_impl` takes main's `disarmed_only` logic and
  resolves the watcher fd through `loop_mut(vm)`.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

@robobun

robobun commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator Author

Merged main (367d939) into this branch at 2cd5c71. The PR body and the status comment describe the result. One part of the merge is not mechanical: main's #38354 fixed the FileSink::setup double close with the opposite fd contract (a failed POSIX start() leaves the fd with the caller), so that half of this PR and its filesink.test.ts case are gone. The loop-bit change is unchanged.

One note for whoever triages a future failure of finalizers that run inside spawnSync do not stall the next spawnSync. The probe spawnSync has timeout: 2000. On a host at load 700, in a run with --timeout 90000 next to the two collectContinuously cases, the fixed debug build failed it once with "exitedDueToTimeout":true,"exitCode":null: the child was still running at 2s and the timeout killed it. The real spin looks different. The child exits on its own and the parent does not see it, so the result is "exitedDueToTimeout":true,"exitCode":0 (3 of 3 runs on 1.4.3-canary b52d513). Alone, the case passes 6 of 6 runs in 1.5s to 1.7s on the same build.

@FredericoAndrade1000

Copy link
Copy Markdown

Independent confirmation on Bun 1.4.2 (744846f844374847c902b5e7fd59b4342a51ef99), Linux 6.8.0-138-generic x86_64. The ownership bug described in this PR matches our observations. We have not tested this PR's branch yet.

We initially observed intermittent spawnSync timeouts in a test suite without GC stress settings: the child was already a zombie while the parent consumed approximately one CPU core. We then reduced the problem to the following dependency-free program.

Save as repro.cjs:

const { spawnSync } = require('node:child_process');
const closeExplicitly = process.argv[2] === 'end';

(async () => {
  for (let round = 0; round < 8; round++) {
    for (let i = 0; i < 3; i++) {
      const sink = Bun.stdout.writer();
      sink.write('test');
      if (closeExplicitly) await sink.end();
    }

    const first = spawnSync('/usr/bin/sleep', ['0.05'], { timeout: 1000 });
    const started = performance.now();
    const second = spawnSync('/usr/bin/printf', ['expected'], {
      timeout: 1000, encoding: 'utf8',
    });

    if (first.error || second.error || second.status !== 0 || second.stdout !== 'expected') {
      console.error(JSON.stringify({
        round,
        firstError: first.error?.code,
        secondError: second.error?.code,
        secondStatus: second.status,
        stdout: second.stdout,
        secondMs: performance.now() - started,
      }));
      process.exit(1);
    }
    await new Promise(resolve => setImmediate(resolve));
  }
  console.error(JSON.stringify({ passed: 8 }));
  process.exit(0);
})();

Run with stdout connected to a pipe and BUN_JSC_collectContinuously=1. This setting accelerates reproduction; it was not enabled in the original suite. Keep an external timeout because the process may also fail to exit naturally. For example, save this launcher as run.cjs and execute node run.cjs:

const { spawn } = require('node:child_process');
const child = spawn('bun', ['repro.cjs', ...process.argv.slice(2)], {
  env: { PATH: process.env.PATH, BUN_JSC_collectContinuously: '1' },
  stdio: ['ignore', 'pipe', 'pipe'],
});
child.stdout.resume();
child.stderr.pipe(process.stderr);
const guard = setTimeout(() => child.kill('SIGKILL'), 12000);
child.once('exit', code => {
  clearTimeout(guard);
  process.exitCode = code ?? 1;
});

In three runs of the reduced program, two failed with ETIMEDOUT after approximately 1,001 ms and one failed with EEXIST from epoll_ctl while creating a writer. In the two timeout cases, status was still 0 and stdout was "expected", despite the timeout error. These counts describe the small confirmation sample, not an estimated failure rate.

The explicit-close control (node run.cjs end) completed all eight rounds. In separate instrumented controls, retaining the sinks until after the synchronous calls, or collecting them with Bun.gc(true) before entering spawnSync, also avoided the counter corruption for eight rounds.

An instrumented version with the same sink/spawn sequence captured this ownership mismatch:

  • Three stdout sink FDs (9, 10, 11) were registered on the main loop's epoll FD 3.
  • While spawnSync had its private loop installed, cleanup attempted to remove those FDs from epoll FD 7 instead:
epoll_ctl(7, EPOLL_CTL_DEL, 9, NULL)  = -1 ENOENT
epoll_ctl(7, EPOLL_CTL_DEL, 10, NULL) = -1 ENOENT
epoll_ctl(7, EPOLL_CTL_DEL, 11, NULL) = -1 ENOENT
  • The main loop retained num_polls = 3; the isolated loop ended at num_polls = -3.
  • The following synchronous spawn added its three polls, bringing the isolated count to zero and causing the wait loop to skip polling until timeout.

The counter reads used the matching release's Linux x86-64 layout and checked the epoll FD and wakeup back-reference. No memory writes or binary patches were used in this reduced reproduction. This gives an independent stdout-based reproducer for the FilePoll ownership issue addressed here.

@robobun

robobun commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

CI data for this PR, from a scan of the annotations of the last 400 finished builds (118369 to 119356, all branches).

The spin shows up in the bun test --parallel batches of the Linux lanes under two signatures:

  • (worker crashed): worker process crashed before reporting results. The job log says Interrupted while still running: <file> (246s): the file is silent until the 4 minute idle timeout of the runner ends the batch. The file passes when it runs alone.
  • A test that takes exactly the timeout of its spawnSync call. test/internal/build-toolchain-identity.test.ts fails with Cannot tell which .../current/clang this is after 30001.36ms (build 119356, :alpine: 3.23 x64). Its tool is a two line shell script.

Files with these signatures, by number of builds:

file builds lanes
test/js/node/fs/fs-stat-seccomp-linux.test.ts 67 alpine x64 and aarch64
test/internal/build-toolchain-identity.test.ts 57 all six Linux release lanes
test/napi/node-napi-tests/test/js-native-api/test_object/do.test.ts 24 debian and ubuntu x64
test/napi/node-napi-tests/test/js-native-api/test_conversions/do.test.ts 13 alpine
test/js/third_party/prisma/prisma.test.ts 11 alpine
test/js/third_party/esbuild/esbuild-child_process.test.ts 6 ubuntu and debian aarch64
test/napi/node-napi-tests/test/node-api/test_cleanup_hook/do.test.ts 6 x64-asan, aarch64
test/js/bun/wasm/wasi.test.js 3 alpine x64

Each of these files calls spawnSync. In the three job logs I read, the file was not the first one of its worker: two to seven files that use Bun.spawn ran in the same process before it. fs-stat-seccomp-linux.test.ts calls spawnSync("cc", ...) at describe time with no timeout, which is why it never ends. On main it hangs in 9 of the 11 builds since 119127.

I could not reproduce the spin on my machine with the same file lists (0 of 420 batch runs), so I have no independent check of the fix. I did not read the diff.

hoklims added a commit to hoklims/semctx that referenced this pull request Sep 23, 2026
…c exits

On the hosted runners the parallel pass failed once on macOS (4 tests)
and once on Ubuntu (5 tests) in the first three runs of #178, never on
Windows. Every failure was a test that ran into its own budget, followed
by Bun killing a dangling child.

Reproduced under WSL Ubuntu with Bun 1.4.0. The full suite at 4 workers,
stopped early, had 40 timeouts and 73 dangling children. --parallel=4
over apps and packages/mcp-server hit it in 2 of 5 runs: 3 tests ran
into their budgets in one, another hung past 25 minutes. The same files
with --isolate and no --parallel passed 441/441. A watcher
caught the state: the git child spawned by spawnSync had exited and stayed
a zombie, the worker held no pipe any more, and its main thread spun at a
full core. This is oven-sh/bun#34069 (open, reproduced on 1.4.0 and 1.4.2;
fix proposed in oven-sh/bun#40078): spawnSync loses a child's exit on the
posix event loops, and parallel workers multiply the exposure.

Only Windows now runs the parallel pass. Linux and macOS run one
sequential pass over the same roots, the canonical command plus a JUnit
report, and the same completeness check covers both plans: every
discovered file reported by exactly one pass, each pass reporting exactly
its expected files. Windows is the critical path of the required verdict,
so the gain stays; re-enable the other platforms once a fixed Bun is
pinned and five hosted runs per OS are green.

Refs: HOK-822
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@robobun

robobun commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

The spin has a third CI signature, and it is the most frequent one I found: a test that times out at the bun test deadline with killed 1 dangling process.

File. test/js/web/fetch/body-clone.test.ts, test "a body over a FIFO opened by path is still teed". Its first statement is Bun.spawnSync({ cmd: ["mkfifo", fifo] }): pidfd, stdout and stderr, three polls. It is the only spawnSync call of the file.

CI data. The flaky annotation names this test in 35 of the 85 builds from 119960 to 120061 that have annotations: 33 on :alpine: 3.23 aarch64, 2 on :alpine: 3.23 x64. It is always in the bun test --parallel=3 batch, and the file passes alone on the retry. Build 120061, alpine aarch64, shard 5:

test/js/web/fetch/body-clone.test.ts:
killed 1 dangling process
✗ clone() of a body over an unread native stream keeps the Blob behind it > a body over a FIFO opened by path is still teed [90001.50ms]
  ^ this test timed out after 90000ms.

The history follows allocation history, as the Notes of this PR predict:

  • 247 sampled builds from 113000 to 117515: 0 hits. First hit: build 117516 (2026-09-18). The file moved from shard 13 (build 116800) to shard 5 (build 118600) in that window.
  • 60 sampled builds from 118930 to 119640: 0 hits. Hits again from build 119650.
  • In all six failing buckets I read (builds 118600, 118900, 118920, 119650, 119980, 120061) the file sorted before body-clone.test.ts is a test/js/third_party/jsonwebtoken/*.test.js file. In the two buckets from the quiet window (builds 119100, 119300) it is grpc-js/test-deadline.test.ts.

Reproduction with the unmodified test. Put the fixture of spawnsync-isolated-event-loop.test.ts from this PR (three Bun.stderr.writer() sinks that the GC finalizes inside a warmed-up spawnSync) as a test in front of the FIFO test, in the same describe. Run it with stderr on a pipe:

$ BUN_JSC_slowPathAllocsBetweenGCs=5 bun test --timeout=6000 body-clone-e2e.test.ts -t "make drift|FIFO" 2>&1 | cat
collectedDuringCall 3
(pass) ... > make drift
killed 1 dangling process
(fail) ... > a body over a FIFO opened by path is still teed [6021.63ms]
  ^ this test timed out after 6000ms.

Release canary 6808912 (linux x64 musl), release canary 367d939 (linux x64 glibc) and a debug build give this output. The dangling process is the mkfifo child, a zombie. Without the fixture I had 0 hits on x64 in 230 runs of the 70-file bucket of build 120061. I did not build this branch, so I have no run of it against this reproduction.

#43862 makes that test create its FIFO with the mkfifo test helper, so the file stops calling spawnSync. It does not touch the bug.

@robobun

robobun commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator Author

CI impact of this bug

Four flaky test files in CI match the two symptoms in this PR. At least one of the four fails in 279 of the last 400 finished builds (Buildkite builds 120072 to 120698, 2026-09-23 to 2026-09-25). All four fail only in the bun test --parallel batch and pass alone.

Test file Builds CI signature Symptom
test/napi/node-napi-tests/test/js-native-api/test_object/do.test.ts 203 Interrupted while still running: ... (245s), then (worker crashed) 1: spawnSync with no timeout never returns
test/internal/build-toolchain-identity.test.ts 111 Cannot tell which .../current/clang this is after 30001 ms (build 120675) 1: spawnSync spins until its 30 s timeout and returns no stdout
test/js/bun/shell/commands/seq.test.ts 51 all 14 stderr assertions receive "" 2: EEXIST on the next dup of fd 2
test/napi/uv.test.ts 42 Failed with exit code 17 2: EEXIST on the next dup of fd 2

How the signatures were matched

The fixtures below ran on 1.4.3-canary.1+367d939d9, the parent of this branch, on Linux x64 with stderr on a pipe.

  • One garbage Bun.stderr.writer(): the next Bun.spawnSync never returns, in 5 of 5 runs. The parent is in state R. Its user time grows by 100 ticks per second and its system time does not grow. The child is [sh] <defunct>. runner: dump the process tree and stacks of a stalled parallel batch before killing it #41454 asks if the stalled worker blocks or spins: it spins.
  • Three garbage writers: a Bun.spawnSync with stdout: "pipe", stderr: "pipe" and timeout: 5000 returns after 5002 to 5043 ms with exitedDueToTimeout: true and an empty stdout, in 5 of 5 runs.
  • Four garbage Bun.file(2).writer(), then Bun.gc(false) and Bun.spawnSync(["sleep", "0.2"]): $`seq` gives stderr: "" with exit code 1, and the next Bun.file(2).writer() throws EEXIST. This happens in each run where the writers are collected inside the spawnSync (2 of 8 runs), and in no other run. In a run with the same collection, but with fds 7 to 10 occupied before the shell runs, the shell and the writer work.
  • With no garbage writers, the two spawnSync calls return in 303 ms.

These fixtures did not run against this branch.

State of this PR's CI

The last CI run of this branch (build 117738) is red on one file: test/js/bun/spawn/spawn.test.ts on x64-asan. That failure is the LeakSanitizer waitpid flake that #42911 and #43646 fix.

Fixtures

spin.js. Run it as BUN_JSC_slowPathAllocsBetweenGCs=5 bun spin.js <writers> <piped|inherit> 2> >(cat >/dev/null).

const n = Number(process.argv[2]);
const shape = process.argv[3];
globalThis.writers = [];
const refs = [];
for (let i = 0; i < n; i++) {
  const writer = Bun.stderr.writer();
  writer.write("");
  writer.flush();
  globalThis.writers.push(writer);
  refs.push(new WeakRef(writer));
}
await Bun.sleep(0);
const first = { cmd: ["echo", "first"], stdout: "pipe", stderr: "ignore", timeout: 2000 };
Bun.spawnSync(first);
globalThis.writers = null;
Bun.spawnSync(first);
let collectedDuringCall = 0;
for (let i = 0; i < refs.length; i++) if (refs[i].deref() === undefined) collectedDuringCall++;
const t0 = performance.now();
const result =
  shape === "piped"
    ? Bun.spawnSync({ cmd: ["sh", "-c", "sleep 0.3; echo 'clang version 21.1.8'"], stdin: "ignore", stdout: "pipe", stderr: "pipe", timeout: 5000 })
    : Bun.spawnSync({ cmd: ["sh", "-c", "sleep 0.3"], stdin: "inherit", stdout: "ignore", stderr: "inherit", timeout: 5000 });
const ms = Math.round(performance.now() - t0);
console.log(JSON.stringify({ n, shape, collectedDuringCall, ms, exitedDueToTimeout: result.exitedDueToTimeout, stdout: result.stdout?.toString() }));

shell.js. Run it as bun shell.js <plain|occupy> 2> >(cat >/dev/null).

import { fstatSync, openSync, readdirSync } from "node:fs";
import { $ } from "bun";
const stderrInode = fstatSync(2).ino;
const stderrDups = () =>
  readdirSync("/proc/self/fd")
    .map(Number)
    .filter(fd => {
      if (fd === 2) return false;
      try {
        return fstatSync(fd).ino === stderrInode;
      } catch {
        return false;
      }
    });
for (let i = 0; i < 4; i++) Bun.file(2).writer().write("");
const before = stderrDups();
Bun.gc(false);
Bun.spawnSync({ cmd: ["sleep", "0.2"] });
const after = stderrDups();
const occupied = [];
if (process.argv[2] === "occupy") for (let i = 0; i < 4; i++) occupied.push(openSync("/dev/null", "r"));
const shell = [];
for (let i = 0; i < 2; i++) {
  const res = await $`seq`.nothrow();
  shell.push({ exitCode: res.exitCode, stderr: res.stderr.toString() });
}
let writerError = null;
try {
  const w = Bun.file(2).writer();
  w.write("writer after\n");
  await w.flush();
} catch (e) {
  writerError = String(e?.code ?? e);
}
console.log(JSON.stringify({ before, after, occupied, shell, writerError }));

@robobun

robobun commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

test/internal/build-toolchain-identity.test.ts is one more casualty of this bug. It is now the second most frequent flaky file in CI.

What CI shows. In the last 400 finished builds (121144 to 121668, 2026-09-27 to 2026-09-29) the file fails in the bun test --parallel batch in 197 builds and passes alone each time. All six Linux lanes without ASAN are affected (debian 13, ubuntu 25.04, alpine 3.23, x64 and aarch64).

BuildError: Cannot tell which /tmp/buntmp-nucydG/tool-identity_Lo7uI9/current/clang this is
      at toolIdentity (scripts/build/tools.ts:282:6)
      at writeToolIdentities (scripts/build/tools.ts:261:49)
✗ a file changes when its tool's path resolves to another tool, and only then [30003.42ms]

toolIdentity runs spawnSync(exe, ["--version"], { timeout: 30_000, stdio: ["ignore", "pipe", "pipe"] }) on a two-line /bin/sh script. That call has three polls (pidfd, stdout, stderr). The test takes 30 s because the call ends at its own timeout.

Probe on main. Release canary 1.4.3-canary.1+ad60a9b7b, linux x64, BUN_JSC_slowPathAllocsBetweenGCs=5, stderr is a pipe. The script is the fixture of this PR's test with the last call replaced by the toolIdentity call (timeout 3000 ms):

writers finalized inside the earlier Bun.spawnSync result of the --version call
3 {"ms":3024,"error":"ETIMEDOUT","status":0,"stdout":""}
0 {"ms":16,"error":null,"status":0,"stdout":"clang version 21.1.8 ...\n"}

The child exits with status 0 at once. The parent does not poll until the timeout closes the pipes.

Not verified. I did not identify which objects supply the count of three in the CI batch. The batch matters: the x64-asan lane runs the file in a different bucket and has 0 failures in the same 400 builds.

This branch has a merge conflict with main at this time.

Conflicts:

- src/io/ParentDeathWatchdog.rs: main renamed `Pollable` to `Flags`. The
  `register` call keeps the `EventLoopCtx` argument of this branch.
- src/runtime/socket/WindowsNamedPipe.rs: main replaced the
  `impl_streaming_writer_parent!` invocation with hand-written Windows
  impls. Take main's file; the `uws_loop` arm this branch removed has no
  user left there.
- src/runtime/timer/mod.rs: main dropped a clippy allow on
  `increment_timer_ref`/`increment_immediate_ref`. Keep this branch's
  signatures, which take the `EventLoopCtx`.
Conflict: src/io/posix_event_loop.rs. main added `Flags::Tty` at the end
of the enum where this branch added `Flags::SpawnSyncLoop`. Keep both.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Merged main into the branch (0dedb9f). The PR was in conflict with main.

  • Four files conflicted, and all four are mechanical. ParentDeathWatchdog.rs: main renamed Pollable to Flags. WindowsNamedPipe.rs: main's hand-written Windows impls replace the macro invocation, so this PR no longer touches the file. timer/mod.rs: a clippy allow above the two ref functions. posix_event_loop.rs: Flags::Tty from process.stdout/stderr: clear O_NONBLOCK for spawned children, don't drop console output on EAGAIN #43868 next to Flags::SpawnSyncLoop. The design did not change.
  • The two new test cases still fail without the src/ change. On a debug build of main dc3660d with this branch's tests, the probe spawnSync hits its 2s timeout with empty stdout, and the second writer gets EEXIST: file already exists, epoll_ctl. Both files pass on the merged branch.
  • build-toolchain-identity.test.ts (Cannot tell which /…) is flaky on 47 of the 60 most recent finished builds. A stand-in for its child_process.spawnSync call, run after a GC inside an earlier Bun.spawnSync finalized three Bun.stderr.writer() sinks, returns ETIMEDOUT in 3 of 3 runs on main and returns normally in 9 of 9 runs on this branch.
  • CI build 121730 on the merged head passes, 181 of 181 jobs. Its annotations name neither build-toolchain-identity.test.ts nor 10887.test.ts.
  • The PR body is rewritten for the current state. It now has a Downsides section with the measured costs. The release binary size comparison is still running and I will add it to that section.

A release build of the previous commit had 66,560 more bytes of .text
than its merge base on linux-x64. The loop lookup that `KeepAlive::ref_`
and `unref` gained was inlined into each of their several hundred
callers.

`EventLoopCtx::loop_ref` now returns the spawnSync-loop bit itself, and
it and `loop_unref_for` are `#[inline(never)]`. A caller keeps a status
check and one call.

`FilePoll::register_with_fd` and `unregister_with_fd` resolve the loop
once and hand it to the private helpers, which take `&mut Loop` again
as on main.
Comment thread src/io/lib.rs Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread test/js/bun/spawn/spawnSync.test.ts
@robobun

robobun commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up on the binary size, which my last comment left open.

  • The first merged revision (0dedb9f) made release .text on linux-x64 66,560 bytes larger than the merge base. The loop lookups that KeepAlive::ref_ and unref gained were inlined with them into 572 symbols.
  • 87f1190 moves that bookkeeping into two out-of-line helpers on EventLoopCtx (loop_ref and loop_unref_for). FilePoll::register_with_fd and unregister_with_fd resolve the loop once, as on main.
  • At 2d3f147, .text is 6,656 bytes smaller than the merge base ba3f27d. CI's binary-size annotation on build 121775 shows no target larger than main.
  • The behaviour did not change. Both new test cases pass, and build 121775 passes on all 181 jobs.

The two builds of this branch since the merge (121730 and 121775) have no annotation for build-toolchain-identity.test.ts or 10887.test.ts. Of the 118 other finished builds from 121608 to 121781, 92 have one for the first and 8 have one for the second. Two builds are a small sample, so the local fail-before runs in the PR body stay the proof.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: spawnSync never returns: child exit lost (child stays zombie), wait loop busy-spins at 100% CPU re-registering a finished pipe reader (macOS ARM64)

3 participants