child_process: emit 'disconnect' before 'exit' for children with an IPC channel

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() {

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();
 

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