Don't forward SIGPWR in spawnSync signal handling - #31183
Merged
Claude / Claude Code Review
completed
May 21, 2026 in 18m 15s
Code review found 2 potential issues
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 2 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟣 Pre-existing | src/jsc/bindings/c-bindings.cpp:940-942 |
SIGPOLL/SIGIO and SIGIOT/SIGABRT are aliases — double registration loses the original handler |
| 🟣 Pre-existing | src/jsc/bindings/c-bindings.cpp:936-942 |
Underlying previous_actions[] race remains — only SIGPWR symptom removed |
Annotations
Check notice on line 942 in src/jsc/bindings/c-bindings.cpp
claude / Claude Code Review
SIGPOLL/SIGIO and SIGIOT/SIGABRT are aliases — double registration loses the original handler
Pre-existing nit, since you're already curating this list: `SIGPOLL` is an alias of `SIGIO` (signal 29) on Linux, and `SIGIOT` is an alias of `SIGABRT` (signal 6) on all POSIX targets — so `Bun__registerSignalsForForwarding()` calls `sigaction()` twice on the same number, the second call overwrites `previous_actions[N]` with the forwarding handler we just installed, and `Bun__unregisterSignalsForForwarding()` then "restores" the forwarding lambda instead of the original. Practical impact is low
Check notice on line 942 in src/jsc/bindings/c-bindings.cpp
claude / Claude Code Review
Underlying previous_actions[] race remains — only SIGPWR symptom removed
Note (pre-existing, non-blocking): the underlying race on `previous_actions[]` described in the PR body is still present — `Bun.openInEditor` calls `Bun__registerSignalsForForwarding`/`Bun__unregisterSignalsForForwarding` from detached `std::thread`s with no synchronization, contradicting the "We only ever use bun.spawnSync on the main thread" comment at line ~912. Concurrent calls can still leave SIGINT/SIGTERM at the forwarding lambda or `SIG_DFL` instead of `onExitSignal`, so TTY restoration
Loading