Repository navigation
fix(runtime): make the SIGSEGV/SIGBUS trap async-signal-safe - #1655
Conversation
Lorenzobattistela
left a comment
There was a problem hiding this comment.
Thanks for this. The async-signal-safety point is real, but the change as written regresses two things the old path did right, so I'm requesting changes. All gates pass with this merged onto main bedc964 (repo 54/54, test 1656/1656, perf 124/124, ping 62/62, safe 0 false rejects). Neither regression is covered by a test.
-
A stack fault now loses output the program already printed. Stdout is fully buffered when it goes to a pipe or file, and
err_trapno longer flushes it. Taketests/io/stack_fault_trap.bend, adda : Unit <- IO.print("BEFORE")before the deep call, build it withbend x.bend -o x, then run./x | cat:- main:
BEFOREon stdout, thenbend: memory fault (machine stack overflow?) - this PR: only the error line;
BEFOREis gone
This happens with
--threads 1and--threads 4. Running throughbend x.bendhides it, so the byte-identical checks in the description didn't catch it. - main:
-
When several threads fail at once, their error lines interleave.
err_writesends the line as three separatewrite(2)calls. Before,fprintfheld the stderr lock and sent the whole line at once. A parallel tree of 2^14 leaves that each overflow a Nat printed garbled stderr in 80 of 100 runs here (e.g.bend: bend: bend: a Nat past the largest immediate 2^48-1a Nat past ...) and in 0 of 100 on main. Since tests compare stderr against their#|lines, any test with parallel failures would become flaky.
Suggested shape:
- Build
"bend: <msg>\n"in a stack buffer and send it with onewrite(2). That's still async-signal-safe, it can't interleave (the line is under PIPE_BUF), and it drops thessize_t n ...; (void)n;workaround. Loop on short writes and EINTR so the numbered line is never cut off. - Keep the flush in
err_trapwhen it's free:if (ftrylockfile(stdout) == 0) { fflush(stdout); funlockfile(stdout); }. The stdio locks are recursive on macOS and glibc, so a fault on the thread that holds the stdout lock doesn't deadlock. If another thread holds it, the trap skips the flush instead of waiting. - Guard
msgagainst NULL.fprintfprinted "(null)", butstrlen(NULL)faults. From reading the code:gpu_failpasses[[err localizedDescription] UTF8String], which is NULL whenerris nil. - Drop the "no buffered output worth risking a hang for" comment; point 1 shows there often is.
- Pin both cases in tests: an
IO.printbefore the fault instack_fault_trap.bendwith#|BEFOREabove the error line, and ideally one parallel fail-stop test that expects a single clean line.
Also, this covers only the stdio part of #1624. The atexit/termios restore and the re-raise of the real signal are still open, so "Part of #1624" fits better than "Resolves".
90aa118 to
80a8ea2
Compare
|
@Lorenzobattistela thanks — both regressions were real, and the review's shape is what the fix is now. 1. The trap lost output it had already printed. if (ftrylockfile(stdout) == 0) { fflush(stdout); funlockfile(stdout); }
Pinned in 2. Threads that fail at once interleaved. The line is one
The comment. Gone — the flush is there, so "no buffered output worth risking a hang for" was wrong. Tests. "Part of #1624". The description says that now: the atexit/termios restore and the re-raise of the real signal are still open. One thing to flag: |
) A 128-byte buffer cut ERR_CNTS (137 chars) mid-word; 512 holds every numbered line and stays at PIPE_BUF's floor, so the line goes out whole in one write(2), with no loop and no errno. stdio.h declares the lock calls on both hosts (_GNU_SOURCE, Darwin), and the trap exits holding the stdout lock, so the externs and the funlockfile go. err_exit ends in _exit for both callers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
) A 128-byte buffer cut ERR_CNTS (137 chars) mid-word; 512 holds every numbered line and stays at PIPE_BUF's floor, so the line goes out whole in one write(2), with no loop and no errno. stdio.h declares the lock calls on both hosts (_GNU_SOURCE, Darwin), and the trap exits holding the stdout lock, so the externs and the funlockfile go. err_exit ends in _exit for both callers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ab848c0 to
b8d9e54
Compare
) A 128-byte buffer cut ERR_CNTS (137 chars) mid-word; 512 holds every numbered line and stays at PIPE_BUF's floor, so the line goes out whole in one write(2), with no loop and no errno. stdio.h declares the lock calls on both hosts (_GNU_SOURCE, Darwin), and the trap exits holding the stdout lock, so the externs and the funlockfile go. err_exit ends in _exit for both callers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ndlang#1655) The trap's flush ran on a FILE the faulting thread could be mid-update on (the stdio locks are recursive, so ftrylockfile lets it through), and a second fault there, with SIGSEGV blocked, died silently or hung. The trap now flushes once, under SA_NODEFER: a fault in that flush re-enters and skips it, and the line still goes out. The line is the constant ERRS[7], so the trap writes it from a literal, with no buffer. err_fail is main's again: fprintf prints a message of any length whole (a 504-byte cap cut Metal's diagnostics) and in one write. io_errs sends its text and newline in one write too, so a fail-stop cannot land between them. parallel_fail_stop passed on main as well, so it goes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…two frames (bendlang#1655) Two threads that faulted at once both saw seen == 0, or the loser wrote its line and _exited mid-flush, cutting stdout. The first fault now takes an atomic flag and the others pause until its _exit; only its own thread (a fault in the flush) passes and skips the flush. It waits up to 100ms for the stdout lock instead of dropping the flush when an fwrite holds it. The altstack is 4 * SIGSTKSZ, so a nested frame from SA_NODEFER fits beside fflush's on glibc's minimal size. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…endlang#1655) The 100ms wait for the stdout lock let the program run on after a fault, so a main that returned in that window exited 0 with no line. The trap now flushes stdout only if its lock is free, with no wait, then writes the line (stdout first, so a combined log keeps its order). Faults on other threads still pause until its _exit. A thread-local flag, set first by pool_stack so the trap never allocates it, replaces the shared pthread_t: a fault in the trap's own flush skips to the line. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…bendlang#1655) The thread-local flag was first touched inside the trap on threads that never ran pool_stack (io_help, GCD callbacks), and on Darwin that first touch calls malloc in the handler. err_claim stores the first thread's pthread_self in an atomic instead: it needs no setup on any thread, and err_fail takes it too, so a fail-stop and a fault no longer race to print. The other thread waits for its _exit; a fault in the claimer's own exit path skips its flush. IO.print writes text and newline in one fwrite, as io_errs now does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…stays out of hot code (bendlang#1655) A thread that faulted inside a stdout write, after another had claimed, paused forever holding the stdout lock, so the claimer's fflush in err_fail never returned: a hang with no line. The other thread now waits a second for the claimer's _exit, then prints its own line and exits. err_fail flushes only when it holds the claim, so a fault in its own flush goes straight to the line, as the trap's does. err_fail is OUTLINE and err_claim FAR: inlined at its call sites, the larger body cost editdist and merkle SEQ-CPU 1.24x and 1.32x on the minis (3 runs each); outlined, perf is 124 / 124. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
err_trap is a signal handler, but it ended in err_fail, which calls fflush(stdout) and fprintf(stderr, ...) -- neither async-signal-safe. A fault while another thread holds a stdio lock could hang the process instead of ending it. err_write now goes straight to fd 2 with write(2), and err_trap calls it directly, dropping the fflush: no stdio lock is touched from the handler. The normal err_fail path still flushes stdout first, so output a program printed before a fail-stop is preserved. The emitted message and the exit code are unchanged on both paths, so every numbered-error test still prints its line and exits 1. Resolves bendlang#1624
) A 128-byte buffer cut ERR_CNTS (137 chars) mid-word; 512 holds every numbered line and stays at PIPE_BUF's floor, so the line goes out whole in one write(2), with no loop and no errno. stdio.h declares the lock calls on both hosts (_GNU_SOURCE, Darwin), and the trap exits holding the stdout lock, so the externs and the funlockfile go. err_exit ends in _exit for both callers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ndlang#1655) The trap's flush ran on a FILE the faulting thread could be mid-update on (the stdio locks are recursive, so ftrylockfile lets it through), and a second fault there, with SIGSEGV blocked, died silently or hung. The trap now flushes once, under SA_NODEFER: a fault in that flush re-enters and skips it, and the line still goes out. The line is the constant ERRS[7], so the trap writes it from a literal, with no buffer. err_fail is main's again: fprintf prints a message of any length whole (a 504-byte cap cut Metal's diagnostics) and in one write. io_errs sends its text and newline in one write too, so a fail-stop cannot land between them. parallel_fail_stop passed on main as well, so it goes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…two frames (bendlang#1655) Two threads that faulted at once both saw seen == 0, or the loser wrote its line and _exited mid-flush, cutting stdout. The first fault now takes an atomic flag and the others pause until its _exit; only its own thread (a fault in the flush) passes and skips the flush. It waits up to 100ms for the stdout lock instead of dropping the flush when an fwrite holds it. The altstack is 4 * SIGSTKSZ, so a nested frame from SA_NODEFER fits beside fflush's on glibc's minimal size. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…endlang#1655) The 100ms wait for the stdout lock let the program run on after a fault, so a main that returned in that window exited 0 with no line. The trap now flushes stdout only if its lock is free, with no wait, then writes the line (stdout first, so a combined log keeps its order). Faults on other threads still pause until its _exit. A thread-local flag, set first by pool_stack so the trap never allocates it, replaces the shared pthread_t: a fault in the trap's own flush skips to the line. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…bendlang#1655) The thread-local flag was first touched inside the trap on threads that never ran pool_stack (io_help, GCD callbacks), and on Darwin that first touch calls malloc in the handler. err_claim stores the first thread's pthread_self in an atomic instead: it needs no setup on any thread, and err_fail takes it too, so a fail-stop and a fault no longer race to print. The other thread waits for its _exit; a fault in the claimer's own exit path skips its flush. IO.print writes text and newline in one fwrite, as io_errs now does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…stays out of hot code (bendlang#1655) A thread that faulted inside a stdout write, after another had claimed, paused forever holding the stdout lock, so the claimer's fflush in err_fail never returned: a hang with no line. The other thread now waits a second for the claimer's _exit, then prints its own line and exits. err_fail flushes only when it holds the claim, so a fault in its own flush goes straight to the line, as the trap's does. err_fail is OUTLINE and err_claim FAR: inlined at its call sites, the larger body cost editdist and merkle SEQ-CPU 1.24x and 1.32x on the minis (3 runs each); outlined, perf is 124 / 124. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…bendlang#1655) sleep(1) returned early on any handled signal, so the waiting thread could print a second line while the claimer was still flushing. nanosleep keeps the time left, and its only failure here is EINTR, so the loop resumes it until the second is up (a fault on the claimer's own thread sleeps 0). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
631f64f to
b66cf64
Compare
…#1655) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Part of #1624
The problem
err_trap, the hostSIGSEGV/SIGBUShandler, ended inerr_fail, so it calledfflush(stdout)andfprintf(stderr, ...)from inside a signal handler. Two things could go wrong:The fix
err_claimpicks one thread to end the process. It records the first caller'spthread_self()in an atomic with a compare-and-swap, and only that thread flushes stdout. Any other thread waits a second (nanosleep, resumed after a signal) for that thread's_exit. It may be holding the very lock the claimer is flushing on, so after the second it prints its own line and exits rather than hang. A second fault on the claiming thread's own exit path skips the flush and goes straight to the line.err_claimneeds no setup and allocates nothing, on any thread. Botherr_failanderr_trapcall it.ftrylockfilesucceeds. Then it writes the constantbend: <ERRS[7]>\n(the text the JS trap already uses) with onewrite(2), and calls_exit(1). Stdout goes before the line, so a log that combines stdout and stderr keeps its order.SA_NODEFER, so a fault in the flush re-enters it.err_claimreturns 0 on re-entry, so the flush is skipped and the line goes out. The altstack is4 * SIGSTKSZso the nested frame fits.err_failstill prints withfprintf, so a message of any length goes out whole. It is nowOUTLINE(cold, never inlined) anderr_claimisFAR. Whenerr_failwas inlined at its call sites, its larger body made editdist and merkle SEQ-CPU 1.24x and 1.32x slower on the minis, in three runs each.io_errsandIO.printsend the text and its newline in one write. They use the byteio_cbufleaves for the NUL, so a fail-stop cannot land between the text and its newline.tests/io/stack_fault_trap.bendprintsBEFOREbefore the deep call and expects#|BEFOREabove the error line. That pins the trap's flush against removal (stdout is buffered when it goes to a pipe).Verification
Gates on b66cf64 over main b0d1ca6:
A C harness ran the emitted
err_claim,err_failanderr_trapverbatim, with stdout and stderr captured as one stream:BEFORE, lineBEFORE, exactly one line, 100/100 runsBEFORE,bend: <the fail-stop's message>onlyerr_fail's flusherr_failBEFORE, linemallochook saw no allocation in the handlerThe harness ran on macOS (arm64) only. The glibc path has not been run.
Trade-offs
Out of scope
stack_fault_traprequires the numbered line andexit 1, never death by signal.io_helpthreads have no altstack, so a stack overflow there still dies silently. That is true on main too.🤖 Generated with Claude Code