diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index ed70f054cd77..25743849c9c2 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -1102,6 +1102,9 @@ class ChildProcess extends EventEmitter { #handle; #closesNeeded = 1; #closesGot = 0; + #hasIpc = false; + #disconnectEmitted = false; + #pendingExit; signalCode = null; exitCode = null; @@ -1118,6 +1121,15 @@ class ChildProcess extends EventEmitter { } #handleOnExit(exitCode, signalCode, err) { + // 'disconnect' precedes 'exit' in Node; the native side always closes the channel on exit. + if (this.#hasIpc && !this.#disconnectEmitted) { + this.#pendingExit = [exitCode, signalCode, err]; + return; + } + this.#emitExit(exitCode, signalCode, err); + } + + #emitExit(exitCode, signalCode, err) { if (signalCode) { this.signalCode = signalCode; } else { @@ -1451,6 +1463,7 @@ class ChildProcess extends EventEmitter { }); if (has_ipc) { + this.#hasIpc = true; this.send = this.#send; this.disconnect = this.#disconnect; this.channel = new Control(); @@ -1549,7 +1562,19 @@ class ChildProcess extends EventEmitter { return; } $assert(!this.connected); - process.nextTick(() => this.emit("disconnect")); + // The flag flips in the same tick that emits, so a #handleOnExit tick queued + // ahead of this one still defers and is flushed by the tick below. + process.nextTick(() => { + this.#disconnectEmitted = true; + this.emit("disconnect"); + }); + process.nextTick(() => { + const pendingExit = this.#pendingExit; + if (pendingExit !== undefined) { + this.#pendingExit = undefined; + this.#emitExit(pendingExit[0], pendingExit[1], pendingExit[2]); + } + }); process.nextTick(() => this.#maybeClose()); } #disconnect() { diff --git a/test/js/node/child_process/child_process.test.ts b/test/js/node/child_process/child_process.test.ts index acac21c9b10b..5a69c79e6679 100644 --- a/test/js/node/child_process/child_process.test.ts +++ b/test/js/node/child_process/child_process.test.ts @@ -753,6 +753,43 @@ it("should call close and exit before process exits", async () => { expect(await proc.exited).toBe(0); }); +it("emits 'disconnect' before 'exit' when the child has an IPC channel", async () => { + using dir = tempDir("child-process-ipc-order", { + "parent.js": ` + const { fork } = require("node:child_process"); + const path = require("node:path"); + + const order = []; + const child = fork(path.join(__dirname, "child.js")); + + child.on("message", () => { + // The child calls process.exit() right after sending this. Block the + // loop so the channel EOF and the process exit are both observed in + // the same poll batch, which is the interleaving that raced. + Bun.sleepSync(100); + }); + child.on("disconnect", () => order.push("disconnect")); + child.on("exit", code => order.push("exit:" + code)); + // 'close' only fires once both of the above have been emitted. + child.on("close", () => console.log(order.join(","))); + `, + "child.js": ` + process.send("bye"); + process.exit(3); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "parent.js"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "disconnect,exit:3", stderr: "", exitCode: 0 }); +}); + it("it accepts stdio passthrough", async () => { const package_dir = tmpdirSync(); diff --git a/test/js/node/cluster.test.ts b/test/js/node/cluster.test.ts index 64d101f15e8f..93f28ad8d04a 100644 --- a/test/js/node/cluster.test.ts +++ b/test/js/node/cluster.test.ts @@ -557,6 +557,54 @@ if (cluster.isPrimary) { expect(exitCode).toBe(0); }); +test("worker 'disconnect' is emitted before 'exit'", async () => { + const dir = tempDirWithFiles("bun-test", { + "index.js": ` +const cluster = require("node:cluster"); + +if (cluster.isPrimary) { + const order = []; + let pending = 2; + const done = () => { + if (--pending === 0) { + console.log(order.join(",")); + process.exit(0); + } + }; + + const worker = cluster.fork(); + worker.on("message", () => { + // The worker calls process.exit() right after sending this. Block the loop + // so the IPC channel EOF and the worker's exit are both observed in the + // same poll batch, which is the interleaving that raced. + Bun.sleepSync(100); + }); + + cluster.on("disconnect", () => { + order.push("disconnect"); + done(); + }); + cluster.on("exit", (_worker, code) => { + order.push("exit:" + code); + done(); + }); +} else { + process.send("bye"); + process.exit(3); +} +`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), joinP(dir, "index.js")], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "disconnect,exit:3", stderr: "", exitCode: 0 }); +}); + test("disconnect() on a cluster.Worker built around a plain object does not abort", async () => { // `kHandle` is a private symbol that only `cluster.fork()` sets, so a // `cluster.Worker({ process })` built around a plain object (how Node's own