shell: close the stdio pipes of a subprocess whose stdin writer failed to start

[robobun]

Problem

• A shell command with a buffer stdin (cat < ${blob}, < ${bytes}) whose stdin writer fails to start leaks three sockets for the life of the process. The command itself fails as intended (bun: Cannot allocate memory, exit 1); the leak is in the cleanup. Reproduced on bun 1.4.0 and on main with an LD_PRELOAD shim that fails the writer's epoll_ctl registration, which is what fs.epoll.max_user_watches exhaustion does.
• ShellSubprocess::spawn_maybe_sync_impl (src/runtime/shell/subproc.rs) starts the stdin writer before the stdout/stderr readers. When that start fails, abort_after_failed_start drops the subprocess. The two PipeReaders were never started, so the parent end of each pipe is still in PipeReader::stdio_result, and nothing closes it: PosixBufferedReader only closes an fd it was started with. Two of the three fds.
• The same function never releases the stdin slot. Writable::Buffer holds the create() ref of the StaticPipeWriter, normally released by on_close_io when the writer closes, and a writer whose start() failed never closes. Writable::finalize only unrefs it; its comment relied on a RefPtr drop that does not exist (RefPtr has no Drop; the Zig version called deref() here). The writer, its Blob/ArrayBuffer source and the fd its failed start left in its poll all leak. The third fd.

Fix

• PipeReader owns the pipe fd until start() hands it to the reader (stdio_result.take()), and its Drop closes an fd that was never handed over. Correct because the reader closes the fd on every path where it was started, so the two never both close it, and a reader dropped without starting holds a fresh pipe end that nothing else references. This covers any path that drops an unstarted reader, not just today's stdin-first ordering.
• abort_after_failed_start empties the stdin slot, closes the writer and releases the slot's ref. The slot is emptied first because close() reports back through on_close_io, which must find nothing left to release; it runs through the raw pointer before the Box is reclaimed so that re-entry aliases nothing. Same shape as deinit_in_flight_io below it. It is also right for the other arm that reaches this function (a reader start failing after the writer started): close() then releases the writer's own start() ref via on_close, and this releases create()'s.
• What close() does to a writer whose registration failed, on main today: PosixBufferedWriter::start had already put the fd in a poll before epoll_ctl failed, so PosixBufferedWriter::close goes through PollOrFd::close_impl, which releases that poll (unregistering is a no-op, nothing was registered), hands the fd to Closer, and calls on_close synchronously. on_close skips the start() ref because started is false and reaches on_close_io, which sees the emptied slot. The deref() that follows is the last ref, and the writer's Drop finds the handle already closed. With io: leave the fd with the caller when a POSIX pipe writer fails to start #38354 the handle is already Closed when we get here, so close() does nothing and deref() alone frees the writer.
• io: leave the fd with the caller when a POSIX pipe writer fails to start #38354 (open) makes StaticPipeWriter::start close its fd itself on failure and hands the reader leak off to this PR. The two compose: with it, close() here finds no fd and deref() still frees the writer; the reader change is independent of it.
• Writable::finalize is unchanged in behavior; its comment and the Writable::Buffer doc now name the sites that release the ref.
• Test: test/js/bun/shell/shell-pipe-read-fault.test.ts, new shim mode SHELL_FAIL_EPOLL_WRITABLE. It fails EPOLL_CTL_ADD with EPOLLOUT on sockets the process itself created with socketpair(), so exactly the child's stdin writer fails and the shell's writers to the fixture's own stdio keep working. The fixture runs < ${blob} and < ${bytes} quietly and one un-quieted < ${blob} (capture readers), then lists /proc/self/fd by inode until it returns to the baseline. Unfixed: nine leaked sockets (three per command), all three commands failing with ENOMEM. Fixed: the same failures and an empty list. The wait is bounded at 2s so the unfixed run reports the list rather than timing out. 5/5 runs on the ASAN debug build.
• Also on the debug build: the rest of that file (the shim now also resets its per-fd state on syscall(SYS_close), which is how bun closes fds), bunshell.test.ts (423 pass), leak.test.ts, shell-write-fault, epipe, shelloutput, bunshell-file, bunshell-instance, exec, shell-hang, throw, lazy, pipeline_stack, shell-worker-terminate-leak, shell-cmdsub-crash; cargo clippy -p bun_runtime, cargo fmt, cargo check -p bun_runtime for aarch64-apple-darwin and x86_64-pc-windows-msvc. leak.test.ts's memleak_* cases and shell-blocking-pipe's heap-snapshot case time out on this machine identically with and without the change.

Background

• The shell runs an external command through ShellSubprocess. Each piped stdio is a socketpair: the child gets one end, the parent end is wrapped in a PipeReader (stdout/stderr) or, for a Blob/ArrayBuffer stdin, a StaticPipeWriter that writes the bytes into the child. The wrappers are created first and started afterwards; starting registers the fd with the event loop (epoll_ctl on Linux).
• On POSIX a reader reports a registration failure through a callback, so the writer's start() is the only stdio start that returns an error to the spawn, and therefore the only way abort_after_failed_start is reached today.
• RefPtr is bun's intrusive refcount handle. Dropping it does not release the ref; every holder must deref() explicitly. A StaticPipeWriter has two refs: create()'s, held by the owner's stdin slot and released in on_close_io, and start()'s, held while a write is in flight.
• on_close_io is how a stdio object tells its ShellSubprocess it closed; for stdin it empties the slot and releases create()'s ref. It runs from inside the writer's close(), which is why teardown has to empty the slot before calling close().
• An fd held by a registered poll is closed on the thread pool (Closer), which is why the test waits for the fd set to drain instead of reading it once.

Repro outside the test

// shim.c: cc -shared -fPIC -o shim.so shim.c -ldl
#define _GNU_SOURCE
#include <dlfcn.h>
#include <errno.h>
#include <stdarg.h>
#include <sys/epoll.h>
#include <sys/syscall.h>
static long (*real)(long, long, long, long, long, long, long);
long syscall(long n, ...) {
  va_list ap; long a, b, c, d, e, f;
  va_start(ap, n); a = va_arg(ap, long); b = va_arg(ap, long); c = va_arg(ap, long);
  d = va_arg(ap, long); e = va_arg(ap, long); f = va_arg(ap, long); va_end(ap);
  if (!real) real = dlsym(RTLD_NEXT, "syscall");
  if (n == SYS_epoll_ctl && (int)b == EPOLL_CTL_ADD && d && (((struct epoll_event *)d)->events & EPOLLOUT)) {
    errno = ENOSPC; return -1;
  }
  return real(n, a, b, c, d, e, f);
}

import { readdirSync, readlinkSync } from "node:fs";
const fds = () => readdirSync("/proc/self/fd").flatMap(fd => { try { return [fd + ":" + readlinkSync("/proc/self/fd/" + fd)]; } catch { return []; } });
const before = fds();
const r = await Bun.$`cat < ${new Blob(["data"])}`.quiet().nothrow();
await Bun.sleep(100);
console.log(r.exitCode, JSON.stringify(r.stderr.toString()), fds().filter(x => !before.includes(x)));

$ LD_PRELOAD=./shim.so bun repro.js        # bun 1.4.0, and main before this change
1 "bun: No space left on device: \n" [ "10:socket:[...]", "11:socket:[...]", "13:socket:[...]" ]
$ LD_PRELOAD=./shim.so bun-debug repro.js  # with this change
1 "bun: No space left on device: \n" []

Noticed on the way and handed off separately: the SIGTERM'd child on this path is never reaped (one zombie per failed command), because close_process tears the watcher down before the child has exited.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 31 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: de2af8fe-6428-4420-b006-72b304b12b69

📥 Commits

Reviewing files that changed from the base of the PR and between f7ad274 and df239b5.

📒 Files selected for processing (2)

• src/runtime/shell/subproc.rs
• test/js/bun/shell/shell-pipe-read-fault.test.ts

---

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

[robobun]

Status

• Reproduced on bun 1.4.0 and on main at f7ad274: with an LD_PRELOAD shim failing the stdin writer's epoll_ctl registration, each cat < ${blob} leaves three sockets open in /proc/self/fd (two unstarted reader ends, one writer end). Repro is in the PR description.
• Fix and test are in this PR; test/js/bun/shell/shell-pipe-read-fault.test.ts ("buffer stdin writer failed to start") lists nine leaked sockets without the change and passes with it on the ASAN debug build.
• Related: io: leave the fd with the caller when a POSIX pipe writer fails to start #38354 closes the writer's own fd inside StaticPipeWriter::start; this PR covers the reader fds and the writer's release, and composes with it.

[claude]

I reviewed this PR and didn't find any bugs. Because it touches refcount balancing and fd-ownership transfer on an unsafe-Rust error path with re-entrant callbacks (close() → on_close → on_close_io while a raw *mut ShellSubprocess is live), a human look would still be worthwhile.

Checked: the stdio_result.take() in PipeReader::start prevents a double-close (Drop only fires when start() never ran); refcount balance in abort_after_failed_start for both arms (writer's start() failed → started=false, on_close skips its deref; writer already started → on_close releases start's ref, buffer.deref() releases create's); the stdin slot is emptied through the raw this before close() re-enters on_close_io, so the re-entry sees Writable::Ignore and can't double-deref. All new Rust code is #[cfg(not(windows))]-gated.

Extended reasoning...

Overview

Fixes a three-fd leak per shell command whose buffer-stdin writer (< ${blob} / < ${bytes}) fails to register with epoll. Two changes in src/runtime/shell/subproc.rs: (1) PipeReader now owns its stdio_result fd until start() transfers it to the reader via .take(), and Drop closes any fd never handed over; (2) abort_after_failed_start now empties the Writable::Buffer stdin slot, calls close() on the writer, and releases the slot's create() ref. Plus doc-comment corrections (the old Writable::finalize comment referenced a RefPtr::Drop that doesn't exist). The test adds a SHELL_FAIL_EPOLL_WRITABLE shim mode that fails only EPOLLOUT registrations on process-created socketpairs, and a fixture that runs three cat < ${...} commands and asserts /proc/self/fd returns to baseline.

Security risks

None. This is an error-path resource leak fix; no user-facing input parsing, auth, or crypto.

Level of scrutiny

High. This is native memory-safety code — the repo's most-blocked review category. It touches intrusive-refcount balancing across a re-entrant callback (StaticPipeWriter::close → writer.close() → on_close → P::on_close_io back into the same ShellSubprocess), fd ownership transfer, and raw-pointer aliasing invariants under Stacked Borrows. I traced both arms that reach abort_after_failed_start (writer start failed vs. reader start failed after a successful writer start) and the refcounts balance in each, matching the shape of the neighbouring deinit_in_flight_io. The .take() change ensures the new Drop close and the started reader's own close are mutually exclusive. But the reasoning is subtle enough — particularly around whether writer.close() on a never-registered writer synchronously fires on_close and closes the poll's fd — that a maintainer familiar with StaticPipeWriter / PosixStreamingWriter internals should confirm.

Other factors

The PR description is exceptionally thorough (traces every ref, names the composing open PR #38354, lists every test file re-run, includes before/after repro output). The test is well-designed: bounded-poll wait for the async Closer thread pool, fd listing by inode so recycled numbers can't hide a leak, both Blob and ArrayBuffer stdin variants, both .quiet() and non-quiet reader setups, and the shim's new forget_fd on syscall(SYS_close) fixes a latent fd-recycling issue in the existing shim modes. The expectShellFault refactor into runShellFaultFixture is a clean extraction. Cross-platform checked (cargo check for darwin and windows targets). No prior human or bot reviews to consider.

[robobun]

On the one point left open above, what close() does to a writer whose registration failed: the writer here is a PosixBufferedWriter (not the streaming one), and its start() stores the fd in a new poll before calling epoll_ctl, so on main the failed writer still holds the fd in PollOrFd::Poll. PosixBufferedWriter::close then goes through PollOrFd::close_impl (src/io/pipes.rs): it reads the fd, releases the poll (the unregister is a no-op since nothing was registered), hands the fd to Closer, and invokes on_close synchronously before returning. StaticPipeWriter::on_close skips the start() deref because started is still false and calls on_close_io(Stdin), which finds the slot already set to Ignore. The deref() after that is the last ref; the writer's Drop calls writer.end() on a handle that is already Closed, which neither closes anything nor reports again.

With #38354 applied the failed start() leaves the handle Closed and closes the fd itself, so close() here is a no-op and only the deref() matters. Both states are what the new test observes: on main the third socket is the one this close() releases.

I added this trace to the Fix section of the description as well.

[robobun]

^{Updated 2:37 AM PT - Aug 14th, 2026}

✅ @robobun, your commit df239b5e7535a606f09ac293298e3855987199ea passed in Build #95497! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38367

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

bun-38367 --bun

[robobun]

^{Updated 1:05 AM PT - Aug 14th, 2026}

@robobun, your commit df239b5 is building: #95497