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