Skip to content

spawn: don't forward SIGPWR — it's JSC's GC suspend/resume signal on Linux#31145

Closed
robobun wants to merge 1 commit into
mainfrom
farm/068a55c8/fix-sigpwr-forwarding
Closed

spawn: don't forward SIGPWR — it's JSC's GC suspend/resume signal on Linux#31145
robobun wants to merge 1 commit into
mainfrom
farm/068a55c8/fix-sigpwr-forwarding

Conversation

@robobun
Copy link
Copy Markdown
Collaborator

@robobun robobun commented May 20, 2026

Fuzzilli found a flaky crash (fingerprint 8f15ef1fddf992a0) where Bun terminates with TERMSIG: 30 (SIGPWR) on Linux.

Root cause

Bun__registerSignalsForForwarding() installs a forwarding signal handler (with SA_RESETHAND) for a set of signals while a synchronous child process runs, so e.g. a SIGINT sent to bun run reaches the script. On Linux that set was copied from npm and included SIGPWR.

Bun's JSC build uses SIGPWR as the thread suspend/resume signal for GC conservative stack scanning (see wtf/posix/ThreadingPOSIX.cpp, g_wtfConfig.sigThreadSuspendResume — Bun switched from the upstream default SIGUSR1 so npm packages can use SIGUSR1).

Bun.openInEditor launches the editor by calling sync::spawn on a detached thread, which hits Bun__registerSignalsForForwarding(). If the GC runs on the JS thread while that forwarding window is open:

  1. Thread::suspend() sends SIGPWR via pthread_kill to the JS thread.
  2. The forwarding handler runs instead of signalHandlerSuspendResume, so the suspend/resume semaphore is never posted → the GC spins or deadlocks.
  3. SA_RESETHAND resets SIGPWR to SIG_DFL.
  4. The next SIGPWR (either the GC retry in the suspend loop or the resume) terminates the process with signal 30.

Repro

Before this change, with a code stub on PATH:

for (let i = 0; i < 200; i++) {
  try { Bun.openInEditor("/tmp/whatever", 1); } catch {}
  Bun.gc(true);
}

hangs or dies with Power failure 15/15 runs under the debug build; 0/15 after.

Fix

Drop SIGPWR from FOR_EACH_LINUX_ONLY_SIGNAL. There is no reason to forward a power-failure signal to a child script, and Bun must never replace this handler while JSC is live.

Adds a Linux-only regression test that puts a stub editor on PATH, interleaves Bun.openInEditor with Bun.gc(true), and asserts the process exits cleanly.

…Linux

Bun__registerSignalsForForwarding() installs a forwarding handler (with
SA_RESETHAND) for a set of signals while a synchronous child process
runs. On Linux that set included SIGPWR, but Bun's JSC build uses SIGPWR
(wtf/posix/ThreadingPOSIX.cpp, g_wtfConfig.sigThreadSuspendResume) to
suspend and resume threads during conservative stack scanning.

Bun.openInEditor spawns the editor via the sync-spawn path on a
background thread. If the GC ran concurrently, the pthread_kill(SIGPWR)
sent to the JS thread was delivered to the forwarding handler instead of
signalHandlerSuspendResume — the semaphore was never posted (deadlock),
and SA_RESETHAND reverted SIGPWR to SIG_DFL so the next suspend killed
the process with signal 30.

Drop SIGPWR from the Linux forwarding set and add a regression test.
@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented May 20, 2026

Updated 11:31 AM PT - May 20th, 2026

@robobun, your commit 62b97dd has 9 failures in Build #56448 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 31145

That installs a local version of the PR into your bun-31145 executable, so you can run:

bun-31145 --bun

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 248b02a5-734d-4548-9cf2-79a48b202825

📥 Commits

Reviewing files that changed from the base of the PR and between a43a01b and 62b97dd.

📒 Files selected for processing (2)
  • src/jsc/bindings/c-bindings.cpp
  • test/js/bun/util/openInEditor.test.ts

Walkthrough

This PR fixes signal forwarding in Bun to prevent interference with JSC's garbage collector. On Linux, SIGPWR is now excluded from spawn signal forwarding because JSC reserves it for thread suspend/resume during GC. A regression test validates that Bun.openInEditor works correctly during concurrent garbage collection.

Changes

SIGPWR Signal Forwarding Fix

Layer / File(s) Summary
Signal forwarding behavior change
src/jsc/bindings/c-bindings.cpp
The FOR_EACH_LINUX_ONLY_SIGNAL macro excludes SIGPWR from signal forwarding and documents that JSC's GC suspend/resume protocol requires this signal to be reserved.
GC signal regression test
test/js/bun/util/openInEditor.test.ts
A Linux-only test for Bun.openInEditor that spawns a process while forcing concurrent garbage collection, verifying normal exit and no signal interference.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: removing SIGPWR from signal forwarding on Linux and explains the critical reason (JSC GC suspend/resume signal).
Description check ✅ Passed The description comprehensively covers what the PR does, the root cause analysis, repro steps, and verification approach, exceeding the template requirements with excellent detail.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. spawnSync: make signal-forwarding register/unregister safe for concurrent callers #30956 - Also removes SIGPWR from signal forwarding in spawnSync on Linux to prevent interference with JSC's GC suspend/resume mechanism

🤖 Generated with Claude Code

@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented May 20, 2026

Duplicate of #30956, which already removes SIGPWR from the forwarding set and additionally fixes the SIGIOT/SIGABRT and SIGPOLL/SIGIO alias overwrites plus adds a depth-counted lock around register/unregister for concurrent callers. Closing this in favor of that PR.

Fuzzer fingerprint 8f15ef1fddf992a0 is the same root cause as 11551e051c08db2c.

@robobun robobun closed this May 20, 2026
@robobun robobun deleted the farm/068a55c8/fix-sigpwr-forwarding branch May 20, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant