spawn: do not forward SIGPWR on Linux#31121
Closed
robobun wants to merge 1 commit into
Closed
Conversation
JavaScriptCore uses SIGPWR to suspend and resume threads during garbage collection on Linux. Bun__registerSignalsForForwarding() (used by bun.spawnSync) was installing a forwarding handler with SA_RESETHAND over it. When a concurrent GC sent SIGPWR to a JS thread, the forwarding handler ran instead of JSC's suspend handler, and SA_RESETHAND reset the disposition to SIG_DFL, so the next SIGPWR terminated the process. Bun.openInEditor() triggers this reliably because it runs bun.spawnSync on a detached background thread while the JS thread continues executing (and potentially collecting garbage). Remove SIGPWR from the forwarded signal set on Linux. Fuzzer fingerprint: fe0542d56ab690b0
Collaborator
Author
|
Updated 11:53 PM PT - May 19th, 2026
❌ @robobun, your commit 1f45203 has 9 failures in
🧪 To try this PR locally: bunx bun-pr 31121That installs a local version of the PR into your bun-31121 --bun |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughJavaScriptCore reserves SIGPWR for internal garbage-collection suspend/resume. This PR removes SIGPWR from the Linux signal-forwarding list in ChangesSIGPWR Signal Handling Fix
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
Collaborator
Author
|
Duplicate of #30956, which has a more comprehensive fix (also handles the concurrent register/unregister race with a depth counter + mutex, and removes the SIGIOT/SIGPOLL aliases). Fuzzer fingerprint |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+16
to
+18
| const { exitCode, signalCode } = Bun.spawnSync({ | ||
| cmd: [bunExe(), "-e", script], | ||
| env: { ...bunEnv, EDITOR: undefined, VISUAL: undefined }, |
Contributor
There was a problem hiding this comment.
🟡 Unsetting EDITOR/VISUAL is not enough to make this test hermetic: bunEnv spreads process.env (including PATH), and when no EDITOR/VISUAL is set EditorContext::detect_editor falls through to Editor::by_fallback, which probes PATH for code/subl/nvim/vim/etc. On a Linux dev box with VS Code installed, running this test will spawn 50 detached code fooN processes (50 editor tabs); on CI it may leave orphaned xdg-open invocations. Add PATH: "/dev/null" to the env override (or pass { editor: "/nonexistent" } as the second arg to Bun.openInEditor) — the SIGPWR race is still exercised either way since the detached spawn thread still runs.
Extended reasoning...
What the bug is
The new regression test attempts to prevent Bun.openInEditor from launching a real editor by setting env: { ...bunEnv, EDITOR: undefined, VISUAL: undefined }. This is insufficient: when neither an explicit editor option nor EDITOR/VISUAL is present, Bun falls back to probing PATH for a list of common editors and will happily launch whichever one it finds — 50 times, on detached background threads.
Code path
bunEnvis defined as{ ...process.env, ... }(test/harness.ts:50-51) and never overridesPATH, so the spawned child inherits the hostPATH.- The inner script calls
Bun.openInEditor("foo" + k)with no second argument, soopen_in_editor(src/runtime/api/BunObject.rs:1084-1095) sees no explicit editor and callsedit.auto_detect_editor(env). detect_editor(src/runtime/cli/open.rs:619-716) findsself.nameempty andEDITOR/VISUALunset, so it falls through toEditor::by_fallbackat line 701.by_fallback(open.rs:221-246) iteratesDEFAULT_PREFERENCE_LIST = [Vscode, Sublime, Atom, Neovim, Webstorm, Intellij, Textmate, Vim](open.rs:392) and callsby_path_for_editor, which runswhich()againstenv.get(b"PATH").- If a match is found,
editor.open()spawns a detachedstd::threadthat callssync::spawn(the sameBun__registerSignalsForForwardingpath this PR fixes) with the resolved binary asargv[0].
Why the existing guard doesn't help
The EDITOR: undefined, VISUAL: undefined override only short-circuits the first two branches of detect_editor (the by_name lookups for $EDITOR and $VISUAL). The third branch — by_fallback — is gated solely on PATH, which the test leaves untouched. On Linux, bin_path()'s hardcoded fallback locations are macOS-only (open.rs:424-437), so PATH is the only thing that matters.
Step-by-step example
On a Linux developer machine with VS Code installed (/usr/bin/code in PATH):
bun test test/js/bun/util/open-in-editor-gc.test.tsspawns the child withPATH=/usr/bin:...,EDITORandVISUALunset.- For
k=0:detect_editor→by_fallback→which("code")resolves/usr/bin/code→Editor::Vscode,edit.path = "/usr/bin/code". editor.open()builds argv["/usr/bin/code", "foo0"](Vscode uses the binary directly, noxdg-openprefix — open.rs:286) and spawns it on a detached thread.- Repeat 50× → 50 VS Code tabs open on the developer's desktop pointing at non-existent
foo0..foo49. - The child process exits; the spawned editor processes are orphaned (not reaped, not killed).
For vim/nvim the argv is prefixed with OPENER = b"xdg-open" (open.rs:20, 276-284), which on headless CI typically fails fast, so the impact there is lower — but it still spawns 50 processes per test run.
Impact
This is a test-hermeticity issue, not a correctness bug. The test itself will not fail or hang: the JS thread never joins the detached spawn threads, and auto_close swallows spawn errors via let _ = sync::spawn(...). The SIGPWR regression is still correctly exercised regardless of whether an editor is found (the detached thread still calls Bun__registerSignalsForForwarding either way). However:
- On a developer machine with
code/subl/ideainPATH, every run of this test pops 50 editor windows. - On CI with
vim/nviminPATH(very common on Linux images), it spawns 50 short-livedxdg-openprocesses per run, adding noise and orphans.
Fix
Add PATH: "/dev/null" (or PATH: "") to the env override:
env: { ...bunEnv, EDITOR: undefined, VISUAL: undefined, PATH: "/dev/null" },Alternatively, pass an explicit non-existent editor so detect_editor never runs the fallback probe:
try { Bun.openInEditor("foo" + k, { editor: "/nonexistent" }); } catch {}Either change keeps the regression coverage intact (the detached-thread spawnSync + concurrent GC race still fires) while making the test hermetic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
On Linux, JavaScriptCore uses
SIGPWRto suspend and resume threads during garbage collection (seeThreadingPOSIX.cpp).Bun__registerSignalsForForwarding()(used bybun.spawnSyncto forward signals likeSIGINT/SIGTERMto child scripts) was installing a forwarding handler withSA_RESETHANDover the GC'sSIGPWRhandler. When a concurrent GC sentSIGPWRto a JS thread during the register/unregister window:SA_RESETHANDreset the disposition toSIG_DFL,SIGPWR(GC resume, or next suspend) terminated the process with "Power failure".Bun.openInEditor()triggers this reliably because it runsbun.spawnSyncon a detached background thread while the JS thread continues executing (and potentially runningBun.gc(true)), so the GC'spthread_kill(thread, SIGPWR)races the background thread'ssigaction()calls.How did you verify your code works?
Minimal repro (crashes 100% before, 0% after in debug builds):
Before:
After: exits 0.
Added a regression test at
test/js/bun/util/open-in-editor-gc.test.ts.Fuzzer fingerprint:
fe0542d56ab690b0