fix: thread shutdown leaks

Signed-off-by: Alexandre Rulleau <[email protected]>

Author: Leiyks

appsec/third_party/libddwaf

@@ -95,6 +95,8 @@ ddog_MaybeError ddog_sidecar_connect_worker(int32_t master_pid,
 
 ddog_MaybeError ddog_sidecar_shutdown_master_listener(void);
 
+ddog_MaybeError ddog_sidecar_clear_inherited_listener(void);
+
 ddog_MaybeError ddog_sidecar_ping(struct ddog_SidecarTransport **transport);
 
 ddog_MaybeError ddog_sidecar_flush_traces(struct ddog_SidecarTransport **transport);

components-rs/sidecar.h

@@ -1,2 +1,2 @@
-BOOKWORM_CURRENT_VERSION=6
-BOOKWORM_NEXT_VERSION=7
+BOOKWORM_CURRENT_VERSION=5
+BOOKWORM_NEXT_VERSION=6

dockerfiles/ci/bookworm/.env

@@ -5,3 +5,5 @@ leak:_dl_map_object_deps
 leak:__res_context_send
 leak:_dl_make_tlsdesc_dynamic
 leak:_dl_catch_exception
+leak:__cxa_thread_atexit
+leak:CURRENT_PARKER

dockerfiles/ci/bookworm/php-8.3/suppr.txt

@@ -1 +1,3 @@
 leak:timer_create
+leak:__cxa_thread_atexit
+leak:CURRENT_PARKER

dockerfiles/ci/bookworm/php-8.4/suppr.txt

@@ -1 +1,3 @@
 leak:timer_create
+leak:__cxa_thread_atexit
+leak:CURRENT_PARKER

dockerfiles/ci/bookworm/php-8.5/suppr.txt

@@ -1605,7 +1605,11 @@ static PHP_MSHUTDOWN_FUNCTION(ddtrace) {
 
     ddtrace_user_req_shutdown();
 
-    ddtrace_sidecar_shutdown();
+    // Only shutdown sidecar in MSHUTDOWN for non-CLI SAPIs.
+    // CLI SAPI shuts down in RSHUTDOWN to allow thread joins before ASAN checks.
+    if (strcmp(sapi_module.name, "cli") != 0) {
+        ddtrace_sidecar_shutdown();
+    }
 
     ddtrace_live_debugger_mshutdown();
 
@@ -2643,6 +2647,9 @@ void dd_internal_handle_fork(void) {
 #ifndef _WIN32
     if (get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && ddtrace_sidecar) {
         if (is_child_process) {
+            // Clear inherited listener state - child doesn't own the master listener thread
+            ddtrace_ffi_try("Failed clearing inherited listener state", ddog_sidecar_clear_inherited_listener());
+
             ddtrace_force_new_instance_id();
 
             if (ddtrace_sidecar_connect_worker_after_fork()) {

ext/ddtrace.c

@@ -27,18 +27,92 @@ static zif_handler dd_pcntl_forkx_handler = NULL;
 #define JOIN_BGS_BEFORE_FORK 1
 #endif
 
+static bool dd_master_listener_was_active = false;
+
 static void dd_prefork() {
 #if JOIN_BGS_BEFORE_FORK
     if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) {
         ddtrace_coms_flush_shutdown_writer_synchronous();
     }
 #endif
+
+#ifndef _WIN32
+    // Check if master listener is active before fork
+    dd_master_listener_was_active = (ddtrace_master_pid != 0 && getpid() == ddtrace_master_pid);
+
+    if (dd_master_listener_was_active) {
+        // Shutdown master listener before fork to avoid Tokio runtime corruption.
+        // Tokio's async runtime and fork() are fundamentally incompatible - forking
+        // while Tokio threads are active causes state corruption in the parent process.
+        // See: https://github.com/tokio-rs/tokio/issues/4301
+
+        // First close parent's worker connection to prevent handler thread deadlock
+        if (ddtrace_sidecar) {
+            ddog_sidecar_transport_drop(ddtrace_sidecar);
+            ddtrace_sidecar = NULL;
+        }
+
+        // Then shutdown the master listener and its Tokio runtime
+        ddog_sidecar_shutdown_master_listener();
+    }
+#endif
 }
 
+static void dd_postfork_parent() {
+#ifndef _WIN32
+    // Restart master listener in parent if it was active before fork
+    if (dd_master_listener_was_active) {
+        // Reinitialize master listener with fresh Tokio runtime.
+        // This recreates the async runtime that was shut down before fork.
+
+        // First, restart the master listener thread
+        if (!ddtrace_ffi_try("Failed restarting sidecar master listener after fork",
+                             ddog_sidecar_connect_master((int32_t)ddtrace_master_pid))) {
+            LOG(WARN, "Failed to restart sidecar master listener after fork");
+            dd_master_listener_was_active = false;
+            return;
+        }
+
+        // Then reconnect to it as a worker
+        ddog_SidecarTransport *sidecar_transport = NULL;
+        if (!ddtrace_ffi_try("Failed reconnecting master to sidecar after fork",
+                             ddog_sidecar_connect_worker((int32_t)ddtrace_master_pid, &sidecar_transport))) {
+            LOG(WARN, "Failed to reconnect master process to sidecar after fork");
+            dd_master_listener_was_active = false;
+            return;
+        }
+
+        ddtrace_sidecar = sidecar_transport;
+        dd_master_listener_was_active = false;
+    }
+#endif
+}
+
+// Declare LSAN runtime interface functions
+#if defined(__SANITIZE_ADDRESS__) && !defined(_WIN32)
+void __lsan_disable(void);
+void __lsan_enable(void);
+#endif
+
 static void dd_handle_fork(zval *return_value) {
     if (Z_LVAL_P(return_value) == 0) {
+        // CHILD PROCESS
+        // Disable ASAN leak detection in child to avoid false positives from inherited
+        // Tokio TLS and runtime state that can't be properly cleaned up after fork
+#if defined(__SANITIZE_ADDRESS__) && !defined(_WIN32)
+        // The child inherits Tokio runtime thread-local storage from the parent's
+        // master listener and connection handlers. These TLS destructors reference
+        // threads that don't exist in the child, causing spurious leak reports.
+        // We disable leak detection in the child since the inherited memory will
+        // be reclaimed when the child process exits.
+        __lsan_disable();
+#endif
         dd_internal_handle_fork();
     } else {
+        // PARENT PROCESS
+        // Restart master listener if it was shut down before fork
+        dd_postfork_parent();
+
 #if JOIN_BGS_BEFORE_FORK
         if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) {
             ddtrace_coms_restart_writer();

ext/handlers_pcntl.c

@@ -9,6 +9,7 @@
 #include <components-rs/ddtrace.h>
 #include <components-rs/sidecar.h>
 #include <zend_string.h>
+#include <main/SAPI.h>
 #include "sidecar.h"
 #include "live_debugger.h"
 #include "telemetry.h"
@@ -153,7 +154,8 @@ static void dd_sidecar_on_reconnect(ddog_SidecarTransport *transport) {
 
 }
 
-static ddog_SidecarTransport *dd_sidecar_connection_factory(void);
+// Forward declaration (non-static for export to handlers_pcntl.c)
+ddog_SidecarTransport *dd_sidecar_connection_factory(void);
 
 static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) {
     // Should not happen, unless the agent url is malformed
@@ -672,6 +674,28 @@ void ddtrace_sidecar_activate(void) {
 
 void ddtrace_sidecar_rshutdown(void) {
     ddog_Vec_Tag_drop(DDTRACE_G(active_global_tags));
+
+    // For CLI SAPI, shut down the master listener thread here in RSHUTDOWN
+    // since the process will exit after this request. For other SAPIs,
+    // the master listener persists across requests and shuts down in MSHUTDOWN.
+    if (strcmp(sapi_module.name, "cli") == 0) {
+#ifndef _WIN32
+        ddtrace_pid_t current_pid = getpid();
+        if (ddtrace_master_pid != 0 && current_pid == ddtrace_master_pid) {
+            // CRITICAL: Close the worker connection BEFORE shutting down the master listener.
+            // The master process has its own worker connection to the master listener.
+            // If we don't close it first, the handler thread waiting on this connection
+            // will never exit, causing the listener thread join to timeout.
+            if (ddtrace_sidecar) {
+                ddog_sidecar_transport_drop(ddtrace_sidecar);
+                ddtrace_sidecar = NULL;
+            }
+
+            ddtrace_ffi_try("Failed shutting down master listener in RSHUTDOWN",
+                          ddog_sidecar_shutdown_master_listener());
+        }
+#endif
+    }
 }
 
 bool ddtrace_alter_test_session_token(zval *old_value, zval *new_value, zend_string *new_str) {

ext/sidecar.c

@@ -44,6 +44,8 @@ ddog_Endpoint *ddtrace_sidecar_agent_endpoint(void);
 void ddtrace_sidecar_submit_root_span_data_direct_defaults(ddog_SidecarTransport **transport, ddtrace_root_span_data *root);
 void ddtrace_sidecar_submit_root_span_data_direct(ddog_SidecarTransport **transport, ddtrace_root_span_data *root, zend_string *cfg_service, zend_string *cfg_env, zend_string *cfg_version);
 
+ddog_SidecarTransport *dd_sidecar_connection_factory(void);
+
 void ddtrace_sidecar_send_debugger_data(ddog_Vec_DebuggerPayload payloads);
 void ddtrace_sidecar_send_debugger_datum(ddog_DebuggerPayload *payload);