Skip to content
Open
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
18 changes: 9 additions & 9 deletions src/js/node/child_process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1691,9 +1691,6 @@ function isInternalIpcMessage(message) {
}

function streamFdOf(item): number | undefined {
const itemFd = ObjectHasOwn(item, "fd") ? item.fd : undefined;
if (typeof itemFd === "number") return itemFd;

const handle = item._handle;
const handleFd = handle ? handle.fd : undefined;
if (typeof handleFd === "number") return handleFd;
Expand All @@ -1709,7 +1706,7 @@ function streamFdOf(item): number | undefined {
return undefined;
}

function nodeToBun(item: string, index: number): string | number | null | NodeJS.TypedArray | ArrayBufferView {
function nodeToBun(item, index: number): string | number | null | NodeJS.TypedArray | ArrayBufferView {
// If not defined, use the default.
// For stdin/stdout/stderr, it's pipe. For others, it's ignore.
if (item == null) {
Expand All @@ -1720,6 +1717,11 @@ function nodeToBun(item: string, index: number): string | number | null | NodeJS
if (typeof item === "number") {
return item;
}
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/child_process.js#L1058
if (typeof item === "object") {
const fd = item.fd;
if (typeof fd === "number") return fd;
}
if (isNodeStreamReadable(item) || isNodeStreamWritable(item)) {
const fd = streamFdOf(item);
if (fd !== undefined) return fd;
Expand Down Expand Up @@ -1785,7 +1787,7 @@ function getBunStdioFromOptions(stdio) {
// overlapped -- same as pipe on Unix based systems
// inherit -- 'inherit': equivalent to ['inherit', 'inherit', 'inherit'] or [0, 1, 2]
// ignore -- > /dev/null, more or less same as null option for Bun.spawn stdio
// TODO: Stream -- use this stream
// Stream, handle or { fd } object -- its underlying FD is shared with the child
// number -- used as FD
// null, undefined: Use default value. Not same as ignore, which is Bun.spawn null.
// null/undefined: For stdio fds 0, 1, and 2 (in other words, stdin, stdout, and stderr) a pipe is created. For fd 3 and up, the default is 'ignore'
Expand All @@ -1800,7 +1802,7 @@ function getBunStdioFromOptions(stdio) {
// overlapped -> pipe
// ignore -> null
// inherit -> inherit (stdin/stdout/stderr)
// Stream -> throw err for now
// Stream / handle / { fd } -> fd (a stream without an fd throws)
const bunStdio = normalizedStdio.map(nodeToBun);
return bunStdio;
}
Expand All @@ -1820,9 +1822,7 @@ function normalizeStdio(stdio): string[] {
throw ERR_INVALID_OPT_VALUE("stdio", stdio);
}
} else if ($isJSArray(stdio)) {
// Validate if each is a valid stdio type
// TODO: Support wrapped types here

// Each entry is validated and translated by nodeToBun.
let processedStdio;
if (stdio.length === 0) processedStdio = ["pipe", "pipe", "pipe"];
else if (stdio.length === 1) processedStdio = [stdio[0], "pipe", "pipe"];
Expand Down
12 changes: 12 additions & 0 deletions test/expectations.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,15 @@
# only hold on the distributing model. Passes on macOS and Windows.
# (Verified still failing on every Linux lane, build 87834.)
[ LINUX ] test/js/node/test/sequential/test-net-listen-shared-ports.js [ FAIL ] # SO_REUSEPORT shared-listener semantics on Linux

# Both files pass `server._handle` to a detached child, which then serves the
# inherited socket with http.createServer().listen({ fd: 3 }). The spawn half
# works; http.Server#listen ignores `fd` (it listens on a fresh port), so the
# test's request to the inherited port is never answered: the file hangs until
# the runner kills it and leaves the detached child running. Before #39220 the
# spawn threw inside the intermediate process and both files exited 0 without
# testing anything. Remove these entries when http listen({ fd }) lands
# (#36491 / #34659); the Bun-side coverage of the spawn half is in
# test/js/node/child_process/child-process-stdio.test.js.
test/js/node/test/parallel/test-listen-fd-detached.js [ TIMEOUT ] # child's http listen({ fd: 3 }) ignores the fd
test/js/node/test/parallel/test-listen-fd-detached-inherit.js [ TIMEOUT ] # child's http listen({ fd: 3 }) ignores the fd
179 changes: 177 additions & 2 deletions test/js/node/child_process/child-process-stdio.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { describe, expect, it } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { execSync, spawn } from "node:child_process";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import { execSync, spawn, spawnSync } from "node:child_process";
import { once } from "node:events";
import { closeSync, openSync, readFileSync } from "node:fs";
import { open } from "node:fs/promises";
import { connect, createServer } from "node:net";
import { join } from "node:path";

const CHILD_PROCESS_FILE = import.meta.dir + "/spawned-child.js";
const OUT_FILE = import.meta.dir + "/stdio-test-out.txt";
Expand Down Expand Up @@ -166,3 +170,174 @@ describe("child.stdin", () => {
});
});
});

// Node shares the descriptor of any stdio entry that has a numeric `fd`
// property (lib/internal/child_process.js, getValidStdio): handle wraps,
// FileHandles, fs/tty streams, plain { fd } objects. Its own test-listen-fd-*
// tests rely on that to hand `server._handle` to a child as fd 3; in Bun that
// handle is the Bun.listen() Listener, which exposes `fd`. Socket descriptors
// cannot be inherited as stdio on Windows (same as Node), so the cases that
// actually inherit a socket are POSIX only.
describe("stdio entries carrying a file descriptor", () => {
async function listeningServer(onConnection) {
const server = createServer(onConnection);
await new Promise((resolve, reject) => {
server.once("error", reject);
server.listen(0, "127.0.0.1", resolve);
});
return { server, port: server.address().port };
}

it.skipIf(isWindows)("a listening server's handle is inherited by the child as that fd", async () => {
const { server, port } = await listeningServer(conn => conn.end("hello from parent"));
let child;
try {
child = spawn(
bunExe(),
[
"-e",
`require("net").createServer(c => c.end("hello from child")).listen({ fd: 3 }, () => console.log("listening"));`,
],
{ env: bunEnv, stdio: ["ignore", "pipe", "inherit", server._handle] },
);
// Like Node, a shared descriptor gets no stream in child.stdio.
expect(child.stdio[3]).toBeNull();

// Close the parent's copy so the child's inherited descriptor is the only
// thing keeping the port open.
const closed = once(server, "close");
server.close();
await closed;

let out = "";
await new Promise((resolve, reject) => {
child.stdout.setEncoding("utf8");
child.stdout.on("data", chunk => {
out += chunk;
if (out.includes("listening")) resolve();
});
child.once("error", reject);
child.once("exit", (code, signal) =>
reject(new Error(`child exited (${code ?? signal}) before listening: ${out}`)),
);
});

const reply = await new Promise((resolve, reject) => {
const socket = connect({ port, host: "127.0.0.1" });
let data = "";
socket.setEncoding("utf8");
socket.on("data", chunk => (data += chunk));
socket.once("end", () => resolve(data));
socket.once("error", reject);
});
expect(reply).toBe("hello from child");
} finally {
child?.kill();
server.close();
}
});

it.skipIf(isWindows)("spawnSync shares listening and connected socket handles with the child", async () => {
// The server leaves the connection open so the client's handle is still
// live while the child is spawned.
const { server, port } = await listeningServer(() => {});
const client = connect({ port, host: "127.0.0.1" });
try {
await once(client, "connect");
const { stdout, status } = spawnSync(
bunExe(),
[
"-e",
`const { fstatSync } = require("fs"); console.log(JSON.stringify([3, 4].map(fd => fstatSync(fd).isSocket())));`,
],
{
env: bunEnv,
encoding: "utf8",
stdio: ["ignore", "pipe", "inherit", server._handle, client._handle],
},
);
expect(stdout).toBe("[true,true]\n");
expect(status).toBe(0);
} finally {
client.destroy();
server.close();
}
});

it.each([
[
"a plain { fd } object",
file => {
const fd = openSync(file, "w");
return { entry: { fd }, close: () => closeSync(fd) };
},
],
[
"a FileHandle",
async file => {
const handle = await open(file, "w");
return { entry: handle, close: () => handle.close() };
},
],
])("%s as stdout makes the child write to that descriptor", async (_label, prepare) => {
using dir = tempDir("stdio-fd-object", {});
const file = join(String(dir), "stdout.txt");
const { entry, close } = await prepare(file);
let status;
try {
({ status } = spawnSync(bunExe(), ["-e", `console.log("written through the shared fd")`], {
env: bunEnv,
stdio: ["ignore", entry, "inherit"],
}));
} finally {
await close();
}
expect(readFileSync(file, "utf8")).toBe("written through the shared fd\n");
expect(status).toBe(0);
});

it.each([
["an object without an fd", {}],
["an object whose fd is not a number", { fd: "3" }],
])("%s is still rejected before anything is spawned", (_label, entry) => {
const options = { env: bunEnv, stdio: ["ignore", "ignore", "ignore", entry] };
expect(() => spawn(bunExe(), ["-e", "0"], options)).toThrow(/stdio/);
expect(() => spawnSync(bunExe(), ["-e", "0"], options)).toThrow(/stdio/);
});

it("a handle whose descriptor is already closed is refused, like passing its fd directly", async () => {
const { server } = await listeningServer(() => {});
const handle = server._handle;
const closed = once(server, "close");
server.close();
await closed;
expect(handle.fd).toBe(-1);

// The handle is only sugar for its `.fd`, so this takes the path a bare -1
// takes: Bun.spawn refuses the descriptor (spawn() throws, spawnSync()
// returns the error). Node's libuv layer instead spawns the child with
// the slot left closed for the negative errno a closed wrap reports
// (EINVAL only for exactly -1); silently dropping the socket is not worth
// reproducing.
const optionsFor = entry => ({ env: bunEnv, stdio: ["ignore", "ignore", "ignore", entry] });
const spawnFailure = entry => {
try {
spawn(bunExe(), ["-e", "0"], optionsFor(entry));
} catch (error) {
return { code: error.code, message: error.message };
}
};
const spawnSyncFailure = entry => {
const { error } = spawnSync(bunExe(), ["-e", "0"], optionsFor(entry));
return error && { code: error.code, message: error.message };
};

const viaHandle = spawnFailure(handle);
expect(viaHandle).toBeDefined();
expect(viaHandle).toEqual(spawnFailure(handle.fd));

const viaHandleSync = spawnSyncFailure(handle);
expect(viaHandleSync).toBeDefined();
expect(viaHandleSync).toEqual(spawnSyncFailure(handle.fd));
});
});
Loading