Add native crash handler: die loudly on C-thread fatal signals#889
Merged
Conversation
A SIGSEGV on a thread the Go runtime does not own (e.g. a DuckDB TaskScheduler thread crashing inside an extension) is routed through the runtime's badsignal path and can leave the process alive but wedged: the crashed thread never releases its locks, and every thread that later touches them blocks forever. In production this turned a postgres_scanner segfault during a JDBC catalog scan into a query that ran 30+ hours, pinning a 15-CPU worker pod and blocking control-plane drains, with no log line anywhere. internal/crashhandler installs (via C constructor, before the Go runtime registers its handlers, so runtime.sigfwdgo chains to it) a native handler for SIGSEGV/SIGBUS/SIGILL/SIGFPE/SIGABRT that writes a grep-able marker + native backtrace to stderr and re-raises with the default disposition, so the process dies and the control plane's existing crash-reap machinery takes over. Faults raised by Go code are not forwarded by the runtime and keep producing ordinary Go panics. Package tests re-exec the test binary and assert death-by-signal plus the stderr marker for a C-thread segfault and a cgo-call segfault, and that a Go nil dereference still panics normally. Not assertable in the e2e harness (no SQL deterministically segfaults a worker) — noted in tests/e2e-mw-dev/README.md.
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: needs review Warnings
|
glibc's backtrace() through a signal frame can yield as few as two frames on linux/amd64, which failed the test's frame-count assertion in CI (the handler itself worked: marker + death by signal). Assert one frame address plus the fault-address line instead, matching both the glibc and macOS backtrace_symbols_fd formats. Make the shallow-trace case actionable: the report line now includes the faulting PC (from the signal ucontext on linux/darwin amd64+arm64) and si_addr, printed with an async-signal-safe hex writer, so a crash site can be resolved with addr2line even without deep unwind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
We found production worker pods pinned for 17–33 hours by JDBC catalog queries (
getColumns()/getPrimaryKeys()) that never finished. Native stack captures (gdb via ephemeral debug container) showed the mechanism:postgres_scanner(PostgresCatalogSet::TryLoadEntries, duringduckdb_indexes()init);runtime.badsignal → raisebadsignal) and the thread wedged there instead of the process dying — no log line anywhere;Executor::ExecuteTaskat exactly 1 core;activeforever, which also pinned control-plane pods in their (by-design unbounded) drain across multiple deploy generations.Killing the process is strictly better than this: the CP's existing health-check/reap machinery handles worker death cleanly within seconds (verified live — when gdb released the pending signal the worker died, the session errored, and the CP finished draining immediately).
What
internal/crashhandler(linked into all three binaries) installs a native handler forSIGSEGV/SIGBUS/SIGILL/SIGFPE/SIGABRTfrom a C constructor, i.e. before the Go runtime registers its own handlers — so the runtime saves it as the pre-existing handler and forwards non-Go faults to it (runtime.sigfwdgo). The handler is async-signal-safe: it writes a grep-able marker (duckgres: fatal native signal) plus abacktrace_symbols_fdnative backtrace to stderr, restores the default disposition, and re-raises — the process dies with the original signal.Faults raised by Go code are untouched: the runtime doesn't forward those, so ordinary Go panics (nil deref etc.) behave exactly as before.
Tests
TDD package tests (
internal/crashhandler) re-exec the test binary and assert on the child's fate:Not assertable in the e2e harness — no SQL statement deterministically segfaults a worker; documented in
tests/e2e-mw-dev/README.md("deliberately not covered"). Verified green on darwin/arm64; Linux runs in CI.Follow-ups (separate)
🤖 Generated with Claude Code