Startup: reopen fd 0-2 as /dev/null only when the fd is closed

[robobun]

Problem

• With an O_PATH descriptor on fd 1 or 2, every bun command dies with panic: abort() called. Bun creates this state itself: after fs.closeSync(1), fs.realpathSync() leaves one on fd 1, and a bun child that inherits it aborts.
• bun_initialize_process (src/jsc/bindings/c-bindings.cpp) reads isatty(fd) == 0 && errno == EBADF as "closed". An open O_PATH descriptor gives the same error, and the code copies /dev/null over it with dup2.

Fix

• openDevNullIfStdioIsClosed confirms the EBADF with fcntl(fd, F_GETFD). Only a closed fd becomes /dev/null. An open descriptor stays, as in node 26.
• A closed slot is the lowest free number, so open("/dev/null") returns it. The spare fd, the dup2 path and both assertions are gone.
• Verified: test/js/bun/spawn/spawn.test.ts (9 fd tables, bun 1.4.3 fails 7). All 27 tables of {pipe, closed, O_PATH}³ match node.
• Self-reviewed: 7 concerns raised, 6 addressed. Open: Bun.$ exit code 65527, not tracked.

Background

• bun_initialize_process runs first in main and fills closed fds 0 to 2, so a later open cannot take a stdio number.
• An O_PATH descriptor permits no I/O: read, write and ioctl fail with EBADF.
• Considered: keep the replacement and repair its bookkeeping. It still overwrites an open descriptor.

Downsides

• Behavior change: an O_PATH fd 0 stays, so reads give EBADF, not EOF. On a directory process.stdin throws EISDIR until process.stdin: end instead of throwing EISDIR when fd 0 is a directory #41484.
• An O_PATH fifo or socket on fd 1 or 2 reaches a process.stdout defect (node:fs: write stdio synchronously when the FileSink cannot be opened #41444) where startup aborted before.
• Cost: one more fcntl per closed stdio slot. Open stdio: 5 → 5 syscalls, 56 → 52 instructions, 701 → 622 bytes.

Notes

How it was found. An automated stdio matrix (fd 0 to 2 of every kind, bun against node) plus a read of the dup2 check. No user reported it. The same err != 0 defect was found once before, in #27041 (closed by the stale sweep after the Rust rewrite).

Repro (bun 1.4.3-canary.1+367d939d9):

import os, subprocess
fd = os.open("/tmp", os.O_PATH)
subprocess.run(["bun", "--version"], stdout=fd).returncode                          # -6
subprocess.run(["bun", "--version"], stderr=fd, stdout=subprocess.PIPE).returncode  # -6

From bun alone, no O_PATH in user code:

const fs = require("node:fs");
fs.closeSync(1);
fs.realpathSync("/usr/lib");   // fd 1 is now bun's own O_PATH descriptor on /usr/lib (#43844)
Bun.spawnSync({ cmd: [process.execPath, "--version"], stdio: ["ignore", "inherit", "pipe"] }).signalCode; // "SIGABRT"

isatty() is ioctl(fd, TCGETS). For fd 1 = O_PATH the old sequence was ioctl(1) = EBADF, openat("/dev/null") = 3, dup2(3, 1) = 1, then if (err != 0) abort(). dup2 returns the target fd, so only fd 0 passed. Now it is ioctl(1) = EBADF, fcntl(1, F_GETFD) = 0, and nothing else.

The faces on main, one predicate

1. fd 1 or 2 = O_PATH: SIGABRT at startup (release and debug).
2. fd 0 = O_PATH: no abort, the descriptor is replaced by /dev/null.
3. fd 0 = O_PATH and fd 1 or 2 closed: the /dev/null fd lands on the closed slot and is then copied over fd 0. The spare fd stays on a stdio number: a debug build fails ASSERTION FAILED: devNullFd_ == -1 || devNullFd_ > 2, and in release that /dev/null is not recorded in bun_is_stdio_null. With fd 1 and fd 2 both closed, release aborts too.

The first version of this PR only changed err != 0 to err < 0. A review found that this turned face 1 into face 2 and extended face 3 to "fd 1 = O_PATH, fd 2 closed". This version removes the cause instead.

27 fd tables {pipe, closed, O_PATH}³, fds 0 to 2 and fs.writeSync results compared with node 26 (CLOEXEC bit ignored):

build	killed by a signal	equal to node
main, release	16	8
first version of this PR, debug	7	8
this PR, debug	0	27

Syscalls over the same 27 tables, inside bun_initialize_process: main issues a dup2 in 19 tables and opens /dev/null 12 times on a number that was not a closed slot. This PR issues no dup2, one open per closed slot (27 of 27 land on the closed slot) and one fcntl(F_GETFD) per EBADF slot.

Prior art. Probe with fcntl(F_GETFD), open /dev/null, require the returned number to be the slot, otherwise stop: this is Go's runtime.checkfds (src/runtime/fds_unix.go) and glibc's check_one_fd (csu/check_fds.c). Node probes with fstat() and copies /dev/null over the slot with dup2 when open() returns another number. Node added that dup2 in nodejs/node#44461, after its older open-and-compare-or-abort code aborted on FreeBSD: there fstat() gives EBADF for a revoked tty that still occupies the slot. Here the open runs only after fcntl(F_GETFD) says the slot is free, and F_GETFD reads the descriptor table only, so an occupied slot never reaches the open.

When /dev/null cannot be opened for a closed slot the process still aborts, as on main (there through dup2(-1, fd)), as in node and Go. #27041 proposed to continue with the slot closed (for #15661, macOS App Sandbox). That leaves a stdio number free for the next open (#43844), so it is a separate decision and not part of this PR.

Measurements

• Startup syscalls with open stdio (ptrace trace of the function): three pipes 5 → 5, three ttys 10 → 10.
• Closed slot: ioctl + openat → ioctl + fcntl + openat, at most 3 extra fcntl per process. O_PATH slot: openat + dup2 + abort → fcntl only.
• .text, release flags: bun_initialize_process 547 + lambda 154 = 701 bytes → bun_initialize_process 515 + openDevNullIfStdioIsClosed 107 = 622 bytes. Measured by compiling c-bindings.cpp with the release compile command and running the ThinLTO backend on that module. For the unchanged file this gives 547 and 154, the same as nm -S on the release binary.
• Instructions executed inside bun_initialize_process per start (gdb nexti count): 56 → 52 with non-tty stdio, 101 → 93 with three ttys. The baseline is from the release binary. The new count is from a harness linked against the release-flag object, and the same harness gives 56 and 101 for the unchanged file.
• The fcntl is in the helper, not in the loop condition. In the loop condition it cost two more instructions per start (one more callee-saved register).

What happens with an O_PATH descriptor left on a slot

• bun --version, console.log, console.error, an uncaught exception: no crash, the write fails with EBADF and is dropped.
• fs.writeSync(1), Bun.write(Bun.stdout): EBADF. process.stdout.write on an O_PATH file, directory or character device: the callback and an 'error' event get EBADF.
• fd 0 = O_PATH directory: process.stdin throws EISDIR, the same as bun x.js < /tmp does today (process.stdin: end instead of throwing EISDIR when fd 0 is a directory #41484 makes both end like node). fd 0 = O_PATH file: 'error' EBADF.
• A child spawned with stdio: "inherit" gets the caller's descriptor.

Open PRs that edit the same block. #37128 gets 3 conflict regions in bun_initialize_process from this change (none against main). #37260 has 1 region in this file with or without it. #35477, #38843 and #39775 merge cleanly. Each of them still carries the setDevNullFd lambda. When one is rebased, keep openDevNullIfStdioIsClosed(fd) and drop the lambda, devNullFd_ and the trailing ASSERT and close: the lambda is the abort. The new test block fails if it comes back.

Not in this PR

• process.stdout / process.stderr when Bun.file(fd).writer() throws (an O_PATH fifo or socket): $assert(underlyingSink) in getStdioWriteStream fails on a debug build, and on release the writes after the first EBADF never call back. Main reaches this today with fs.closeSync(1); fs.openSync(fifo, O_PATH). node:fs: write stdio synchronously when the FileSink cannot be opened #41444 is the open PR for it.
• Bun.$ builtin echo exits with 65527 when stdout is not writable (bun x.js 1</dev/null on main). Found during this work, not tracked yet.
• The Windows arm of the function is unchanged. It closes the slot before it opens nul.

Self-review. Raised: say how the bug was found, list the open PRs in the same block with a resolution rule, cite #27041, state why Node has a dup2 here, link #41444 and #41484, run the closed-slot path on macOS (the two closed-only tables now run on every POSIX platform), and track the Bun.$ exit code. The last one is still open.

Tests run. bun bd test test/js/bun/spawn/spawn.test.ts on the debug ASAN build: 187 pass, 8 skip, 0 fail, and the new block passes 9 of 9. On bun 1.4.3 the new block fails 7 of 9. The two closed-only tables pass there: they guard the rewritten closed path and also run on macOS. With node as the child all 9 expectations hold.

---

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/bun/spawn/spawn.test.ts

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Essentials
• Run ID: eb3a07bc-8283-4085-8396-3ee1b6ace01a

📥 Commits

Reviewing files that changed from the base of the PR and between 5259adb and d4c1f4b.

📒 Files selected for processing (2)

• src/jsc/bindings/c-bindings.cpp
• test/js/bun/spawn/spawn.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

Walkthrough

Changes

Standard descriptor initialization

Layer / File(s)	Summary
Open /dev/null for closed standard descriptors src/jsc/bindings/c-bindings.cpp, test/js/bun/spawn/spawn.test.ts	bun_initialize_process opens /dev/null directly for each closed standard descriptor and retries if open returns EINTR. POSIX tests check closed slots, Linux O_PATH slots, write results, child exit status, and descriptor cleanup.

Suggested reviewers: jarred-sumner, cirospaciari, dylan-conway

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to d4c1f

Startup now replaces only descriptors that are actually closed, and POSIX tests cover the closed and O_PATH cases. No concrete merge-blocking risk remains.

🚥 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: Bun now reopens standard descriptors as /dev/null only when they are closed.
Description check	✅ Passed	The description explains the problem, fix, behavior changes, and verification results. It does not use the template’s exact headings, but it provides the information those sections request.

---

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

[robobun]

Status: reproduced on bun 1.4.3 with fd = os.open("/tmp", os.O_PATH) passed as stdout or stderr of bun --version (exit -6, SIGABRT), and from bun alone with fs.closeSync(1); fs.realpathSync("/usr/lib") followed by a bun child that inherits stdout.

The PR fixes the cause: startup confirms isatty()'s EBADF with fcntl(fd, F_GETFD) and opens /dev/null only on a closed fd. The new block in test/js/bun/spawn/spawn.test.ts (9 stdio fd tables) fails 7 of 9 on bun 1.4.3 and passes on this branch.

CI (build 123286, d4c1f4b): every lane that ran is green. That is Linux x64 and aarch64 (glibc, musl, ASAN), macOS aarch64, and Windows x64 and aarch64. The two darwin x64 - test-bun jobs wait for an agent. Ready for review. The decision for a maintainer is the behaviour change in Downsides: an O_PATH descriptor on fd 0 stays in place and is no longer replaced by /dev/null.

[claude]

LGTM — dup2 returns the target fd on success, so != 0 was wrong for fds 1 and 2; < 0 is the correct check.

What was reviewed:

• Verified dup2(2) semantics and that the retry loop already used < 0, so only the abort check was inconsistent.
• Checked the other dup2 call sites in src/jsc/bindings/bun-spawn.cpp — they already compare against -1, so this was the only instance of the bug class.
• Test follows harness conventions: await using, output asserted before exitCode, fd closed in finally, only one pipe so no drain deadlock, and O_PATH (0o10000000) is Linux-only so the describe.if(isLinux) gate is correct.

Extended reasoning...

Overview

This PR changes one comparison in bun_initialize_process() in src/jsc/bindings/c-bindings.cpp: the post-dup2 check goes from if (err != 0) abort() to if (err < 0) abort(). dup2(oldfd, newfd) returns newfd on success and -1 on failure, so the old check aborted on a successful redirect of fd 1 or 2. The path is reachable when isatty() returns EBADF for an occupied fd — which happens on Linux for O_PATH descriptors — because in that case open("/dev/null") does not land on the target slot and the dup2 branch actually runs. A Linux-gated test in test/js/bun/spawn/spawn.test.ts spawns Bun with an O_PATH fd on stdout, then stderr, and asserts the process prints to the other stream and exits 0.

Security risks

None. This is process-startup stdio hygiene; the change relaxes an over-strict success check on a syscall whose return semantics are well-defined. No untrusted input parsing, no auth/crypto/permissions surface.

Level of scrutiny

Low. The fix is a textbook errno-check correction that matches the EINTR retry loop's own < 0 condition two lines above and the == -1 checks at the sibling dup2 sites in bun-spawn.cpp. The added comment is load-bearing (explains the non-obvious return value). The test is placed in the existing spawn test file, uses bunExe()/bunEnv/await using, drains the single pipe concurrently with proc.exited, asserts output before exit code, and cleans up the fd in finally.

Other factors

Per REVIEW.md's "fix the whole bug class" rule, I grepped for other dup2 return-value checks in src/jsc/bindings/ — the two in bun-spawn.cpp already use == -1, so no siblings share the defect. The O_PATH constant (0o10000000 = 0x200000) matches the Linux fcntl.h value. The PR description's note about an unrelated pre-existing flaky test in the same file is disclosed and not touched here.

[robobun]

^{Updated 4:59 AM PT - Oct 3rd, 2026}

✅ @robobun, your commit d4c1f4b883c17fc5f45b62c52e081fe4277a4af1 passed in Build #123286! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 41749

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

bun-41749 --bun

[claude]

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

[claude]

LGTM — the rewrite to probe with fcntl(F_GETFD) and open /dev/null only for a genuinely closed slot resolves both points from the earlier review, and the new mixed closed+O_PATH cells cover them.

What was reviewed:

• The "lower fds are all open, so open() returns this fd" invariant in openDevNullIfStdioIsClosed: holds on each of the three iterations since a slot is either left alone (fcntl succeeded) or just reopened; the abort() on open() failure is the same outcome the old dup2(-1, fd) path had.
• Behavior change for an O_PATH fd 0 (left in place, reads now fail with EBADF rather than reading /dev/null): intentional, matches node, and bun_is_stdio_null consumers in the shell interpreter just inherit the fd in that case.
• Test shape: it.concurrent.each matrix, fd 3 report asserted together with exitCode/signalCode, resources released in finally; the stdout/stderr pipes only receive a few bytes so leaving them undrained cannot block the child.

Extended reasoning...

The diff replaces the setDevNullFd dup2 lambda in bun_initialize_process (src/jsc/bindings/c-bindings.cpp, POSIX-only) with a 15-line static helper that probes fcntl(F_GETFD) and opens /dev/null only when the fd is really closed, and adds one describe block with a 9-cell startup matrix to test/js/bun/spawn/spawn.test.ts. It touches process startup but no security-sensitive surface (no auth, parsing of untrusted data, or privilege boundaries). The two inline comments from the previous run (fd 1 O_PATH + fd 2 closed hitting the debug ASSERT; serial it instead of it.concurrent) are both addressed in the current code and explicitly covered by test cells. The change is small, the invariant reasoning is checkable by inspection, the function is called once before any thread starts, and the changed paths are not under CODEOWNERS, which decided approve over defer.

[claude]

I re-reviewed the reworked version and found no bugs; the earlier concern about the spare /dev/null fd landing on another stdio slot no longer applies since the dup2 path is gone, and the tests now run concurrently. Because this changes startup behavior for every bun process on Linux/macOS/FreeBSD (an O_PATH or otherwise-EBADF-but-open descriptor on fd 0-2 is now left in place instead of replaced), a human look at that policy choice is still worthwhile.

What was reviewed:

• Traced the lowest-free-fd assumption in openDevNullIfStdioIsClosed: the loop handles 0,1,2 in order and each lower slot is either open, replaced, or aborted, so open() must return fd.
• Checked the non-Linux arms: Darwin/FreeBSD reach the fcntl(F_GETFD) recheck only for closed fds (or a revoked tty on FreeBSD, which now stays on its slot rather than being overwritten).
• Test block: fd 3 report pattern matches the existing stdio[3] tests in the file, resources are released in finally, O_PATH cells are Linux-only, and the O_PATH cells abort on an unfixed binary.

Extended reasoning...

The diff replaces the setDevNullFd lambda in bun_initialize_process (src/jsc/bindings/c-bindings.cpp) with a static helper that confirms EBADF via fcntl(F_GETFD) before opening /dev/null and aborts if the returned fd is not the target slot; the dup2 path, cached fd, and trailing close/ASSERT are deleted. It adds a nine-cell it.concurrent.each matrix to test/js/bun/spawn/spawn.test.ts covering closed and O_PATH stdio slots. No security-sensitive surface (auth, crypto, injection) is touched and no CODEOWNERS entry covers these files. Both of my earlier inline findings are addressed by the rework rather than patched around. Deferring rather than approving because the change alters process-startup semantics on all POSIX platforms for an open-but-EBADF descriptor, which is a behavior decision a maintainer should confirm.