spawnSync: make signal-forwarding register/unregister safe for concurrent callers

src/jsc/bindings/NoOrphansTracker.cpp

@@ -26,8 +26,8 @@
 // birth records its uniqueid — is narrowed by the freeze-then-rescan loop in
 // `killTracked()` but cannot be fully closed from userspace.
 //
-// All state is process-global; spawnSync is single-threaded by design (see
-// Bun__currentSyncPID), so no locking.
+// All state is process-global; this is only reached from the thread that
+// armed the parent-death watchdog (pdeathsig::is_arming_thread), so no locking.
 
 #include "root.h"
 
@@ -65,9 +65,8 @@ static_assert(sizeof(ProcUniqIdentifierInfo) == 56, "xnu ABI");
 class NoOrphansTracker {
 public:
     // Function-local static: lazy first-use construction, no global ctor,
-    // thread-safe per C++11 [stmt.dcl]. spawnSync is single-threaded anyway
-    // (see Bun__currentSyncPID), but this keeps the binary's static-init
-    // section clean.
+    // thread-safe per C++11 [stmt.dcl]. Only the arming thread gets here
+    // anyway, but this keeps the binary's static-init section clean.
     static NoOrphansTracker& get()
     {
         static NoOrphansTracker instance;

src/jsc/bindings/c-bindings.cpp

@@ -937,11 +937,14 @@ extern "C" int ffi_fileno(FILE* file)
 #if OS(LINUX) || OS(DARWIN) || OS(FREEBSD)
 #include <signal.h>
 #include <pthread.h>
+#include <wtf/Lock.h>
 
-// Note: We only ever use bun.spawnSync on the main thread.
+// Bun.openInEditor runs spawnSync on detached threads, so register/unregister can overlap.
 extern "C" int64_t Bun__currentSyncPID = 0;
 static int Bun__pendingSignalToSend = 0;
 static struct sigaction previous_actions[NSIG];
+static WTF::Lock signalForwardingLock;
+static int signalForwardingDepth = 0;
 
 // npm's signal list minus SIGIOT/SIGPOLL (aliases of SIGABRT/SIGIO; listing both would overwrite previous_actions[N]).
 // https://github.com/npm/cli/blob/fefd509992a05c2dfddbe7bc46931c42f1da69d7/workspaces/arborist/lib/signals.js#L26-L57
@@ -1004,6 +1007,10 @@ extern "C" void Bun__sendPendingSignalIfNecessary()
 
 extern "C" void Bun__registerSignalsForForwarding()
 {
+    WTF::Locker<WTF::Lock> locker(signalForwardingLock);
+    if (signalForwardingDepth++ != 0)
+        return;
+
     Bun__pendingSignalToSend = 0;
     struct sigaction sa;
     memset(&sa, 0, sizeof(sa));
@@ -1032,6 +1039,10 @@ extern "C" void Bun__unregisterSignalsForForwarding()
 {
     Bun__currentSyncPID = 0;
 
+    WTF::Locker<WTF::Lock> locker(signalForwardingLock);
+    if (--signalForwardingDepth != 0)
+        return;
+
 #define UNREGISTER_SIGNAL(SIG)                                \
     if (sigaction(SIG, &previous_actions[SIG], NULL) == -1) { \
     }

src/spawn/Cargo.toml

@@ -20,7 +20,6 @@ scopeguard.workspace = true
 bun_analytics.workspace = true
 bun_dispatch.workspace = true
 bun_core.workspace = true
-bun_crash_handler.workspace = true
 bun_event_loop.workspace = true
 bun_io.workspace = true
 bun_output.workspace = true

src/spawn/lib.rs

@@ -9,8 +9,8 @@
 //! `bun_runtime` re-exports them. The only non-leaf dependencies are
 //! `bun_io` (`FilePoll`/`KeepAlive`/`EventLoopCtx`), `bun_ptr`
 //! (`ThreadSafeRefCount`), `bun_io` (`BufferedWriter`), `bun_event_loop`,
-//! `bun_threading`, and `bun_crash_handler` — none of which depend back on
-//! this crate, so no cycle.
+//! and `bun_threading` — none of which depend back on this crate, so no
+//! cycle.
 
 use core::ffi::c_char;
 

src/spawn/process.rs

@@ -2853,8 +2853,7 @@ mod spawn_process_body {
         };
 
         /// RAII guard around `Bun__registerSignalsForForwarding`: registers on
-        /// construction, unregisters and restores the crash-handler signal
-        /// disposition on drop.
+        /// construction, unregisters on drop.
         #[cfg(unix)]
         struct SignalForwarding;
         #[cfg(unix)]
@@ -2869,7 +2868,6 @@ mod spawn_process_body {
         impl Drop for SignalForwarding {
             fn drop(&mut self) {
                 Bun__unregisterSignalsForForwarding();
-                bun_crash_handler::reset_on_posix();
             }
         }
 

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

@@ -152,3 +152,54 @@
 
   await Promise.all(runs);
 });
+
+// Each detached editor thread goes through bun.spawnSync's signal-forwarding
+// register/unregister, which swaps every forwarded signal's disposition in a
+// process-global table. With many threads overlapping, an unsynchronized
+// table ends up restoring SIG_DFL (or the forwarding handler itself) instead
+// of the handler that was installed before the burst, so a JS signal
+// listener registered beforehand silently stops working or kills the process.
+test.skipIf(!isLinux)("Bun.openInEditor bursts do not drop previously installed signal handlers", async () => {
+  const sleep = ["/usr/bin/sleep", "/bin/sleep"].find(p => existsSync(p));
+  expect(sleep).toBeDefined();
+
+  using dir = tempDir("open-in-editor-signal-table", {
+    "run.js": `
+      let fired = false;
+      process.on("SIGUSR2", () => { fired = true; });
+
+      let spawned = 0;
+      for (let i = 0; i < 64; i++) {
+        try { Bun.openInEditor("0.1", { editor: ${JSON.stringify(sleep)} }); spawned++; } catch {}
+      }
+      if (spawned === 0) { console.log("no editor threads spawned"); process.exit(2); }
+
+      // The editor children each run for 100ms, so by now the burst has
+      // started and the forwarding handler owns SIGUSR2; the original handler
+      // only comes back once the last thread unregisters. Keep delivering
+      // until it does (or the table was corrupted and it never does).
+      await Bun.sleep(50);
+      const deadline = Date.now() + 5000;
+      while (!fired && Date.now() < deadline) {
+        process.kill(process.pid, "SIGUSR2");
+        await Bun.sleep(20);
+      }
+      console.log(fired ? "ok" : "handler lost");
+    `,
+  });
+
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "run.js"],
+    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(stderr).toBe("");
+  expect(stdout.trim()).toBe("ok");
+  expect(proc.signalCode).toBeNull();
+  expect(exitCode).toBe(0);
+});