Skip to content

Commit 4559bcc

Browse files
committed
Guard sigchld and sigpipe; add unit tests
1 parent d1eb663 commit 4559bcc

4 files changed

Lines changed: 90 additions & 1 deletion

File tree

bin_tests/tests/crashtracker_bin_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ fn test_collector_no_allocations_stacktrace_modes() {
299299
let _ = fs::remove_file(&detector_log_path);
300300

301301
let config = CrashTestConfig::new(
302-
BuildProfile::Debug,
302+
BuildProfile::Release,
303303
TestMode::RuntimePreloadLogger,
304304
CrashType::NullDeref,
305305
)

libdd-crashtracker/src/collector/crash_handler.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
use super::collector_manager::Collector;
77
use super::receiver_manager::Receiver;
8+
use super::saguard::SaGuard;
89
use super::signal_handler_manager::chain_signal_handler;
910
use crate::crash_info::Metadata;
1011
use crate::shared::configuration::CrashtrackerConfiguration;
@@ -263,6 +264,15 @@ fn handle_posix_signal_impl(
263264
return Ok(());
264265
}
265266

267+
// Block SIGCHLD and SIGPIPE during crash handling. Our collector spawns child processes
268+
// and writes to pipes, both of which can generate these signals. Rather than risk
269+
// re-entering a signal handler or aborting due to SIGPIPE, suppress them for the
270+
// duration and restore when the guard drops
271+
let _sa_guard = SaGuard::new(&[
272+
nix::sys::signal::Signal::SIGCHLD,
273+
nix::sys::signal::Signal::SIGPIPE,
274+
]);
275+
266276
// Take config and metadata out of global storage.
267277
// We borrow via raw pointer and intentionally leak (do not reconstruct the Box) to avoid
268278
// calling `drop`, and therefore `free`, inside a signal handler, which is not

libdd-crashtracker/src/collector/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ mod crash_handler;
1010
mod emitters;
1111
mod process_handle;
1212
mod receiver_manager;
13+
mod saguard;
1314
mod signal_handler_manager;
1415
mod spans;
1516

libdd-crashtracker/src/collector/saguard.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,81 @@ impl<const N: usize> Drop for SaGuard<N> {
8484
);
8585
}
8686
}
87+
88+
#[cfg(test)]
89+
mod tests {
90+
use super::*;
91+
use nix::sys::signal::{self, Signal};
92+
use nix::unistd::Pid;
93+
use std::sync::atomic::{AtomicBool, Ordering};
94+
95+
#[test]
96+
#[cfg_attr(miri, ignore)]
97+
fn signal_is_ignored_while_guard_is_active() {
98+
let guard = SaGuard::<1>::new(&[Signal::SIGUSR1]).unwrap();
99+
100+
// Send SIGUSR1 to the process. The default action is to terminate, so if
101+
// the guard didn't set SIG_IGN this test process would die
102+
signal::kill(Pid::this(), Signal::SIGUSR1).unwrap();
103+
104+
// If we get here, signal was successfully ignored
105+
drop(guard);
106+
}
107+
108+
/// After the guard is dropped, the original handler should be restored.
109+
/// Install a custom handler, create a guard,drop the guard, then send the
110+
/// signal and verify the custom handler fires
111+
#[test]
112+
#[cfg_attr(miri, ignore)]
113+
fn original_handler_restored_after_drop() {
114+
static HANDLER_CALLED: AtomicBool = AtomicBool::new(false);
115+
116+
extern "C" fn custom_handler(_: libc::c_int) {
117+
HANDLER_CALLED.store(true, Ordering::SeqCst);
118+
}
119+
120+
// Install a custom handler
121+
let custom_action = SigAction::new(
122+
SigHandler::Handler(custom_handler),
123+
SaFlags::empty(),
124+
signal::SigSet::empty(),
125+
);
126+
let prev = unsafe { signal::sigaction(Signal::SIGUSR2, &custom_action).unwrap() };
127+
128+
// Create then drop the guard (dropped when out of scope)
129+
{
130+
let _guard = SaGuard::<1>::new(&[Signal::SIGUSR2]).unwrap();
131+
signal::kill(Pid::this(), Signal::SIGUSR2).unwrap();
132+
assert!(
133+
!HANDLER_CALLED.load(Ordering::SeqCst),
134+
"custom handler should not fire while guard is active"
135+
);
136+
}
137+
// Guard is dropped; custom handler should be restored
138+
HANDLER_CALLED.store(false, Ordering::SeqCst);
139+
unsafe {
140+
libc::raise(Signal::SIGUSR2 as libc::c_int);
141+
}
142+
assert!(
143+
HANDLER_CALLED.load(Ordering::SeqCst),
144+
"custom handler should fire after guard is dropped"
145+
);
146+
147+
// Restore original handler
148+
unsafe {
149+
signal::sigaction(Signal::SIGUSR2, &prev).unwrap();
150+
}
151+
}
152+
153+
#[test]
154+
#[cfg_attr(miri, ignore)]
155+
fn multiple_signals_ignored() {
156+
let guard = SaGuard::<2>::new(&[Signal::SIGUSR1, Signal::SIGUSR2]).unwrap();
157+
158+
// Both signals should be safely ignored
159+
signal::kill(Pid::this(), Signal::SIGUSR1).unwrap();
160+
signal::kill(Pid::this(), Signal::SIGUSR2).unwrap();
161+
162+
drop(guard);
163+
}
164+
}

0 commit comments

Comments
 (0)