node:fs: release descriptor-owning async jobs when their worker is gone

[robobun]

Symptom

Terminating a worker (or a worker exiting on its own) with fs.close() calls in flight leaks the file descriptors. A long-lived parent that spawns and terminates workers doing fs work accumulates fds until EMFILE.

// parent: spawn worker, terminate on first message, watch /proc/self/fd
// worker: const go = () => { const fd = fs.openSync(__filename, 'r'); fs.close(fd, go); };
//         for (let i = 0; i < 48; i++) go();

48 fds leaked over 20 rounds on current canary; 0 on builds before the worker teardown rework (the old teardown waited for in-flight fs work). The sibling leg leaks on every build: an in-flight fs.open() whose worker goes away leaks the descriptor it just opened.

Cause

Async fs operations run as Job<AsyncFSTask> on the work pool. When the worker's VM handle is already closed by the time the pool reaches the job, Job::run_on_pool skips C::run and the refused job is dropped. Two resources go with it:

• args::Close owns the fd the caller handed over; dropping the job without running it means the close(2) never happens.
• ret::Open = FD holds the fd a completed open produced; a completion refused at the handle (or released unrun at teardown) dropped it without a close.

Fix

• JobContext::run_refused (default no-op), called from run_on_pool's VM-gone branch: the one place a job learns its run will never happen. AsyncFSTask routes it to a per-op NodeFSDispatch::release_unrun hook; the Close op closes the descriptor it owns (there is nowhere left to report an error, matching what the completion-less teardown already implies).
• ret::Open becomes an owning newtype: FsReturn::fs_to_js takes the fd out when delivering it to JS, and Drop closes a still-valid fd. That covers the refused drop (pool thread), the unrun release (JS thread at teardown), and the Windows UVFSRequest drop with one mechanism.

Verification

• In-flight close/terminate repro above: 48 fds leaked over 20 rounds before, 0 after. The fs.open variant: 0 after (leaked on every build before).
• Two deterministic tests added to worker-refused-completion.test.ts, using BUN_DEBUG_TEST_WORKER_REFUSAL_GATE plus UV_THREADPOOL_SIZE=2: two parked stat completions pin the pool so the close jobs are provably still queued when the handle closes, and the open results are provably refused. Both fail on the unfixed build (2-8 fds leaked) and pass with the fix.
• worker-refused-completion.test.ts 17/17, worker-terminate-funnels.test.ts 10/10, fs.test.ts 458 pass, fs/promises.test.js 30 pass, worker_threads.test.ts 121 pass.
• cargo check -p bun_runtime for x86_64-pc-windows-msvc, aarch64-pc-windows-msvc, aarch64-apple-darwin.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/workers/worker-refused-completion.test.ts

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 12 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: 7fedc557-ca26-4265-bda4-f8f6f73fde46

📥 Commits

Reviewing files that changed from the base of the PR and between d2f48c6 and f7b9bb4.

📒 Files selected for processing (3)

• src/jsc/job.rs
• src/runtime/node/node_fs.rs
• test/js/web/workers/worker-refused-completion.test.ts

---

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

[claude]

I reviewed this PR and the bug hunting system found no issues. Because it adds a new lifecycle hook to the core JobContext carrier and changes ret::Open to an owning type with Drop, a human look at the cross-thread ownership reasoning would still be worthwhile.

What was reviewed:

• run_refused ordering in run_on_pool: called before done.finish(); the subsequent release_refused drop of off cannot double-close (FD is Copy, no Drop).
• ret::Open newtype flows: sync path (node_fs_binding.rs:54-57), async then, and Windows UVFSRequest::run_from_js_thread all go through fs_to_js, which mem::replaces to FD::INVALID so the guarded Drop is a no-op on the delivered path.
• close_unrun uses the same close_allowing_standard_io helper as NodeFS::close; safe on a pool thread with no VM borrow.
• On non-Windows UVFSRequest aliases AsyncFSTask, so the unrun = close_unrun hook is reachable for Close; tests correctly skipIf(isWindows).

Extended reasoning...

Overview

The PR fixes fd leaks when a worker terminates with async fs.close/fs.open jobs in flight. It touches three files: src/jsc/job.rs (adds JobContext::run_refused default-no-op hook, called from the VM-gone branch of run_on_pool), src/runtime/node/node_fs.rs (adds NodeFSDispatch::release_unrun per-op hook wired through the node_fs_ops! macro; changes ret::Open from a type alias for FD to an owning newtype with Drop; adds close_unrun), and two new deterministic fd-leak tests in worker-refused-completion.test.ts.

Security risks

None identified. The change only affects when close(2) is issued for descriptors the runtime already owns; there is no new user-input parsing, no auth/crypto, no privilege boundary.

Level of scrutiny

High. src/jsc/job.rs is the cross-thread job carrier that mediates VM lifetime, JsSide release, and pool-thread ownership — the module doc explicitly frames it around memory-safety invariants. Adding a hook that runs on a pool thread with no VM borrow, and changing a return type to have a Drop that issues a syscall, both require tracing every consumer to rule out double-close or use-after-close. I traced the sync dispatch path, the async then path, the Windows UVFSRequest completion path, and the three release paths (release_refused, release_unrun_on, normal then drop) and found the ownership handoff correct in each — fs_to_js neutralizes the fd via mem::replace(.., FD::INVALID) before the guarded Drop runs, and FD itself is Copy with no Drop so close_unrun followed by the args drop cannot double-close.

Other factors

• The macro extension (unrun = ...) is opt-in per op and only Close uses it, so the blast radius is contained.
• close_unrun reuses the exact helper (close_allowing_standard_io) that NodeFS::close already uses, matching the repo's "use the in-tree helper" rule.
• Tests use the existing BUN_DEBUG_TEST_WORKER_REFUSAL_GATE mechanism and poll the fd table with a deadline (no bare sleeps), gated to debug/ASAN builds like the surrounding tests.
• The PR author ran the full fs test suites and cross-target cargo check.

That said, this is not a mechanical change — it introduces a new lifecycle contract on a core trait and an owning Drop on a value that flows through generated dispatch code on three platforms. Per the approval guidelines (complex change touching critical code paths), I'm deferring rather than approving.

[robobun]

^{Updated 11:18 AM PT - Aug 9th, 2026}

❌ @robobun, your commit f7b9bb4 has 1 failures in Build #91020 (All Failures):

• test/cli/test/parallel.test.ts - code 1 on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 37278

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

bun-37278 --bun

[robobun]

CI: 195/196 jobs passed. The one red job is darwin 14 x64, failing on test/cli/test/parallel.test.ts (the >64MB result line test timing out at 60s), which also fails on main and is unrelated to this change. The remaining annotations are known-flaky tests that passed on retry. The diff itself is green on every lane, including the debug/ASAN lanes that run the new worker-refused-completion fd tests.

[robobun]

Re-checked on current main (97a4363) after the teardown rework in #38299. Both legs still leak, so this stays open; the branch needs a rebase onto the ticket model.

Observed on a debug build of main, with only this PR's tests applied (BUN_DEBUG_TEST_WORKER_REFUSAL_GATE renamed to what main has now, BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE):

• fs.close jobs still queued when the worker's teardown reaches its wait are handed back unrun (Job::run_on_pool sees the cancelled ticket) and released by dropping the job, so the descriptor in args::Close is never closed. The gate test fails with leaked: 8. Without the gate it reproduces the same way: UV_THREADPOOL_SIZE=2, both pool threads parked in a readFile of a FIFO, 8 fs.close() queued behind them, terminate(), then release the FIFOs: 8 descriptors leaked.
• ret::Open is still a bare FD, and an open completion that lands after terminate() (complete_erased with script no longer allowed, or the teardown's release) drops it without closing. The gate test fails with leaked: 2 (the two opens the 2-thread pool ran; the other six were handed back unrun and never opened anything). Without the gate: a worker running 32 concurrent fs.open/fs.close chains, terminated 10 times in a row, leaks 318 descriptors in the parent; the same loop with fs.stat chains leaks 0.

For the rebase: the ret::Open newtype with its Drop applies as is, since both release paths now drop the result on the JS thread. JobContext::run_refused has no equivalent any more; jobs that never ran are released through Job::release_unrun -> take in src/jsc/job.rs, which just drops both halves, so a per-op release for Close needs a hook there (or on AsyncFSTask) instead of in run_on_pool.