Skip to content

Add bun.sys.sigaction with bionic-correct layout for Android#30389

Merged
dylan-conway merged 6 commits into
mainfrom
farm/f921133c/android-sigaction
May 8, 2026
Merged

Add bun.sys.sigaction with bionic-correct layout for Android#30389
dylan-conway merged 6 commits into
mainfrom
farm/f921133c/android-sigaction

Conversation

@robobun

@robobun robobun commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Problem

std.c.Sigaction / std.c.sigset_t for .linux assume the glibc/musl layout: { handler, mask[128B], flags, restorer }. bionic LP64 is { int sa_flags; sa_handler; sigset_t sa_mask; sa_restorer } where sigset_t is a single unsigned long.

When Bun calls std.posix.sigaction() on Android, bionic reads sa_handler from offset 8 — which is Zig's mask[0]:

  • Every handler installed with mask = sigemptyset() (SIGPIPE/SIGXFSZ in main, the crash handler, SIGINT in repl/filter_run/multi_run/Coordinator) silently becomes SIG_DFL. Broken-pipe kills the process, no crash trace on SEGV, Ctrl+C isn't caught.
  • WaiterThread.reloadHandlers() sets mask[0] = 1<<16 (SIGCHLD), so bionic installs the handler 0x10000. When SIGCHLD fires, the kernel jumps there → SEGV_MAPERR, rip=0x10000, rdi=17. Reproduces 100% on a full Android emulator (not under qemu-user).

Fix

Add bun.sys.{Sigaction, sigset_t, sigemptyset, sigaddset, sigaction} in src/sys/sys.zig:

  • Android: an extern struct matching bionic bits/signal_types.h (__LP64__) and an @extern to libc sigaction that takes it. sigset_t = c_ulong.
  • Everywhere else: transparent aliases of std.posix.*, so nothing changes.

Replace all nine callsites (main.zig, crash_handler.zig, process.zig, repl.zig, filter_run.zig, multi_run.zig, Coordinator.zig, Global.zig).

A comptime tripwire fires if @sizeOf(std.c.Sigaction) ever shrinks to the bionic 32 bytes, so the workaround can be dropped once the Zig stdlib bug is fixed upstream.

Also adds zig build check-android[-debug], gated on -Dandroid_ndk_sysroot like check-freebsd, so the Android-only struct gets compile-checked.

Verification

$ zig build check-android-debug -Dandroid_ndk_sysroot=…/sysroot
check-android-debug success
  compile obj bun-debug Debug x86_64-linux-android  success
  compile obj bun-debug Debug aarch64-linux-android success

bun run zig:check-all still passes all 16 targets. Host smoke tests (spawn/exit-code, spawn/spawn-signal, spawn/spawn-kill-signal, plus BUN_FEATURE_FLAG_FORCE_WAITER_THREAD=1 to hit the modified SIGCHLD path) pass.

No runtime test is included because the misbehaviour only surfaces on a real Android kernel, which CI does not run.

std.c.Sigaction and std.c.sigset_t on .linux assume the glibc/musl
layout (handler, 128-byte mask, flags). bionic LP64 is
{ int sa_flags; sa_handler; sigset_t sa_mask; sa_restorer } with
sigset_t = unsigned long. Passing the glibc-shaped struct to bionic
sigaction() makes it read sa_handler from offset 8 — Zig's mask[0].
A zeroed mask silently installs SIG_DFL; a mask with SIGCHLD set
(WaiterThread.reloadHandlers) installs 0x10000 and segfaults on
delivery.

Add bun.sys.{Sigaction,sigset_t,sigemptyset,sigaddset,sigaction}: the
bionic extern struct on Android, transparent aliases to std.posix
elsewhere. Replace every std.posix.sigaction callsite.

Includes a comptime tripwire that fires once std.c.Sigaction gains a
bionic case upstream, and a zig build check-android step (gated on
-Dandroid_ndk_sysroot like check-freebsd).
@github-actions github-actions Bot added the claude label May 8, 2026
@robobun

robobun commented May 8, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 2:12 AM PT - May 8th, 2026

@robobun, your commit bd939db has 3 failures in Build #52750 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 30389

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

bun-30389 --bun

@coderabbitai

coderabbitai Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

Review 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. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 63bb2915-743e-41d0-aef3-d7b7991172cc

📥 Commits

Reviewing files that changed from the base of the PR and between 0d03ce5 and bd939db.

📒 Files selected for processing (1)
  • test/internal/sigaction-layout.test.ts

Walkthrough

Refactors POSIX signal handling to use an Android-aware bun.sys abstraction, migrates many sigaction usages to bun.sys, adds a TestingAPI + JS binding and POSIX tests to validate sigaction layout, and guards Android build checks on NDK presence.

Changes

Signal Handling Abstraction Migration

Layer / File(s) Summary
Signal Type & API Abstraction Layer
src/sys/sys.zig
New bun.sys.sigset_t, bun.sys.Sigaction, bun.sys.sigemptyset(), bun.sys.sigaddset(), and bun.sys.sigaction() provide Android-specific signal ABI implementations for bionic libc while delegating to posix on other platforms.
Main & Runtime Signal Setup
src/main.zig, src/runtime/api/bun/process.zig
Signal handler registrations for SIG.PIPE, SIG.XFSZ, and SIGCHLD are updated to use bun.sys types and functions instead of std.posix equivalents.
Process & REPL Abort Handlers
src/cli/filter_run.zig, src/cli/multi_run.zig, src/cli/repl.zig, src/cli/test/parallel/Coordinator.zig
SIGINT/SIGTERM handler registration and restoration in abort handlers migrate from std.posix.Sigaction/std.posix.sigaction to bun.sys equivalents, keeping handler logic and flags unchanged.
Global Panic Signal Handler
src/bun_core/Global.zig
raiseIgnoringPanicHandler POSIX signal setup switches from std.c/std.posix APIs to bun.sys equivalents while preserving handler reset and abort flow.
Crash Handler Segfault Setup
src/crash_handler/crash_handler.zig
Segfault/crash signal registration functions (updatePosixSegfaultHandler, resetOnPosix, crash()) migrate to construct and register bun.sys.Sigaction structures via bun.sys.sigaction() instead of std.posix calls.
Testing APIs & Tests
src/sys_jsc/error_jsc.zig, src/js/internal-for-testing.ts, test/internal/sigaction-layout.test.ts
Adds TestingAPIs.sigactionLayout, exposes sigactionLayout to JS, and adds a POSIX-gated test verifying libc round-trip readback and struct size/handler pointer properties.
CLI Integration Test
test/cli/run/run-propagate-sigkill.test.ts
Adds a POSIX-only integration test that spawns a child which self-terminates with SIGKILL and asserts the parent observes/re-raises SIGKILL.
Build Configuration
build.zig
Android semantic-check build steps (check-android, check-android-debug) are now conditionally registered only when android_ndk_sysroot is non-null.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding bun.sys.sigaction with bionic-correct layout for Android to fix signal handling on Android platforms.
Description check ✅ Passed The description comprehensively covers the problem statement, the fix implemented, and verification steps. It follows the template with clear sections explaining what the PR does and how it was verified.
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.

Comment thread src/sys/sys.zig
pub const handler_fn = *align(1) const fn (i32) callconv(.c) void;
pub const sigaction_fn = *const fn (i32, *const posix.siginfo_t, ?*anyopaque) callconv(.c) void;

flags: c_uint,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pretty sure this should be c_int instead of c_uint

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

bionic does declare int sa_flags, but SA_RESETHAND = 0x80000000 — Zig refuses to coerce that into a c_int literal, so .flags = SA.SIGINFO | SA.RESTART | SA.RESETHAND in crash_handler.zig / filter_run.zig / multi_run.zig would stop compiling on Android. c_uint is ABI-identical (same size/align) and is what std.c.Sigaction uses for every other Linux libc for the same reason. Added a comment at the field explaining this.

Comment thread src/sys/sys.zig Outdated
robobun and others added 5 commits May 8, 2026 06:37
Installs a known SIGUSR2 handler via bun.sys.sigaction, reads it
back via sigaction(sig, NULL, &old), and asserts the handler
pointer and SA_RESTART flag survive the trip through libc. That
holds iff bun.sys.Sigaction's field offsets agree with the host
libc's struct sigaction — the property that is violated by
std.posix.Sigaction on Android bionic.
raiseIgnoringPanicHandler re-raises a child's terminating signal,
which can be SIGKILL (bun run after kill -9). std.posix.sigaction
does 'else => unreachable' on the EINVAL that follows, so the
previous non-Android path was a reachable-unreachable regression.
Call std.c.sigaction directly and discard the result instead.

Also:
- comptime tripwire now compares @offsetof(flags)/@offsetof(handler)
  instead of @sizeof
- note why sa_flags stays c_uint (SA_RESETHAND = 0x80000000 doesn't
  fit c_int; ABI-identical, matches std.c.Sigaction on other Linux)
- add test/cli/run/run-propagate-sigkill.test.ts to pin the
  bun run -> SIGKILL re-raise path
A static import throws at module load when the export is absent,
which the junit reporter records as zero test failures. require()
inside the test lets expect(...).toBeFunction() fail the test
properly.
@dylan-conway dylan-conway merged commit d5945cf into main May 8, 2026
74 of 77 checks passed
@dylan-conway dylan-conway deleted the farm/f921133c/android-sigaction branch May 8, 2026 07:49
@claude claude Bot mentioned this pull request May 9, 2026
springmin pushed a commit to springmin/bun that referenced this pull request May 10, 2026
…#30389)

## Problem

`std.c.Sigaction` / `std.c.sigset_t` for `.linux` assume the glibc/musl
layout: `{ handler, mask[128B], flags, restorer }`. bionic LP64 is `{
int sa_flags; sa_handler; sigset_t sa_mask; sa_restorer }` where
`sigset_t` is a single `unsigned long`.

When Bun calls `std.posix.sigaction()` on Android, bionic reads
`sa_handler` from offset 8 — which is Zig's `mask[0]`:

* Every handler installed with `mask = sigemptyset()` (SIGPIPE/SIGXFSZ
in `main`, the crash handler, SIGINT in
repl/filter_run/multi_run/Coordinator) silently becomes `SIG_DFL`.
Broken-pipe kills the process, no crash trace on SEGV, Ctrl+C isn't
caught.
* `WaiterThread.reloadHandlers()` sets `mask[0] = 1<<16` (SIGCHLD), so
bionic installs the handler `0x10000`. When SIGCHLD fires, the kernel
jumps there → `SEGV_MAPERR`, `rip=0x10000`, `rdi=17`. Reproduces 100% on
a full Android emulator (not under qemu-user).

## Fix

Add `bun.sys.{Sigaction, sigset_t, sigemptyset, sigaddset, sigaction}`
in `src/sys/sys.zig`:

* **Android**: an `extern struct` matching bionic `bits/signal_types.h`
(`__LP64__`) and an `@extern` to libc `sigaction` that takes it.
`sigset_t = c_ulong`.
* **Everywhere else**: transparent aliases of `std.posix.*`, so nothing
changes.

Replace all nine callsites (`main.zig`, `crash_handler.zig`,
`process.zig`, `repl.zig`, `filter_run.zig`, `multi_run.zig`,
`Coordinator.zig`, `Global.zig`).

A comptime tripwire fires if `@sizeOf(std.c.Sigaction)` ever shrinks to
the bionic 32 bytes, so the workaround can be dropped once the Zig
stdlib bug is fixed upstream.

Also adds `zig build check-android[-debug]`, gated on
`-Dandroid_ndk_sysroot` like `check-freebsd`, so the Android-only struct
gets compile-checked.

## Verification

```
$ zig build check-android-debug -Dandroid_ndk_sysroot=…/sysroot
check-android-debug success
  compile obj bun-debug Debug x86_64-linux-android  success
  compile obj bun-debug Debug aarch64-linux-android success
```

`bun run zig:check-all` still passes all 16 targets. Host smoke tests
(`spawn/exit-code`, `spawn/spawn-signal`, `spawn/spawn-kill-signal`,
plus `BUN_FEATURE_FLAG_FORCE_WAITER_THREAD=1` to hit the modified
SIGCHLD path) pass.

No runtime test is included because the misbehaviour only surfaces on a
real Android kernel, which CI does not run.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Signed-off-by: Sisyphus <sisyphus@ohos-bun.dev>
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.

2 participants