Spawn the editor from Bun.openInEditor without spawnSync's signal forwarding

src/runtime/cli/open.rs

@@ -7,8 +7,6 @@ use bun_paths::{self, MAX_PATH_BYTES, PathBuffer};
 use bun_resolver::fs as Fs;
 use bun_which::which;
 
-use crate::api::bun::process::sync;
-
 // ──────────────────────────────────────────────────────────────────────────
 
 #[cfg(target_os = "macos")]
@@ -289,6 +287,9 @@ impl Editor {
         }
 
         spawned.argc = i;
+        // Zig stored `std.process.Child.init(args_buf[0..i], default_allocator)` here and
+        // spawned a detached std.Thread to run it; the detached thread below does the
+        // spawn+wait itself via `spawn_sync_inherit` (see `auto_close`).
         let spawned_ptr = bun_core::heap::into_raw(spawned);
         // bun_threading has no detached-spawn helper; std::thread::spawn is used
         // and the JoinHandle is dropped, detaching the thread.
@@ -415,33 +416,12 @@ fn auto_close(spawned: *mut SpawnedEditorContext) {
         argv[j] = unsafe { bun_core::ffi::slice(p, l) };
     }
 
-    // FIXME(windows-leak): the sync::spawn path
-    // requires a `WindowsOptions.loop_`; `MiniEventLoop::init_global` heap-allocates a
-    // MiniEventLoop + uv_loop_t into a thread-local that is NEVER torn down. Because this
-    // runs on a fresh detached std::thread per `Editor::open()` call, every editor-open on
-    // Windows leaks one MiniEventLoop + uv_loop_t (+ DotEnv Loader/Map if env was null).
-    // Proper fix needs either (a) a MiniEventLoop teardown helper (none exists today), or
-    // (b) plumbing the caller's existing EventLoopHandle through SpawnedEditorContext
-    // (signature change to Editor::open + callers). Both are out-of-scope for this file.
-    let owned_argv: Vec<Box<[u8]>> = argv[0..spawned.argc]
-        .iter()
-        .map(|s| s.to_vec().into_boxed_slice())
-        .collect();
-    let _ = sync::spawn(&sync::Options {
-        argv: owned_argv,
-        envp: None,
-        stderr: sync::SyncStdio::Inherit,
-        stdout: sync::SyncStdio::Inherit,
-        stdin: sync::SyncStdio::Inherit,
-        #[cfg(windows)]
-        windows: crate::api::bun::process::WindowsOptions {
-            loop_: bun_jsc::EventLoopHandle::init_mini(bun_event_loop::MiniEventLoop::init_global(
-                None, None,
-            )),
-            ..Default::default()
-        },
-        ..Default::default()
-    });
+    // Zig called `child_process.spawn()` then `.wait()` via std.process.Child. Use the
+    // minimal spawn+wait helper here, NOT `sync::spawn` (bun.spawnSync): spawnSync's
+    // signal-forwarding setup mutates process-wide state (`Bun__registerSignalsForForwarding`,
+    // `Bun__currentSyncPID`) that is only safe on the main thread, and this runs on a
+    // detached thread per `Editor::open()` call.
+    let _ = bun_core::util::spawn_sync_inherit(&argv[0..spawned.argc]);
 }
 
 // ──────────────────────────────────────────────────────────────────────────

test/js/bun/util/open-in-editor-gc.test.ts

@@ -3,8 +3,77 @@ import { bunEnv, bunExe, isLinux, tempDir } from "harness";
 import { existsSync, symlinkSync } from "node:fs";
 import { join } from "node:path";
 
+// The detached editor thread spawned by Bun.openInEditor must not mutate process-wide
+// signal state. It used to run bun.spawnSync's signal-forwarding setup, which is only
+// safe on the main thread: concurrent openInEditor calls raced on the shared
+// previous_actions[] array and flipped unrelated signal dispositions process-wide
+// (installing a one-shot forwarding handler, then resetting them to SIG_DFL), which can
+// get the process killed by a stray signal while the GC suspend signal (SIGPWR) is in
+// flight. Sample the process's caught-signal mask (SigCgt in /proc/self/status) while
+// hammering openInEditor and assert it never changes.
+test.skipIf(!isLinux)("concurrent Bun.openInEditor calls do not touch process signal handlers", async () => {
+  const sleep = ["/usr/bin/sleep", "/bin/sleep"].find(p => existsSync(p));
+  expect(sleep).toBeDefined();
+
+  using dir = tempDir("open-in-editor-signals", {
+    "storm.js": `
+      const { readFileSync } = require("node:fs");
+      const sleepBin = process.argv[2];
+      function caughtMask() {
+        const status = readFileSync("/proc/self/status", "utf8");
+        const line = status.split("\\n").find(l => l.startsWith("SigCgt:"));
+        return BigInt("0x" + line.slice("SigCgt:".length).trim());
+      }
+      // Warm-up: let any lazy one-time handler installation happen before baselining.
+      try { Bun.openInEditor("0.05", { editor: sleepBin }); } catch {}
+      await Bun.sleep(150);
+      const baseline = caughtMask();
+      let changed = 0n;
+      // Each call spawns a detached editor thread that runs \`sleep 0.15\` and waits for
+      // it, so dozens of editor threads overlap.
+      for (let i = 0; i < 64; i++) {
+        try { Bun.openInEditor("0.15", { editor: sleepBin }); } catch {}
+        if ((i & 7) === 0) changed |= baseline ^ caughtMask();
+      }
+      // Keep sampling while the editor threads drain.
+      for (let i = 0; i < 150; i++) {
+        changed |= baseline ^ caughtMask();
+        await Bun.sleep(5);
+      }
+      // Force GC (which uses SIGPWR on Linux to suspend/resume threads) to prove the
+      // process still survives it.
+      Bun.gc(true);
+      const changedSignals = [];
+      for (let sig = 1; sig <= 64; sig++) {
+        if (changed & (1n << BigInt(sig - 1))) changedSignals.push(sig);
+      }
+      console.log("CHANGED:" + JSON.stringify(changedSignals));
+      console.log("ALIVE");
+    `,
+  });
+
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "storm.js", sleep!],
+    env: bunEnv,
+    cwd: String(dir),
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  expect(stdout).toContain("ALIVE");
+  const changedLine = stdout.match(/^CHANGED:(.*)$/m);
+  expect(changedLine).not.toBeNull();
+  const changed = JSON.parse(changedLine![1]);
+  expect(changed).toEqual([]);
+  expect(stderr).toBe("");
+  expect(proc.signalCode).toBeNull();
+  expect(exitCode).toBe(0);
+});
+
 // On Linux, JSC uses SIGPWR to suspend/resume threads for GC and the libpas
-// scavenger. Bun.openInEditor spawns a detached thread that goes through
+// scavenger. Bun.openInEditor spawns a detached thread that used to go through
 // bun.spawnSync, whose signal-forwarding setup must not touch SIGPWR or the
 // process is terminated the next time GC/scavenger fires.
 test.skipIf(!isLinux)("Bun.openInEditor does not break GC signal handling", async () => {