diff --git a/components-rs/datadog.h b/components-rs/datadog.h index bb08a554b1..35fa8b0781 100644 --- a/components-rs/datadog.h +++ b/components-rs/datadog.h @@ -187,6 +187,8 @@ void ddog_set_dynamic_instrumentation_enabled(struct ddog_RemoteConfigState *rem void ddog_rshutdown_remote_config(struct ddog_RemoteConfigState *remote_config); +void ddog_live_debugger_free_retired(struct ddog_RemoteConfigState *remote_config); + void ddog_shutdown_remote_config(struct ddog_RemoteConfigState*); void ddog_log_debugger_data(const struct ddog_Vec_DebuggerPayload *payloads); diff --git a/components-rs/remote_config.rs b/components-rs/remote_config.rs index 5b08de64a8..c893f83520 100644 --- a/components-rs/remote_config.rs +++ b/components-rs/remote_config.rs @@ -111,6 +111,15 @@ pub struct LiveDebuggerCallbacks { pub struct LiveDebuggerState { pub spans_map: HashMap, pub active: HashMap>, // Box<> for stable heap address! + // Parsed configs removed (or replaced) from `active` whose probe hooks may still be + // installed: the C side stores shallow `ddog_Probe` copies that borrow `CharSlice`s + // (probe id, capture config, shm limiter, ...) into these boxes, and `zai_hook_remove` + // can defer the actual teardown of those hooks past this point. Dropping the box here + // would free the borrowed strings while a deferred hook can still fire and read them + // (use-after-free). Keep them alive until the probe hooks are torn down, then free + // them via `ddog_live_debugger_free_retired` (called from ddtrace_live_debugger_rshutdown, + // after zai_hook_clean), which bounds growth to a single request's config churn. + pub retired: Vec>, pub config_id: String, pub allow_dfa: Option, pub deny_dfa: Option, @@ -392,10 +401,13 @@ pub extern "C" fn ddog_process_remote_configs(remote_config: &mut RemoteConfigSt RemoteConfigProduct::LiveDebugger => { let val = Box::new((data, MaybeShmLimiter::open(limiter_index))); let rc_ref: &mut RemoteConfigState = unsafe { mem::transmute(remote_config as *mut _) }; // sigh, borrow checker + let mut retired_old = None; let entry = remote_config.live_debugger.active.entry(value.config_id); let (parsed, limiter) = match entry { Entry::Occupied(mut e) => { - e.insert(val); + // Retire (don't drop) the replaced box: an orphaned hook may + // still borrow its strings until request shutdown. + retired_old = Some(e.insert(val)); let r = e.into_mut(); (&r.0, &r.1) } @@ -407,6 +419,9 @@ pub extern "C" fn ddog_process_remote_configs(remote_config: &mut RemoteConfigSt if let Some(debugger) = parsed.downcast::() { apply_config(rc_ref, debugger, limiter); } + if let Some(old) = retired_old { + rc_ref.live_debugger.retired.push(old); + } } RemoteConfigProduct::ApmTracing => { if let Some(config_data) = data.downcast::() { @@ -442,6 +457,9 @@ pub extern "C" fn ddog_process_remote_configs(remote_config: &mut RemoteConfigSt if let Some(debugger) = boxed.0.downcast::() { remove_config(remote_config, debugger); } + // Don't drop `boxed` here: a deferred hook teardown can still borrow + // its strings. Retire it until request shutdown instead. + remote_config.live_debugger.retired.push(boxed); } } RemoteConfigProduct::ApmTracing => { @@ -747,6 +765,19 @@ pub extern "C" fn ddog_rshutdown_remote_config(remote_config: &mut RemoteConfigS ]); } +/// Free the parsed configs retired during this request (see `LiveDebuggerState::retired`). +/// +/// MUST be called only after all live debugger probe hooks have been torn down for the +/// request (i.e. after `zai_hook_clean`), since those hooks hold C-side `def->probe` +/// shallow copies whose `CharSlice`s / limiter pointer borrow into these boxes. It is +/// invoked from `ddtrace_live_debugger_rshutdown()` right after the hook table is +/// destroyed — NOT from `ddog_rshutdown_remote_config`, which runs at the very start of +/// RSHUTDOWN, before the shutdown-time flush that can still fire (and tear down) probes. +#[no_mangle] +pub extern "C" fn ddog_live_debugger_free_retired(remote_config: &mut RemoteConfigState) { + remote_config.live_debugger.retired.clear(); +} + #[no_mangle] pub extern "C" fn ddog_shutdown_remote_config(_: Box) {} diff --git a/tracer/live_debugger.c b/tracer/live_debugger.c index 81a5109e4a..91b2566e06 100644 --- a/tracer/live_debugger.c +++ b/tracer/live_debugger.c @@ -1677,6 +1677,12 @@ void ddtrace_live_debugger_rinit(void) { void ddtrace_live_debugger_rshutdown(void) { zend_hash_destroy(&DDTRACE_G(active_live_debugger_hooks)); + // The probe hooks (and their def->probe shallow copies borrowing into retired + // parsed configs) are now fully torn down, so it is finally safe to free the + // configs retired during this request. + if (DATADOG_G(remote_config_state)) { + ddog_live_debugger_free_retired(DATADOG_G(remote_config_state)); + } } bool ddtrace_alter_dynamic_instrumentation_config(zval *old_value, zval *new_value, zend_string *new_str) {