Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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