Skip to content

Commit 7ec74cb

Browse files
committed
Retrieve panic message when crashing
1 parent 8e56742 commit 7ec74cb

File tree

8 files changed

+127
-10
lines changed

8 files changed

+127
-10
lines changed

libdd-crashtracker/src/collector/api.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44

55
use super::{crash_handler::enable, receiver_manager::Receiver};
66
use crate::{
7-
clear_spans, clear_traces, collector::signal_handler_manager::register_crash_handlers,
8-
crash_info::Metadata, reset_counters, shared::configuration::CrashtrackerReceiverConfig,
9-
update_config, update_metadata, CrashtrackerConfiguration,
7+
clear_spans, clear_traces, collector::crash_handler::register_panic_hook,
8+
collector::signal_handler_manager::register_crash_handlers, crash_info::Metadata,
9+
reset_counters, shared::configuration::CrashtrackerReceiverConfig, update_config,
10+
update_metadata, CrashtrackerConfiguration,
1011
};
1112

1213
pub static DEFAULT_SYMBOLS: [libc::c_int; 4] =
@@ -43,6 +44,8 @@ pub fn on_fork(
4344
// The altstack (if any) is similarly unaffected by fork:
4445
// https://man7.org/linux/man-pages/man2/sigaltstack.2.html
4546

47+
// panic hook is unaffected by fork.
48+
4649
update_metadata(metadata)?;
4750
update_config(config)?;
4851
Receiver::update_stored_config(receiver_config)?;
@@ -68,6 +71,7 @@ pub fn init(
6871
update_config(config.clone())?;
6972
Receiver::update_stored_config(receiver_config)?;
7073
register_crash_handlers(&config)?;
74+
register_panic_hook()?;
7175
enable();
7276
Ok(())
7377
}

libdd-crashtracker/src/collector/collector_manager.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ impl Collector {
3030
config: &CrashtrackerConfiguration,
3131
config_str: &str,
3232
metadata_str: &str,
33+
message_ptr: *mut String,
3334
sig_info: *const siginfo_t,
3435
ucontext: *const ucontext_t,
3536
) -> Result<Self, CollectorSpawnError> {
@@ -45,6 +46,7 @@ impl Collector {
4546
config,
4647
config_str,
4748
metadata_str,
49+
message_ptr,
4850
sig_info,
4951
ucontext,
5052
receiver.handle.uds_fd,
@@ -66,10 +68,12 @@ impl Collector {
6668
}
6769
}
6870

71+
#[allow(clippy::too_many_arguments)]
6972
pub(crate) fn run_collector_child(
7073
config: &CrashtrackerConfiguration,
7174
config_str: &str,
7275
metadata_str: &str,
76+
message_ptr: *mut String,
7377
sig_info: *const siginfo_t,
7478
ucontext: *const ucontext_t,
7579
uds_fd: RawFd,
@@ -96,6 +100,7 @@ pub(crate) fn run_collector_child(
96100
config,
97101
config_str,
98102
metadata_str,
103+
message_ptr,
99104
sig_info,
100105
ucontext,
101106
ppid,

libdd-crashtracker/src/collector/crash_handler.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ use crate::crash_info::Metadata;
1010
use crate::shared::configuration::CrashtrackerConfiguration;
1111
use libc::{c_void, siginfo_t, ucontext_t};
1212
use libdd_common::timeout::TimeoutManager;
13+
use std::panic;
14+
use std::panic::PanicHookInfo;
1315
use std::ptr;
1416
use std::sync::atomic::Ordering::SeqCst;
1517
use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU64};
@@ -35,6 +37,10 @@ use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU64};
3537
// `Box::from_raw` to recreate the box, then dropping it.
3638
static METADATA: AtomicPtr<(Metadata, String)> = AtomicPtr::new(ptr::null_mut());
3739
static CONFIG: AtomicPtr<(CrashtrackerConfiguration, String)> = AtomicPtr::new(ptr::null_mut());
40+
static PANIC_MESSAGE: AtomicPtr<String> = AtomicPtr::new(ptr::null_mut());
41+
42+
type PanicHook = Box<dyn Fn(&PanicHookInfo<'_>) + Send + Sync>;
43+
static PREVIOUS_PANIC_HOOK: AtomicPtr<PanicHook> = AtomicPtr::new(ptr::null_mut());
3844

3945
#[derive(Debug, thiserror::Error)]
4046
pub enum CrashHandlerError {
@@ -72,6 +78,73 @@ pub fn update_metadata(metadata: Metadata) -> anyhow::Result<()> {
7278
Ok(())
7379
}
7480

81+
/// Register the panic hook.
82+
///
83+
/// This function is used to register the panic hook and store the previous hook.
84+
/// PRECONDITIONS:
85+
/// None
86+
/// SAFETY:
87+
/// Crash-tracking functions are not guaranteed to be reentrant.
88+
/// No other crash-handler functions should be called concurrently.
89+
/// ATOMICITY:
90+
/// This function uses a swap on an atomic pointer.
91+
pub fn register_panic_hook() -> anyhow::Result<()> {
92+
// register only once, if it is already registered, do nothing
93+
if !PREVIOUS_PANIC_HOOK.load(SeqCst).is_null() {
94+
return Ok(());
95+
}
96+
97+
let old_hook = panic::take_hook();
98+
let old_hook_ptr = Box::into_raw(Box::new(old_hook));
99+
PREVIOUS_PANIC_HOOK.swap(old_hook_ptr, SeqCst);
100+
panic::set_hook(Box::new(panic_hook));
101+
Ok(())
102+
}
103+
104+
/// The panic hook function.
105+
///
106+
/// This function is used to update the metadata with the panic message
107+
/// and call the previous hook.
108+
/// PRECONDITIONS:
109+
/// None
110+
/// SAFETY:
111+
/// Crash-tracking functions are not guaranteed to be reentrant.
112+
/// No other crash-handler functions should be called concurrently.
113+
/// ATOMICITY:
114+
/// This function uses a load on an atomic pointer.
115+
fn panic_hook(panic_info: &PanicHookInfo<'_>) {
116+
if let Some(s) = panic_info.payload().downcast_ref::<&str>() {
117+
let message_ptr = PANIC_MESSAGE.swap(Box::into_raw(Box::new(s.to_string())), SeqCst);
118+
// message_ptr should be null, but just in case.
119+
if !message_ptr.is_null() {
120+
unsafe {
121+
std::mem::drop(Box::from_raw(message_ptr));
122+
}
123+
}
124+
}
125+
call_previous_panic_hook(panic_info);
126+
}
127+
128+
/// Call the previous panic hook.
129+
///
130+
/// This function is used to call the previous panic hook.
131+
/// PRECONDITIONS:
132+
/// None
133+
/// SAFETY:
134+
/// Crash-tracking functions are not guaranteed to be reentrant.
135+
/// No other crash-handler functions should be called concurrently.
136+
fn call_previous_panic_hook(panic_info: &PanicHookInfo<'_>) {
137+
let old_hook_ptr = PREVIOUS_PANIC_HOOK.load(SeqCst);
138+
if !old_hook_ptr.is_null() {
139+
// Safety: This pointer can only come from Box::into_raw above in register_panic_hook.
140+
// We borrow it here without taking ownership so it remains valid for future calls.
141+
unsafe {
142+
let old_hook = &*old_hook_ptr;
143+
old_hook(panic_info);
144+
}
145+
}
146+
}
147+
75148
/// Updates the crashtracker config for this process
76149
/// Config is stored in a global variable and sent to the crashtracking
77150
/// receiver when a crash occurs.
@@ -172,6 +245,10 @@ fn handle_posix_signal_impl(
172245
}
173246
let (_metadata, metadata_string) = unsafe { &*metadata_ptr };
174247

248+
// Get the panic message pointer but don't dereference or deallocate in signal handler.
249+
// The collector child process will handle converting this to a String after forking.
250+
let message_ptr = PANIC_MESSAGE.swap(ptr::null_mut(), SeqCst);
251+
175252
let timeout_manager = TimeoutManager::new(config.timeout());
176253

177254
// Optionally, create the receiver. This all hinges on whether or not the configuration has a
@@ -190,6 +267,7 @@ fn handle_posix_signal_impl(
190267
config,
191268
config_str,
192269
metadata_string,
270+
message_ptr,
193271
sig_info,
194272
ucontext,
195273
)?;

libdd-crashtracker/src/collector/emitters.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,15 +132,18 @@ unsafe fn emit_backtrace_by_frames(
132132
Ok(())
133133
}
134134

135+
#[allow(clippy::too_many_arguments)]
135136
pub(crate) fn emit_crashreport(
136137
pipe: &mut impl Write,
137138
config: &CrashtrackerConfiguration,
138139
config_str: &str,
139140
metadata_string: &str,
141+
message_ptr: *mut String,
140142
sig_info: *const siginfo_t,
141143
ucontext: *const ucontext_t,
142144
ppid: i32,
143145
) -> Result<(), EmitterError> {
146+
emit_message(pipe, message_ptr)?;
144147
emit_metadata(pipe, metadata_string)?;
145148
emit_config(pipe, config_str)?;
146149
emit_siginfo(pipe, sig_info)?;
@@ -191,6 +194,17 @@ fn emit_metadata(w: &mut impl Write, metadata_str: &str) -> Result<(), EmitterEr
191194
Ok(())
192195
}
193196

197+
fn emit_message(w: &mut impl Write, message_ptr: *mut String) -> Result<(), EmitterError> {
198+
if !message_ptr.is_null() {
199+
let message = unsafe { &*message_ptr };
200+
writeln!(w, "{DD_CRASHTRACK_BEGIN_MESSAGE}")?;
201+
writeln!(w, "{message}")?;
202+
writeln!(w, "{DD_CRASHTRACK_END_MESSAGE}")?;
203+
w.flush()?;
204+
}
205+
Ok(())
206+
}
207+
194208
fn emit_procinfo(w: &mut impl Write, pid: i32) -> Result<(), EmitterError> {
195209
writeln!(w, "{DD_CRASHTRACK_BEGIN_PROCINFO}")?;
196210
writeln!(w, "{{\"pid\": {pid} }}")?;

libdd-crashtracker/src/crash_info/builder.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,10 @@ impl CrashInfoBuilder {
420420
self.metadata.is_some()
421421
}
422422
}
423+
424+
pub fn has_message(&self) -> bool {
425+
self.error.message.is_some()
426+
}
423427
}
424428

425429
#[cfg(test)]

libdd-crashtracker/src/crash_info/metadata.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ mod tests {
6565
tag!("runtime-id", "xyz"),
6666
tag!("language", "native"),
6767
],
68+
message: None,
6869
}
6970
}
7071
}

libdd-crashtracker/src/receiver/receive_report.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ pub(crate) enum StdinState {
7373
// may have lines that we need to accumulate depending on runtime (e.g. Python)
7474
RuntimeStackFrame(Vec<StackFrame>),
7575
RuntimeStackString(Vec<String>),
76+
Message,
7677
}
7778

7879
/// A state machine that processes data from the crash-tracker collector line by
@@ -183,20 +184,27 @@ fn process_line(
183184
StdinState::SigInfo if line.starts_with(DD_CRASHTRACK_END_SIGINFO) => StdinState::Waiting,
184185
StdinState::SigInfo => {
185186
let sig_info: SigInfo = serde_json::from_str(line)?;
186-
// By convention, siginfo is the first thing sent.
187-
let message = format!(
188-
"Process terminated with {:?} ({:?})",
189-
sig_info.si_code_human_readable, sig_info.si_signo_human_readable
190-
);
187+
if !builder.has_message() {
188+
let message = format!(
189+
"Process terminated with {:?} ({:?})",
190+
sig_info.si_code_human_readable, sig_info.si_signo_human_readable
191+
);
192+
builder.with_message(message)?;
193+
}
191194

192195
builder
193196
.with_timestamp_now()?
194197
.with_sig_info(sig_info)?
195-
.with_incomplete(true)?
196-
.with_message(message)?;
198+
.with_incomplete(true)?;
197199
StdinState::SigInfo
198200
}
199201

202+
StdinState::Message if line.starts_with(DD_CRASHTRACK_END_MESSAGE) => StdinState::Waiting,
203+
StdinState::Message => {
204+
builder.with_message(line.to_string())?;
205+
StdinState::Message
206+
}
207+
200208
StdinState::SpanIds if line.starts_with(DD_CRASHTRACK_END_SPAN_IDS) => StdinState::Waiting,
201209
StdinState::SpanIds => {
202210
let span_ids: Vec<Span> = serde_json::from_str(line)?;
@@ -246,6 +254,7 @@ fn process_line(
246254
StdinState::ProcInfo
247255
}
248256
StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_SIGINFO) => StdinState::SigInfo,
257+
StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_MESSAGE) => StdinState::Message,
249258
StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_SPAN_IDS) => {
250259
StdinState::SpanIds
251260
}

libdd-crashtracker/src/shared/constants.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ pub const DD_CRASHTRACK_BEGIN_SPAN_IDS: &str = "DD_CRASHTRACK_BEGIN_SPAN_IDS";
1717
pub const DD_CRASHTRACK_BEGIN_STACKTRACE: &str = "DD_CRASHTRACK_BEGIN_STACKTRACE";
1818
pub const DD_CRASHTRACK_BEGIN_TRACE_IDS: &str = "DD_CRASHTRACK_BEGIN_TRACE_IDS";
1919
pub const DD_CRASHTRACK_BEGIN_UCONTEXT: &str = "DD_CRASHTRACK_BEGIN_UCONTEXT";
20+
pub const DD_CRASHTRACK_BEGIN_MESSAGE: &str = "DD_CRASHTRACK_BEGIN_MESSAGE";
2021
pub const DD_CRASHTRACK_DONE: &str = "DD_CRASHTRACK_DONE";
2122
pub const DD_CRASHTRACK_END_ADDITIONAL_TAGS: &str = "DD_CRASHTRACK_END_ADDITIONAL_TAGS";
2223
pub const DD_CRASHTRACK_END_CONFIG: &str = "DD_CRASHTRACK_END_CONFIG";
@@ -31,5 +32,6 @@ pub const DD_CRASHTRACK_END_SPAN_IDS: &str = "DD_CRASHTRACK_END_SPAN_IDS";
3132
pub const DD_CRASHTRACK_END_STACKTRACE: &str = "DD_CRASHTRACK_END_STACKTRACE";
3233
pub const DD_CRASHTRACK_END_TRACE_IDS: &str = "DD_CRASHTRACK_END_TRACE_IDS";
3334
pub const DD_CRASHTRACK_END_UCONTEXT: &str = "DD_CRASHTRACK_END_UCONTEXT";
35+
pub const DD_CRASHTRACK_END_MESSAGE: &str = "DD_CRASHTRACK_END_MESSAGE";
3436

3537
pub const DD_CRASHTRACK_DEFAULT_TIMEOUT: Duration = Duration::from_millis(5_000);

0 commit comments

Comments
 (0)