Skip to content

Commit 368a737

Browse files
sebastianstclaude
andcommitted
op-reth: print a backtrace when an integration test dies on a signal
`p2p_version::peers_negotiate_eth_69` intermittently segfaults during teardown, after its body has already passed (#20973). A binary killed by a signal produces no Rust backtrace — `RUST_BACKTRACE` only covers panics — so five recurrences over two months have yielded nothing beyond the signal number, and the crash has never reproduced locally across 570+ attempts. Investigation is currently stalled for want of a faulting stack. Core dumps are not an option here: on the CircleCI Docker executor `/proc/sys` is read-only so `core_pattern` cannot be set, and the host's pipe pattern makes the kernel write the core outside the container. An in-process handler needs no privileges, no artifacts and no CI configuration, and works locally too. Install a handler for SIGSEGV/SIGBUS/SIGILL/SIGFPE that reports the faulting thread's name and address, then its stack. The thread name alone discriminates between the candidate causes: a database `atexit` destructor racing its own live threads, background threads outliving process exit, or a runtime teardown race. The stack is emitted in two stages, which matters: - `backtrace_symbols_fd` first. It writes straight to the fd and does not call malloc, so it is safe here. This is the guaranteed output. - `std::backtrace::Backtrace` second, for names and line numbers. Its symbolization allocates heavily and faults a second time when called from a handler — killing the process before anything is printed, since SIGSEGV is blocked inside its own handler. Attempting it only after the reliable frames are already on the wire means that failure costs nothing. The handler re-raises with the default disposition, so the process still dies with the original signal and nextest still records a crash. That composes with the retry added in the parent commit: nextest prints a failed attempt's stderr even when a later attempt passes, so a recurrence yields a stack while the job stays green. `libc` is already a workspace dependency; this adds it as a dev-dependency only. Instrumentation only — no behavior change to any test, and no effect unless a fatal signal fires. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d1cb297 commit 368a737

5 files changed

Lines changed: 158 additions & 0 deletions

File tree

rust/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/op-reth/crates/node/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ alloy-op-hardforks.workspace = true
105105
alloy-rpc-types-admin.workspace = true
106106
alloy-signer-local.workspace = true
107107
futures.workspace = true
108+
# Fatal-signal handler in tests/it/crash_backtrace.rs.
109+
libc.workspace = true
108110
op-alloy-network.workspace = true
109111

110112
[features]
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
//! Prints a backtrace when the test process dies on a fatal signal.
2+
//!
3+
//! A binary killed by a signal produces no Rust backtrace — `RUST_BACKTRACE` only
4+
//! covers panics — so a crash that happens *after* a test body has passed leaves
5+
//! nothing to diagnose beyond the signal number. Installing a handler recovers the
6+
//! faulting thread's name and stack, which is the evidence needed to tell apart the
7+
//! candidate causes of a teardown crash (a database's `atexit` destructor racing its
8+
//! own live threads, background threads outliving process exit, and so on).
9+
//!
10+
//! The handler re-raises with the default disposition, so the process still dies with
11+
//! the original signal and the test runner still records a crash rather than a pass.
12+
13+
use std::sync::Once;
14+
15+
static INIT: Once = Once::new();
16+
17+
/// Installs the fatal-signal handler once per process.
18+
///
19+
/// Call at the start of a test that launches a full node. The handler stays installed
20+
/// for the lifetime of the process, so it also covers teardown after the test body has
21+
/// returned — which is when such crashes typically occur.
22+
pub(crate) fn install() {
23+
INIT.call_once(install_inner);
24+
}
25+
26+
/// Seconds allowed for backtrace capture before the watchdog kills the process.
27+
const CAPTURE_WATCHDOG_SECS: libc::c_uint = 10;
28+
29+
/// Frames captured on a fault. Deep enough for a teardown stack through a runtime.
30+
const MAX_FRAMES: usize = 64;
31+
32+
fn install_inner() {
33+
unsafe {
34+
// The first `backtrace()` call loads libgcc's unwinder and may allocate. Doing it
35+
// here means the call inside the handler does not have to.
36+
let mut warmup = [std::ptr::null_mut::<libc::c_void>(); 1];
37+
libc::backtrace(warmup.as_mut_ptr(), warmup.len() as libc::c_int);
38+
39+
let mut action: libc::sigaction = std::mem::zeroed();
40+
action.sa_sigaction = handler as *const () as usize;
41+
action.sa_flags = libc::SA_SIGINFO | libc::SA_ONSTACK;
42+
libc::sigemptyset(&raw mut action.sa_mask);
43+
44+
for sig in [libc::SIGSEGV, libc::SIGBUS, libc::SIGILL, libc::SIGFPE] {
45+
libc::sigaction(sig, &raw const action, std::ptr::null_mut());
46+
}
47+
}
48+
}
49+
50+
/// Writes directly to fd 2. `println!`/`eprintln!` take a lock and allocate, neither of
51+
/// which is legal in a signal handler; `write(2)` is async-signal-safe.
52+
fn write_stderr(bytes: &[u8]) {
53+
unsafe {
54+
libc::write(libc::STDERR_FILENO, bytes.as_ptr().cast(), bytes.len());
55+
}
56+
}
57+
58+
fn write_u64(mut n: u64) {
59+
let mut buf = [0u8; 20];
60+
let mut i = buf.len();
61+
loop {
62+
i -= 1;
63+
buf[i] = b'0' + (n % 10) as u8;
64+
n /= 10;
65+
if n == 0 {
66+
break;
67+
}
68+
}
69+
write_stderr(&buf[i..]);
70+
}
71+
72+
extern "C" fn handler(sig: libc::c_int, info: *mut libc::siginfo_t, _ctx: *mut libc::c_void) {
73+
// Everything up to the watchdog is async-signal-safe, so this much is emitted even
74+
// if the richer capture below wedges.
75+
write_stderr(b"\n=== fatal signal ");
76+
write_u64(sig as u64);
77+
write_stderr(b" in thread '");
78+
write_thread_name();
79+
write_stderr(b"'");
80+
81+
if !info.is_null() {
82+
write_stderr(b" faulting address 0x");
83+
let addr = unsafe { (*info).si_addr() } as usize;
84+
write_hex(addr as u64);
85+
}
86+
write_stderr(b" ===\n");
87+
88+
// Belt and braces: `backtrace()` is warmed up at install time so it should not
89+
// allocate here, but a wedge would otherwise hang the job until the CI timeout.
90+
unsafe {
91+
libc::signal(libc::SIGALRM, libc::SIG_DFL);
92+
libc::alarm(CAPTURE_WATCHDOG_SECS);
93+
}
94+
95+
// `backtrace_symbols_fd` writes straight to the fd and is documented not to call
96+
// malloc, unlike `std::backtrace::Backtrace`, whose symbolization allocates heavily
97+
// and faults a second time when called from here — killing the process before it can
98+
// print anything, since SIGSEGV is blocked inside its own handler.
99+
//
100+
// The tradeoff is resolution: this yields function names and addresses but no line
101+
// numbers or inlined frames. Addresses can be resolved offline with addr2line against
102+
// the same binary when more detail is needed.
103+
unsafe {
104+
let mut frames = [std::ptr::null_mut::<libc::c_void>(); MAX_FRAMES];
105+
let n = libc::backtrace(frames.as_mut_ptr(), frames.len() as libc::c_int);
106+
libc::backtrace_symbols_fd(frames.as_ptr(), n, libc::STDERR_FILENO);
107+
}
108+
109+
// Now that the reliable frames are already on the wire, try for names and line
110+
// numbers as a bonus. This is the step that can fault; if it does, the process dies
111+
// with the original signal anyway and nothing above is lost.
112+
write_stderr(b"--- symbolized (best effort; empty here means symbolization faulted) ---\n");
113+
write_stderr(std::backtrace::Backtrace::force_capture().to_string().as_bytes());
114+
write_stderr(b"\n");
115+
116+
// Die with the original signal so the runner still sees a crash. Restoring the
117+
// default disposition first prevents recursing back into this handler.
118+
unsafe {
119+
libc::alarm(0);
120+
libc::signal(sig, libc::SIG_DFL);
121+
libc::raise(sig);
122+
}
123+
}
124+
125+
fn write_hex(n: u64) {
126+
const DIGITS: &[u8; 16] = b"0123456789abcdef";
127+
let mut buf = [0u8; 16];
128+
for (i, slot) in buf.iter_mut().enumerate() {
129+
*slot = DIGITS[((n >> (60 - i * 4)) & 0xf) as usize];
130+
}
131+
write_stderr(&buf);
132+
}
133+
134+
/// The faulting thread's name is the single most useful field here: it distinguishes a
135+
/// crash in, say, a database transaction-manager thread from one in a runtime worker.
136+
///
137+
/// Writes straight from a stack buffer — building a `String` would allocate, which is
138+
/// not legal before the watchdog below is armed.
139+
fn write_thread_name() {
140+
let mut buf = [0i8; 32];
141+
let rc = unsafe { libc::pthread_getname_np(libc::pthread_self(), buf.as_mut_ptr(), buf.len()) };
142+
if rc != 0 {
143+
write_stderr(b"<unknown>");
144+
return;
145+
}
146+
let len = buf.iter().position(|&c| c == 0).unwrap_or(buf.len());
147+
let bytes = unsafe { std::slice::from_raw_parts(buf.as_ptr().cast::<u8>(), len) };
148+
write_stderr(bytes);
149+
}

rust/op-reth/crates/node/tests/it/main.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#![allow(missing_docs)]
22

3+
mod crash_backtrace;
4+
35
mod builder;
46

57
mod custom_pool;

rust/op-reth/crates/node/tests/it/p2p_version.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ const EXPECTED_ETH_VERSION: u64 = 69;
1515

1616
#[tokio::test]
1717
async fn peers_negotiate_eth_69() -> eyre::Result<()> {
18+
// This test intermittently segfaults during teardown, after the body below has
19+
// passed (ethereum-optimism/optimism#20973). The handler outlives the body, so it
20+
// reports the faulting thread and stack when that happens.
21+
crate::crash_backtrace::install();
1822
reth_tracing::init_test_tracing();
1923

2024
let (nodes, _wallet) = setup(2).await?;

0 commit comments

Comments
 (0)