Skip to content

Commit 831dbb6

Browse files
ivoanjodanielsn
andauthored
[PROF-11745] Allow crashtracker to be enabled/disabled + reconfigured (#1066)
# What does this PR do? This PR adds the following new APIs to the crashtracker: * `ddog_crasht_enable` * `ddog_crasht_disable` * `ddog_crasht_reconfigure` # Motivation Prior to #1000, it was possible to: * reconfigure the crashtracker by calling init again * stop the crashtracker Both features were removed as it could cause issues with signal handlers and chaining. But, we still need to be able to update the configuration as for instance the following can change at run-time for Ruby and need to be updated: * Endpoint * Tags This PR restores some of the behavior removed in #1000, without the unsafety of touching signal handlers: * `enable`/`disable` just skip the crashtracker signal handling code, but never uninstall it * `reconfigure` does the same as `init` BUT without touching the signal handlers # Additional Notes This PR supercedes #1017 . # How to test the change? I've validated this branch with DataDog/dd-trace-rb#4577 on the Ruby side, and I'm able to get a green test suite for the Ruby crashtracker. --------- Co-authored-by: Daniel Schwartz-Narbonne <[email protected]>
1 parent 09474fc commit 831dbb6

File tree

5 files changed

+125
-6
lines changed

5 files changed

+125
-6
lines changed

datadog-crashtracker-ffi/src/collector/mod.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,40 @@ use ddcommon_ffi::{wrap_with_void_ffi_result, Slice, VoidResult};
1414
use function_name::named;
1515
pub use spans::*;
1616

17+
#[no_mangle]
18+
#[must_use]
19+
/// Disables the crashtracker.
20+
/// Note that this does not restore the old signal handlers, but rather turns crash-tracking into a
21+
/// no-op, and then chains the old handlers. This means that handlers registered after the
22+
/// crashtracker will continue to work as expected.
23+
///
24+
/// # Preconditions
25+
/// None
26+
/// # Safety
27+
/// None
28+
/// # Atomicity
29+
/// This function is atomic and idempotent. Calling it multiple times is allowed.
30+
pub unsafe extern "C" fn ddog_crasht_disable() -> VoidResult {
31+
datadog_crashtracker::disable();
32+
VoidResult::Ok(true)
33+
}
34+
35+
#[no_mangle]
36+
#[must_use]
37+
/// Enables the crashtracker, if it had been previously disabled.
38+
/// If crashtracking has not been initialized, this function will have no effect.
39+
///
40+
/// # Preconditions
41+
/// None
42+
/// # Safety
43+
/// None
44+
/// # Atomicity
45+
/// This function is atomic and idempotent. Calling it multiple times is allowed.
46+
pub unsafe extern "C" fn ddog_crasht_enable() -> VoidResult {
47+
datadog_crashtracker::enable();
48+
VoidResult::Ok(true)
49+
}
50+
1751
#[no_mangle]
1852
#[must_use]
1953
#[named]
@@ -75,6 +109,34 @@ pub unsafe extern "C" fn ddog_crasht_init(
75109
})
76110
}
77111

112+
#[no_mangle]
113+
#[must_use]
114+
#[named]
115+
/// Reconfigure the crashtracker and re-enables it.
116+
/// If the crashtracker has not been initialized, this function will have no effect.
117+
///
118+
/// # Preconditions
119+
/// None.
120+
/// # Safety
121+
/// Crash-tracking functions are not reentrant.
122+
/// No other crash-handler functions should be called concurrently.
123+
/// # Atomicity
124+
/// This function is not atomic. A crash during its execution may lead to
125+
/// unexpected crash-handling behaviour.
126+
pub unsafe extern "C" fn ddog_crasht_reconfigure(
127+
config: Config,
128+
receiver_config: ReceiverConfig,
129+
metadata: Metadata,
130+
) -> VoidResult {
131+
wrap_with_void_ffi_result!({
132+
datadog_crashtracker::reconfigure(
133+
config.try_into()?,
134+
receiver_config.try_into()?,
135+
metadata.try_into()?,
136+
)?;
137+
})
138+
}
139+
78140
#[no_mangle]
79141
#[must_use]
80142
#[named]

datadog-crashtracker/src/collector/api.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33
#![cfg(unix)]
44

5-
use super::receiver_manager::Receiver;
5+
use super::{crash_handler::enable, receiver_manager::Receiver};
66
use crate::{
77
clear_spans, clear_traces, collector::signal_handler_manager::register_crash_handlers,
88
crash_info::Metadata, reset_counters, shared::configuration::CrashtrackerReceiverConfig,
@@ -68,6 +68,29 @@ pub fn init(
6868
update_config(config.clone())?;
6969
Receiver::update_stored_config(receiver_config);
7070
register_crash_handlers(&config)?;
71+
enable();
72+
Ok(())
73+
}
74+
75+
/// Reconfigure the crash-tracking infrastructure.
76+
///
77+
/// PRECONDITIONS:
78+
/// None.
79+
/// SAFETY:
80+
/// Crash-tracking functions are not reentrant.
81+
/// No other crash-handler functions should be called concurrently.
82+
/// ATOMICITY:
83+
/// This function is not atomic. A crash during its execution may lead to
84+
/// unexpected crash-handling behaviour.
85+
pub fn reconfigure(
86+
config: CrashtrackerConfiguration,
87+
receiver_config: CrashtrackerReceiverConfig,
88+
metadata: Metadata,
89+
) -> anyhow::Result<()> {
90+
update_metadata(metadata)?;
91+
update_config(config.clone())?;
92+
Receiver::update_stored_config(receiver_config);
93+
enable();
7194
Ok(())
7295
}
7396

datadog-crashtracker/src/collector/crash_handler.rs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use nix::sys::signal;
1515
use std::io::Write;
1616
use std::ptr;
1717
use std::sync::atomic::Ordering::SeqCst;
18-
use std::sync::atomic::{AtomicPtr, AtomicU64};
18+
use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU64};
1919
use std::time::Instant;
2020

2121
// Note that this file makes use the following async-signal safe functions in a signal handler.
@@ -100,10 +100,44 @@ pub(crate) extern "C" fn handle_posix_sigaction(
100100
unsafe { chain_signal_handler(signum, sig_info, ucontext) };
101101
}
102102

103+
static ENABLED: AtomicBool = AtomicBool::new(true);
104+
105+
/// Disables the crashtracker.
106+
/// Note that this does not restore the old signal handlers, but rather turns crash-tracking into a
107+
/// no-op, and then chains the old handlers. This means that handlers registered after the
108+
/// crashtracker will continue to work as expected.
109+
///
110+
/// # Preconditions
111+
/// None
112+
/// # Safety
113+
/// None
114+
/// # Atomicity
115+
/// This function is atomic and idempotent. Calling it multiple times is allowed.
116+
pub fn disable() {
117+
ENABLED.store(false, SeqCst);
118+
}
119+
120+
/// Enables the crashtracker, if had been previously disabled.
121+
/// If crashtracking has not been initialized, this function will have no effect.
122+
///
123+
/// # Preconditions
124+
/// None
125+
/// # Safety
126+
/// None
127+
/// # Atomicity
128+
/// This function is atomic and idempotent. Calling it multiple times is allowed.
129+
pub fn enable() {
130+
ENABLED.store(true, SeqCst);
131+
}
132+
103133
fn handle_posix_signal_impl(
104134
sig_info: *const siginfo_t,
105135
ucontext: *const ucontext_t,
106136
) -> anyhow::Result<()> {
137+
if !ENABLED.load(SeqCst) {
138+
return Ok(());
139+
}
140+
107141
// If this is a SIGSEGV signal, it could be called due to a stack overflow. In that case, since
108142
// this signal allocates to the stack and cannot guarantee it is running without SA_NODEFER, it
109143
// is possible that we will re-emit the signal. Contemporary unices handle this just fine (no

datadog-crashtracker/src/collector/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ pub use additional_tags::{
1818
};
1919
pub use api::*;
2020
pub use counters::{begin_op, end_op, reset_counters, OpTypes};
21-
pub use crash_handler::{update_config, update_metadata};
21+
pub use crash_handler::{disable, enable, update_config, update_metadata};
2222
pub use spans::{clear_spans, clear_traces, insert_span, insert_trace, remove_span, remove_trace};

datadog-crashtracker/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ mod shared;
6363
#[cfg(all(unix, feature = "collector"))]
6464
pub use collector::{
6565
begin_op, clear_additional_tags, clear_spans, clear_traces, consume_and_emit_additional_tags,
66-
default_signals, end_op, init, insert_additional_tag, insert_span, insert_trace, on_fork,
67-
remove_additional_tag, remove_span, remove_trace, reset_counters, update_config,
68-
update_metadata, OpTypes, DEFAULT_SYMBOLS,
66+
default_signals, disable, enable, end_op, init, insert_additional_tag, insert_span,
67+
insert_trace, on_fork, reconfigure, remove_additional_tag, remove_span, remove_trace,
68+
reset_counters, update_config, update_metadata, OpTypes, DEFAULT_SYMBOLS,
6969
};
7070

7171
#[cfg(all(windows, feature = "collector_windows"))]

0 commit comments

Comments
 (0)