Skip to content

[jsc] don't override JSC's thread suspend signal on FreeBSD - #44596

Open
samm-git wants to merge 7 commits into
oven-sh:mainfrom
samm-git:freebsd-sigusr1-suspend
Open

samm-git wants to merge 7 commits into
oven-sh:mainfrom
samm-git:freebsd-sigusr1-suspend

Conversation

@samm-git

@samm-git samm-git commented Oct 5, 2026

Copy link
Copy Markdown

What does this PR do?

Bun protects the signal JSC uses to suspend/resume threads from being overridden by a JS listener (process.on(...)) only under #if OS(LINUX):

#if OS(LINUX)
    // SIGKILL and SIGSTOP cannot be handled, and JSC needs its own signal handler to
    // suspend and resume the JS thread which we must not override.
    if (signalNumber != SIGKILL && signalNumber != SIGSTOP && signalNumber != g_wtfConfig.sigThreadSuspendResume) {
#elif OS(DARWIN) || OS(FREEBSD)
    // these signals cannot be handled
    if (signalNumber != SIGKILL && signalNumber != SIGSTOP) {   // <-- no suspend-signal guard

JSC picks that signal in wtf/posix/ThreadingPOSIX.cpp: SIGPWR on Linux, SIGUSR1 everywhere else (including FreeBSD). So on FreeBSD a process.on('SIGUSR1', ...) listener makes Bun install its own SIGUSR1 handler and clobber JSC's. The GC/VMTraps suspend signal is then delivered to Bun's handler, globalSemaphoreForSuspendResume is never posted, and anything that suspends a thread never resumes — tearing down a VM (e.g. a Web Worker, WebWorker__teardownJSCVM → destroyVM) deadlocks.

This is easy to hit: any JS dependency that handles SIGUSR1, or just process.on('SIGUSR1', () => {}), breaks GC suspension for the life of the process.

How did you verify your code works?

On FreeBSD/aarch64 (a compiled bun --compile app that registers a SIGUSR1 listener), before the change the process deadlocked on exit 5/5. With this change the same deterministic test is exit=OK 15/15, and the JS turns it exercises still pass.

Instrumentation used to confirm the mechanism:

  • A write(2) log in Thread::signalHandlerSuspendResume (to a dedicated file fd) showed the handler never executes, while the suspender's side (Thread::suspend) does run.
  • ktrace -t s showed PSIG SIGUSR1 caught ... code=SI_LWP delivered to the owner thread but to a non-JSC handler; the thread's mask is clear and its sigfastblock word is 0.
  • Live stacks on the hang: VMTraps::SignalSender::work() → Thread::suspend() → sem_wait, with a GC helper blocked in MachineThreads::tryCopyOtherThreadStacks → the process-global suspend lock the sender holds.

oven-sh/WebKit#693 (the signalHandlerSuspendResume request/generation rework) does not fix this, which is consistent: the problem is that JSC's handler is not installed at all.

Risk

Low. It only makes FreeBSD behave like Linux for sigThreadSuspendResume: Bun will refuse to install a JS listener for it, the same way it already does on Linux. macOS is unaffected (it uses Mach thread suspension, not signals), so the SIGUSR1 guard is #if OS(FREEBSD) only.

The guard that stops JS listeners from overriding the signal JSC uses for thread
suspend/resume is `#if OS(LINUX)`. JSC uses SIGPWR on Linux but SIGUSR1 on FreeBSD
(see wtf/posix/ThreadingPOSIX.cpp). So on FreeBSD, `process.on('SIGUSR1', ...)` made Bun
install its own SIGUSR1 handler and clobber JSC's. The suspend signal was then delivered
to the wrong handler, `Thread::suspend()` never got its acknowledgement, and tearing down
a VM (e.g. a Web Worker) deadlocked in `VMTraps::SignalSender` with a GC helper blocked
on the process-global suspend lock it held.

Treat `g_wtfConfig.sigThreadSuspendResume` as unhandleable on FreeBSD too, as Linux does.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Walkthrough

Signal listener installation now skips the thread suspend/resume signal on FreeBSD. Darwin continues to skip only SIGKILL and SIGSTOP.

Changes

Platform signal listener filtering

Layer / File(s) Summary
FreeBSD signal exclusions
src/jsc/bindings/BunProcess.cpp
The FreeBSD check now also skips g_wtfConfig.sigThreadSuspendResume. The Darwin check still skips only SIGKILL and SIGSTOP.

Suggested reviewers: dylan-conway

Merge Risk: Minimal · previous review 57f07

The narrowly scoped signal-filtering change has no established merge-blocking risk.

Architecture risk: Low · up to 35676

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/jsc/bindings/BunProcess.cpp: The shared Darwin/FreeBSD branch replaces the generic unhandleable-signal check. FreeBSD now also skips installing a process listener’s signal handler for g_wtfConfig.sigThreadSuspendResume, in addition to SIGKILL and SIGSTOP; Darwin retains the two-signal check.
Pre-merge checks: Passed 4

Check name Status Explanation
Description check Passed The description includes both required sections. It clearly explains the FreeBSD signal-handler bug, the code change, verification results, instrumentation, and risk.
Title check Passed The title clearly and concisely describes the main change: preventing Bun from overriding JSC's thread suspend signal on FreeBSD.
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.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

ℹ️ Recent review info

⚙️ Run configuration

  • Source mode: Auto
  • Repository: oven-sh/bun
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: b6a5ece
  • Layers: Repository YAML
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 45733af0-3841-4afe-b144-2343bcdefa2f
📥 Commits

Reviewing files that changed from the base of the PR and between 5716a6b and 57f0709.

📒 Files selected for processing: 1

  • src/jsc/bindings/BunProcess.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@samm-git

Copy link
Copy Markdown
Author

@dylan-conway hi Dylan. Any chance to get your review? Thank you.

@samm-git

Copy link
Copy Markdown
Author

@Jarred-Sumner any chance?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant