Skip to content

Navigation Menu

Sign in
Sign up

Commit dc578b1

Browse files
authored
spawn: return caller-supplied fds from Subprocess.stdio[N] (oven-sh#29629)
1 parent e8188f0 commit dc578b1

7 files changed

Lines changed: 71 additions & 27 deletions

File tree

‎packages/bun-types/bun.d.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7162,8 +7162,10 @@ declare module "bun" {
71627162
/**
71637163
* Access extra file descriptors passed to the `stdio` option in the options object.
71647164
*
7165-
* Entries beyond index 2 are `number` for `"pipe"` slots and `null` otherwise
7166-
* (including when a raw file descriptor was supplied — that fd remains owned by the caller).
7165+
* Entries beyond index 2 are `number` for `"pipe"` slots and, on POSIX, for slots
7166+
* where a raw file descriptor was supplied (the same fd is returned; it remains
7167+
* owned by the caller and is never closed by the subprocess). Other slots —
7168+
* including raw fds on Windows — are `null`.
71677169
*/
71687170
readonly stdio: [null, null, null, ...(number | null)[]];
71697171

‎src/bun.js/api/bun/js_bun_spawn_bindings.zig‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ pub fn spawnMaybeSync(
678678
});
679679

680680
const posix_ipc_fd = if (Environment.isPosix and !is_sync and maybe_ipc_mode != null)
681-
spawned.extra_pipes.items[@intCast(ipc_channel)]
681+
spawned.extra_pipes.items[@intCast(ipc_channel)].fd()
682682
else
683683
bun.invalid_fd;
684684

@@ -795,7 +795,7 @@ pub fn spawnMaybeSync(
795795
subprocess.ipc_data.?.socket = .{ .open = posix_ipc_info };
796796
}
797797
// uws owns the fd now (owns_fd=1); neutralize the slot so finalizeStreams doesn't double-close.
798-
subprocess.stdio_pipes.items[@intCast(ipc_channel)] = bun.invalid_fd;
798+
subprocess.stdio_pipes.items[@intCast(ipc_channel)] = .unavailable;
799799
} else {
800800
if (ipc_data.windowsConfigureServer(
801801
subprocess.stdio_pipes.items[@intCast(ipc_channel)].buffer,

‎src/bun.js/api/bun/process.zig‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,16 +1156,39 @@ pub const PosixSpawnResult = struct {
11561156
stdout: ?bun.FD = null,
11571157
stderr: ?bun.FD = null,
11581158
ipc: ?bun.FD = null,
1159-
extra_pipes: std.array_list.Managed(bun.FD) = std.array_list.Managed(bun.FD).init(bun.default_allocator),
1159+
extra_pipes: std.array_list.Managed(ExtraPipe) = std.array_list.Managed(ExtraPipe).init(bun.default_allocator),
11601160

11611161
memfds: [3]bool = .{ false, false, false },
11621162

11631163
// ESRCH can happen when requesting the pidfd
11641164
has_exited: bool = false,
11651165

1166-
pub fn close(this: *WindowsSpawnResult) void {
1167-
for (this.extra_pipes.items) |fd| {
1168-
fd.close();
1166+
/// Entry in `extra_pipes` for a stdio slot at index >= 3.
1167+
pub const ExtraPipe = union(enum) {
1168+
/// We created this fd (e.g. socketpair for `"pipe"`); expose it via
1169+
/// `Subprocess.stdio[N]` and close it in `finalizeStreams`.
1170+
owned_fd: bun.FD,
1171+
/// The caller supplied this fd in the stdio array; expose it via
1172+
/// `Subprocess.stdio[N]` but never close it — the caller retains ownership.
1173+
unowned_fd: bun.FD,
1174+
/// Nothing to expose for this slot (`"ignore"`, `"inherit"`, a path, or
1175+
/// the IPC channel after ownership has been transferred to uSockets).
1176+
unavailable: void,
1177+
1178+
pub fn fd(this: ExtraPipe) bun.FD {
1179+
return switch (this) {
1180+
.owned_fd, .unowned_fd => |f| f,
1181+
.unavailable => bun.invalid_fd,
1182+
};
1183+
}
1184+
};
1185+
1186+
pub fn close(this: *PosixSpawnResult) void {
1187+
for (this.extra_pipes.items) |item| {
1188+
switch (item) {
1189+
.owned_fd => |f| f.close(),
1190+
.unowned_fd, .unavailable => {},
1191+
}
11691192
}
11701193

11711194
this.extra_pipes.clearAndFree();
@@ -1324,7 +1347,7 @@ pub fn spawnProcessPosix(
13241347
try actions.chdir(options.cwd);
13251348
}
13261349
var spawned = PosixSpawnResult{};
1327-
var extra_fds = std.array_list.Managed(bun.FD).init(bun.default_allocator);
1350+
var extra_fds = std.array_list.Managed(PosixSpawnResult.ExtraPipe).init(bun.default_allocator);
13281351
errdefer extra_fds.deinit();
13291352
var stack_fallback = std.heap.stackFallback(2048, bun.default_allocator);
13301353
const allocator = stack_fallback.get();
@@ -1480,16 +1503,16 @@ pub fn spawnProcessPosix(
14801503
.dup2 => @panic("TODO dup2 extra fd"),
14811504
.inherit => {
14821505
try actions.inherit(fileno);
1483-
try extra_fds.append(bun.invalid_fd);
1506+
try extra_fds.append(.unavailable);
14841507
},
14851508
.ignore => {
14861509
try actions.openZ(fileno, "/dev/null", bun.O.RDWR, 0o664);
1487-
try extra_fds.append(bun.invalid_fd);
1510+
try extra_fds.append(.unavailable);
14881511
},
14891512

14901513
.path => |path| {
14911514
try actions.open(fileno, path, bun.O.RDWR | bun.O.CREAT, 0o664);
1492-
try extra_fds.append(bun.invalid_fd);
1515+
try extra_fds.append(.unavailable);
14931516
},
14941517
.ipc, .buffer => {
14951518
const fds: [2]bun.FD = try bun.sys.socketpair(
@@ -1508,14 +1531,14 @@ pub fn spawnProcessPosix(
15081531
try actions.dup2(fds[1], fileno);
15091532
if (fds[1] != fileno)
15101533
try actions.close(fds[1]);
1511-
try extra_fds.append(fds[0]);
1534+
try extra_fds.append(.{ .owned_fd=fds[0] });
15121535
},
15131536
.pipe => |fd| {
15141537
try actions.dup2(fd, fileno);
15151538
// The fd was supplied by the caller (a number in the stdio array) and is
1516-
// not owned by us. Record an invalid sentinel so finalizeStreams skips it
1517-
// instead of closing the caller's descriptor out from under them.
1518-
try extra_fds.append(bun.invalid_fd);
1539+
// not owned by us. Record it so `stdio[N]` returns the caller's fd, but
1540+
// mark it unowned so finalizeStreams leaves it open.
1541+
try extra_fds.append(.{ .unowned_fd=fd });
15191542
},
15201543
}
15211544
}
@@ -1546,7 +1569,7 @@ pub fn spawnProcessPosix(
15461569
.result => |pid| {
15471570
spawned.pid = pid;
15481571
spawned.extra_pipes = extra_fds;
1549-
extra_fds = std.array_list.Managed(bun.FD).init(bun.default_allocator);
1572+
extra_fds = std.array_list.Managed(PosixSpawnResult.ExtraPipe).init(bun.default_allocator);
15501573

15511574
if (comptime Environment.isLinux) {
15521575
// If it's spawnSync and we want to block the entire thread

‎src/bun.js/api/bun/subprocess.zig‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ process: *Process,
1717
stdin: Writable,
1818
stdout: Readable,
1919
stderr: Readable,
20-
stdio_pipes: if (Environment.isWindows) std.ArrayListUnmanaged(StdioResult) else std.ArrayListUnmanaged(bun.FD) = .{},
20+
stdio_pipes: if (Environment.isWindows) std.ArrayListUnmanaged(StdioResult) else std.ArrayListUnmanaged(bun.spawn.PosixSpawnResult.ExtraPipe) = .{},
2121
pid_rusage: ?Rusage = null,
2222

2323
/// Terminal attached to this subprocess (if spawned with terminal option)
@@ -485,10 +485,9 @@ pub fn getStdio(this: *Subprocess, global: *JSGlobalObject) bun.JSError!JSValue
485485
} else {
486486
try array.push(global, .null);
487487
}
488-
} else if (item.isValid()) {
489-
try array.push(global, JSValue.jsNumber(item.cast()));
490-
} else {
491-
try array.push(global, .null);
488+
} else switch (item) {
489+
.owned_fd, .unowned_fd => |fd| try array.push(global, JSValue.jsNumber(fd.cast())),
490+
.unavailable => try array.push(global, .null),
492491
}
493492
}
494493
return array;
@@ -749,8 +748,9 @@ pub fn finalizeStreams(this: *Subprocess) void {
749748
if (item == .buffer) {
750749
item.buffer.close(onPipeClose);
751750
}
752-
} else if (item.isValid()) {
753-
item.close();
751+
} else switch (item) {
752+
.owned_fd => |fd| fd.close(),
753+
.unowned_fd, .unavailable => {},
754754
}
755755
}
756756
this.stdio_pipes.clearAndFree(bun.default_allocator);

‎src/cli/test/parallel/Worker.zig‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ pub fn start(this: *Worker) !void {
9090
if (spawned.stdout) |fd| try this.out.reader.start(fd, true).unwrap();
9191
if (spawned.stderr) |fd| try this.err.reader.start(fd, true).unwrap();
9292
if (spawned.extra_pipes.items.len > 0) {
93-
if (!this.ipc.adopt(coord.vm, spawned.extra_pipes.items[0])) return error.ChannelAdoptFailed;
93+
if (!this.ipc.adopt(coord.vm, spawned.extra_pipes.items[0].fd())) return error.ChannelAdoptFailed;
9494
} else {
9595
this.ipc.done = true;
9696
}

‎src/install/PackageManager/security_scanner.zig‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,7 @@ pub const SecurityScanSubprocess = struct {
795795
this.ipc_reader.flags.nonblocking = true;
796796
this.ipc_reader.flags.socket = false;
797797

798-
try this.finishSpawn(&spawned, ipc_output_fds[0], spawned.extra_pipes.items[1]);
798+
try this.finishSpawn(&spawned, ipc_output_fds[0], spawned.extra_pipes.items[1].fd());
799799
}
800800

801801
/// Windows fd 4: .buffer stdio for extra_fds sets UV_OVERLAPPED_PIPE on the

‎test/js/bun/spawn/spawn.test.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,9 @@ describe("close handling", () => {
826826
stdio: ["ignore", "ignore", "ignore", fd],
827827
}),
828828
);
829-
expect(procs[0].stdio[3]).toBe(null);
829+
// The caller-supplied fd should be exposed on stdio[N] (not null) while
830+
// still not being closed by the subprocess.
831+
expect(procs[0].stdio).toEqual([null, null, null, fd]);
830832
await Promise.all(procs.map(p => p.exited));
831833
})();
832834

@@ -848,6 +850,23 @@ describe("close handling", () => {
848850
} catch {}
849851
}
850852
});
853+
854+
it.skipIf(isWindows)("stdio[N] for non-fd extra slots is null", async () => {
855+
const fd = openSync(import.meta.path, "r");
856+
try {
857+
await using proc = spawn({
858+
cmd: [bunExe(), "-e", ""],
859+
env: bunEnv,
860+
stdio: ["ignore", "ignore", "ignore", "ignore", fd],
861+
});
862+
expect(proc.stdio).toEqual([null, null, null, null, fd]);
863+
await proc.exited;
864+
} finally {
865+
try {
866+
closeSync(fd);
867+
} catch {}
868+
}
869+
});
851870
});
852871

853872
it("dispose keyword works", async () => {

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /