Skip to content

spawnSync: do not forward SIGPWR on Linux #30956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 5 commits into
base: main
Choose a base branch
from
+72 −4
Open

spawnSync: do not forward SIGPWR on Linux #30956

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4b9d469
spawnSync: do not forward SIGPWR on Linux
robobun May 18, 2026
ac02046
[autofix.ci] apply automated fixes
autofix-ci[bot] May 18, 2026
1b8f2ae
ci: retrigger
robobun May 18, 2026
2d5b42e
test: drop negative crash-marker assertion
robobun May 18, 2026
7e2c283
spawnSync: make signal-forwarding register/unregister reentrant
robobun May 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/jsc/bindings/c-bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,9 +933,11 @@ static struct sigaction previous_actions[NSIG];
M(SIGIO);

#if OS(LINUX)
// SIGPWR is intentionally excluded: JSC uses it for GC thread suspend/resume
// (see WTF/wtf/posix/ThreadingPOSIX.cpp). Replacing its handler can terminate
// the process when the GC next signals a thread.
#define FOR_EACH_LINUX_ONLY_SIGNAL(M) \
M(SIGPOLL); \
M(SIGPWR); \
M(SIGSTKFLT);
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.

#endif
Expand Down
50 changes: 50 additions & 0 deletions test/js/bun/spawn/spawnSync-sigpwr-gc.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { expect, test } from "bun:test";
import { symlinkSync } from "fs";
import { bunEnv, bunExe, isLinux, tempDir } from "harness";
import { join } from "path";

// Regression: spawnSync's signal-forwarding list included SIGPWR on Linux.
// JSC uses SIGPWR to suspend/resume threads for GC. Bun.openInEditor spawns a
// detached thread per call that runs spawnSync; concurrent calls race on the
// process-wide previous-handler table and can leave SIGPWR at SIG_DFL. The
// next GC suspend then terminates the process with signal 30.
test.skipIf(!isLinux)("spawnSync signal forwarding does not clobber JSC's SIGPWR handler", async () => {
const sleepBin = Bun.which("sleep");
expect(sleepBin).toBeTruthy();

using dir = tempDir("spawnSync-sigpwr", {
"run.js": `
for (let i = 0; i < 64; i++) {
try { Bun.openInEditor("0.2"); } catch {}
}
let junk = [];
for (let i = 0; i < 2000; i++) {
junk.push({ a: new Uint8Array(4096).fill(i), b: { c: i } });
}
for (let i = 0; i < 30; i++) Bun.gc(true);
console.log("ok");
`,
});
// Make `code` (first in the editor preference list) resolve to `sleep`, so
// each background spawnSync holds its signal-forwarding window open long
// enough for threads to overlap.
symlinkSync(sleepBin!, join(String(dir), "code"));

await using proc = Bun.spawn({
cmd: [bunExe(), join(String(dir), "run.js")],
env: {
...bunEnv,
PATH: String(dir),
EDITOR: "",
VISUAL: "",
},
stdout: "pipe",
stderr: "inherit",
});

const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);

expect(proc.signalCode).toBeNull();
expect(stdout.trim()).toBe("ok");
expect(exitCode).toBe(0);
Comment on lines +41 to +49
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Pipe stderr for better CI failure diagnostics.

With stderr: "inherit", you cannot programmatically assert stderr contents or surface them in the test diff on failure. Per repo conventions for subprocess tests using bunEnv, prefer piping stderr and asserting it's empty (after filtering ASAN noise if needed).

♻️ Suggested change 
     stdout: "pipe",
-    stderr: "inherit",
+    stderr: "pipe",
   });

-  const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
+  const [stdout, stderr, exitCode] = await Promise.all([
+    proc.stdout.text(),
+    proc.stderr.text(),
+    proc.exited,
+  ]);

   expect(proc.signalCode).toBeNull();
   expect(stdout.trim()).toBe("ok");
+  if (exitCode !== 0) {
+    expect(stderr).toBe("");
+  }
   expect(exitCode).toBe(0);

Based on learnings: "In oven-sh/bun Jest/Bun test files under test/js/ that spawn subprocesses using bunEnv from the harness module, it's safe and intentional to assert expect(stderr).toBe("") unconditionally" and "write if (exitCode !== 0) { expect(stderr).toBe(""); } immediately before expect(exitCode).toBe(0)."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
stdout: "pipe",
stderr: "inherit",
});
const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
expect(proc.signalCode).toBeNull();
expect(stdout.trim()).toBe("ok");
expect(exitCode).toBe(0);
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([
proc.stdout.text(),
proc.stderr.text(),
proc.exited,
]);
expect(proc.signalCode).toBeNull();
expect(stdout.trim()).toBe("ok");
if (exitCode !== 0) {
expect(stderr).toBe("");
}
expect(exitCode).toBe(0);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/js/bun/spawn/spawnSync-sigpwr-gc.test.ts` around lines 41 - 49, Change
the subprocess spawn options to pipe stderr (replace stderr: "inherit" with
stderr: "pipe"), capture proc.stderr.text() alongside proc.stdout.text() and
proc.exited (e.g. await Promise.all([... , proc.stderr.text(), proc.exited])),
store that value in a stderr variable, and insert the CI-friendly assertion if
(exitCode !== 0) { expect(stderr).toBe(""); } immediately before
expect(exitCode).toBe(0); keep the existing checks on proc.signalCode and
stdout.trim() as-is.

});
Loading