Fix regression from #13414 (#14092)

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

@@ -211,6 +211,7 @@ pub const Subprocess = struct {
         killed: bool = false,
         has_stdin_destructor_called: bool = false,
         finalized: bool = false,
+        deref_on_stdin_destroyed: bool = false,
     };
 
     pub const SignalCode = bun.SignalCode;
@@ -703,9 +704,13 @@ pub const Subprocess = struct {
     }
 
     pub fn onStdinDestroyed(this: *Subprocess) void {
+        const must_deref = this.flags.deref_on_stdin_destroyed;
+        this.flags.deref_on_stdin_destroyed = false;
+        defer if (must_deref) this.deref();
+
         this.flags.has_stdin_destructor_called = true;
         this.weak_file_sink_stdin_ptr = null;
-        defer this.deref();
+
         if (!this.flags.finalized) {
             // otherwise update the pending activity flag
             this.updateHasPendingActivity();
@@ -1240,6 +1245,7 @@ pub const Subprocess = struct {
                             pipe.writer.setParent(pipe);
                             subprocess.weak_file_sink_stdin_ptr = pipe;
                             subprocess.ref();
+                            subprocess.flags.deref_on_stdin_destroyed = true;
                             subprocess.flags.has_stdin_destructor_called = false;
 
                             return Writable{
@@ -1300,6 +1306,7 @@ pub const Subprocess = struct {
                     subprocess.weak_file_sink_stdin_ptr = pipe;
                     subprocess.ref();
                     subprocess.flags.has_stdin_destructor_called = false;
+                    subprocess.flags.deref_on_stdin_destroyed = true;
 
                     pipe.writer.handle.poll.flags.insert(.socket);
 
@@ -1345,12 +1352,23 @@ pub const Subprocess = struct {
                 .pipe => |pipe| {
                     this.* = .{ .ignore = {} };
                     if (subprocess.process.hasExited() and !subprocess.flags.has_stdin_destructor_called) {
+                        // onAttachedProcessExit() can call deref on the
+                        // subprocess. Since we never called ref(), it would be
+                        // unbalanced to do so, leading to a use-after-free.
+                        // So, let's not do that.
+                        // https://github.com/oven-sh/bun/pull/14092
+                        bun.debugAssert(!subprocess.flags.deref_on_stdin_destroyed);
+                        const debug_ref_count: if (Environment.isDebug) u32 else u0 = if (Environment.isDebug) subprocess.ref_count else 0;
                         pipe.onAttachedProcessExit();
+                        if (comptime Environment.isDebug) {
+                            bun.debugAssert(subprocess.ref_count == debug_ref_count);
+                        }
                         return pipe.toJS(globalThis);
                     } else {
                         subprocess.flags.has_stdin_destructor_called = false;
                         subprocess.weak_file_sink_stdin_ptr = pipe;
                         subprocess.ref();
+                        subprocess.flags.deref_on_stdin_destroyed = true;
                         if (@intFromPtr(pipe.signal.ptr) == @intFromPtr(subprocess)) {
                             pipe.signal.clear();
                         }
@@ -1447,6 +1465,7 @@ pub const Subprocess = struct {
         if (stdin) |pipe| {
             this.weak_file_sink_stdin_ptr = null;
             this.flags.has_stdin_destructor_called = true;
+            // It is okay if it does call deref() here, as in that case it was truly ref'd.
             pipe.onAttachedProcessExit();
         }
 
@@ -2157,6 +2176,9 @@ pub const Subprocess = struct {
                 default_max_buffer_size,
                 is_sync,
             ),
+            // 1. JavaScript.
+            // 2. Process.
+            .ref_count = 2,
             .stdio_pipes = spawned.extra_pipes.moveToUnmanaged(),
             .on_exit_callback = if (on_exit_callback != .zero) JSC.Strong.create(on_exit_callback, globalThis) else .{},
             .on_disconnect_callback = if (on_disconnect_callback != .zero) JSC.Strong.create(on_disconnect_callback, globalThis) else .{},
@@ -2177,7 +2199,7 @@ pub const Subprocess = struct {
                 .is_sync = is_sync,
             },
         };
-        subprocess.ref(); // + one ref for the process
+
         subprocess.process.setExitHandler(subprocess);
 
         if (subprocess.ipc_data) |*ipc_data| {

test/js/bun/spawn/bad-fixture.js

@@ -0,0 +1,3 @@
+process.stdin.read();
+
+throw new Error("test");

test/js/bun/spawn/spawn-stdin-destroy.test.ts

@@ -0,0 +1,38 @@
+import { bunEnv, bunExe } from "harness";
+import path from "path";
+
+test("stdin destroy after exit crash", async () => {
+  let before;
+  await (async () => {
+    const child = Bun.spawn({
+      cmd: [bunExe(), path.join(import.meta.dir, "bad-fixture.js")],
+      env: bunEnv,
+      stdout: "pipe",
+      stdin: "pipe",
+    });
+
+    await Bun.sleep(80);
+    await child.stdin.write("dylan\n");
+    await child.stdin.write("999\n");
+    await child.stdin.flush();
+    await child.stdin.end();
+
+    async function read() {
+      var out = "";
+      for await (const chunk of child.stdout) {
+        out += new TextDecoder().decode(chunk);
+      }
+      return out;
+    }
+
+    // This bug manifested as child.exited rejecting with an error code of "TODO"
+    const [out, exited] = await Promise.all([read(), child.exited]);
+
+    expect(out).toBe("");
+    expect(exited).toBe(1);
+
+    Bun.gc(true);
+    await Bun.sleep(50);
+  })();
+  Bun.gc(true);
+});