Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/bun-types/bun.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7220,10 +7220,10 @@ declare module "bun" {
*
* Entries beyond index 2 are `number` for `"pipe"` and `"socket-fd"` slots and,
* on POSIX, for slots where a raw file descriptor was supplied (the same fd is
* returned). For `"pipe"`, the subprocess owns and closes the fd. For
* `"socket-fd"` and raw-fd slots, the fd remains owned by the caller and is
* never closed by the subprocess. Other slots — including raw fds on Windows —
* are `null`.
* returned). On POSIX, reading this property transfers ownership of any
* `"pipe"` fds to the caller, who is then responsible for closing them; the
* subprocess will not close them. `"socket-fd"` and raw-fd slots are likewise
* caller-owned. Other slots — including raw fds on Windows — are `null`.
*/
readonly stdio: [null, null, null, ...(number | null)[]];

Expand Down
11 changes: 11 additions & 0 deletions src/runtime/api/bun/subprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,17 @@ impl Subprocess<'_> {
}
}
}
// The raw fd numbers are now visible to JS and the caller owns them.
// Downgrade so finalize_streams never closes a number JS may have
// already closed (whose value the kernel may have since recycled).
#[cfg(not(windows))]
this.stdio_pipes.with_mut(|pipes| {
for slot in pipes.iter_mut() {
if let ExtraPipe::OwnedFd(fd) = *slot {
*slot = ExtraPipe::UnownedFd(fd);
}
}
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Ok(array)
}

Expand Down
5 changes: 3 additions & 2 deletions src/spawn_sys/spawn_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,9 @@ pub struct PosixSpawnResult {

/// Entry in `extra_pipes` for a stdio slot at index >= 3.
pub enum ExtraPipe {
/// We created this fd (e.g. socketpair for `"pipe"`); expose it via
/// `Subprocess.stdio[N]` and close it in `finalizeStreams`.
/// We created this fd (e.g. socketpair for `"pipe"`); `finalizeStreams`
/// closes it. Downgraded to `UnownedFd` once `.stdio` is read (the caller
/// then owns the raw number and is responsible for closing it).
OwnedFd(Fd),
/// The caller supplied this fd in the stdio array; expose it via
/// `Subprocess.stdio[N]` but never close it — the caller retains ownership.
Expand Down
39 changes: 39 additions & 0 deletions test/js/bun/spawn/spawn.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,45 @@ describe("close handling", () => {
expect(() => fstatSync(fd as number)).toThrow(expect.objectContaining({ code: "EBADF" }));
},
);

it.skipIf(isWindows)("'pipe' at index >= 3: reading .stdio transfers fd ownership to the caller", async () => {
// Once .stdio exposes the raw fd number, JS owns it; the Subprocess
// finalizer must not close that number again at GC time (the kernel may
// have recycled it). Run in a child so a debug abort shows as exit != 0.
const fixture = /* js */ `
const fs = require("node:fs");
let hits = 0;
for (let i = 0; i < 4; i++) {
let p = Bun.spawn({
cmd: ["/bin/sh", "-c", "printf hi >&3"],
stdio: ["ignore", "ignore", "ignore", "pipe"],
});
await p.exited;
const fd = p.stdio[3];
if (typeof fd !== "number") throw new Error("stdio[3] not a number: " + fd);
const b = Buffer.alloc(8);
if (fs.readSync(fd, b) !== 2 || b.subarray(0, 2).toString() !== "hi")
throw new Error("stdio[3] unreadable");
fs.closeSync(fd);
const victim = fs.openSync(process.execPath, "r");
p = null;
Bun.gc(true);
await Bun.sleep(0);
Bun.gc(true);
try { fs.fstatSync(victim); } catch { hits++; }
try { fs.closeSync(victim); } catch {}
}
if (hits) throw new Error("finalizer closed " + hits + "/4 recycled fds");
console.log("PASS");
`;
await using proc = spawn({
cmd: [bunExe(), "-e", fixture],
env: bunEnv,
stdio: ["ignore", "pipe", "pipe"],
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "PASS", stderr: "", exitCode: 0 });
});
});
});

Expand Down
Loading