fix(profiling) wall-time crash after PHP runs on a native thread in ext-grpc

.github/workflows/prof_asan.yml

@@ -62,7 +62,7 @@ jobs:
           export DDTRACE_PROFILING_TARGET="$(uname -m)-unknown-linux-gnu"
           export DDTRACE_PROFILING_CARGO_BUILD_FLAGS='-Zbuild-std=std,panic_abort'
           phpize
-          ./configure --disable-ddtrace-tracer --enable-ddtrace-profiling --disable-ddtrace-rust-debug
+          DDTRACE_PROFILING_FEATURES="debug_stats,test,tracing,tracing-subscriber,trigger_time_sample" ./configure --disable-ddtrace-tracer --enable-ddtrace-profiling --disable-ddtrace-rust-debug
           make -j"$(nproc)"
           cp -v modules/datadog-profiling.so "$(php-config --extension-dir)/datadog-profiling.so"
 
@@ -128,7 +128,7 @@ jobs:
           export RUSTC_LINKER=clang-21
           export RUSTFLAGS='-C link-arg=-fsanitize=undefined,local-bounds -C link-arg=-fno-sanitize-recover=all'
           phpize
-          ./configure --disable-ddtrace-tracer --enable-ddtrace-profiling
+          DDTRACE_PROFILING_FEATURES="debug_stats,test,tracing,tracing-subscriber,trigger_time_sample" ./configure --disable-ddtrace-tracer --enable-ddtrace-profiling
           make -j"$(nproc)"
           cp -v modules/datadog-profiling.so "$(php-config --extension-dir)/datadog-profiling.so"
 

.gitlab/generate-profiler.php

@@ -59,14 +59,14 @@
     - '# NTS'
     - '# Use if/then instead of `command -v switch-php && switch-php` — the && form exits 1 when switch-php is absent, which FF_ENABLE_BASH_EXIT_CODE_CHECK treats as a job failure'
     - if command -v switch-php > /dev/null 2>&1; then switch-php "${PHP_MAJOR_MINOR}"; fi
-    - (cd ..; phpize && DDTRACE_PROFILING_FEATURES="debug_stats,stack_walking_tests,test,tracing,tracing-subscriber,trigger_time_sample" ./configure --disable-ddtrace-tracer --enable-ddtrace-profiling && make -j$(nproc))
+    - (cd ..; phpize && DDTRACE_PROFILING_FEATURES="debug_stats,test,tracing,tracing-subscriber,trigger_time_sample" ./configure --disable-ddtrace-tracer --enable-ddtrace-profiling && make -j$(nproc))
     - (cd ../; TEST_PHP_JUNIT="${CI_PROJECT_DIR}/artifacts/profiler-tests/nts-results.xml" php profiling/tests/run-tests.php -d "extension=${CI_PROJECT_DIR}/modules/datadog-profiling.so" --show-diff -g "FAIL,XFAIL,BORK,WARN,LEAK,XLEAK,SKIP" "profiling/tests/phpt")
 
 
     - '# ZTS'
     - if command -v switch-php > /dev/null 2>&1; then switch-php "${PHP_MAJOR_MINOR}-zts"; fi
     - touch ../profiling/build.rs # force regeneration after switch-php changes the php-config symlink target
-    - (cd ..; make distclean || true; phpize && DDTRACE_PROFILING_FEATURES="debug_stats,stack_walking_tests,test,tracing,tracing-subscriber,trigger_time_sample" ./configure --disable-ddtrace-tracer --enable-ddtrace-profiling && make -j$(nproc))
+    - (cd ..; make distclean || true; phpize && DDTRACE_PROFILING_FEATURES="debug_stats,test,tracing,tracing-subscriber,trigger_time_sample" ./configure --disable-ddtrace-tracer --enable-ddtrace-profiling && make -j$(nproc))
     - (cd ../; TEST_PHP_JUNIT="${CI_PROJECT_DIR}/artifacts/profiler-tests/zts-results.xml" php profiling/tests/run-tests.php -d "extension=${CI_PROJECT_DIR}/modules/datadog-profiling.so" --show-diff -g "FAIL,XFAIL,BORK,WARN,LEAK,XLEAK,SKIP" "profiling/tests/phpt")
   after_script:
     - |

profiling/build.rs

@@ -575,11 +575,14 @@ fn apple_linker_flags() {
         "_zend_accel_schedule_restart_hook",
         "_zend_alter_ini_entry_ex",
         "_zend_ce_throwable",
+        "_zend_call_function",
+        "_zend_call_stack_init",
         "_zend_compile_file",
         "_zend_compile_string",
         "_zend_empty_string",
         "_zend_extensions",
         "_zend_flf_functions",
+        "_zend_fcall_info_init",
         "_zend_flf_handlers",
         "_zend_gc_get_status",
         "_zend_generator_check_placeholder_frame",
@@ -652,6 +655,8 @@ fn apple_linker_flags() {
         "_executor_globals_offset",
         "_sapi_globals_offset",
         // Zend parameter error (may appear with certain PHP versions/features)
+        "_zend_wrong_parameter_error",
+        "_zend_wrong_parameters_count_error",
         "_zend_wrong_parameters_none_error",
     ];
 

profiling/src/module_globals.rs

@@ -7,7 +7,9 @@ use core::sync::atomic::AtomicU32;
 
 #[cfg(target_os = "linux")]
 use crate::profiling::process_context::ProcessContextCache;
-#[cfg(target_os = "linux")]
+#[cfg(php_run_time_cache)]
+use crate::profiling::string_set::StringSet;
+#[cfg(any(target_os = "linux", php_run_time_cache))]
 use core::cell::RefCell;
 
 #[cfg(php_zend_mm_set_custom_handlers_ex)]
@@ -31,6 +33,9 @@ pub struct ProfilerGlobals {
     /// Per-thread allocation sampling state. Kept in PHP globals so allocator
     /// hooks can reuse an already-resolved TSRM cache instead of accessing Rust TLS.
     pub allocation_profiling_stats: UnsafeCell<MaybeUninit<allocation::AllocationProfilingStats>>,
+    /// String cache backing pointers stored in PHP runtime cache slots.
+    #[cfg(php_run_time_cache)]
+    pub cached_strings: UnsafeCell<MaybeUninit<RefCell<StringSet>>>,
 }
 
 /// We need TSRM to call into GINIT and GSHUTDOWN to observe spawning and
@@ -51,6 +56,8 @@ pub static mut GLOBALS: ProfilerGlobals = ProfilerGlobals {
     #[cfg(target_os = "linux")]
     process_context: RefCell::new(ProcessContextCache::new()),
     allocation_profiling_stats: UnsafeCell::new(MaybeUninit::uninit()),
+    #[cfg(php_run_time_cache)]
+    cached_strings: UnsafeCell::new(MaybeUninit::uninit()),
 };
 
 #[cfg(php_zts)]
@@ -129,11 +136,13 @@ pub unsafe extern "C" fn ginit(_globals_ptr: *mut c_void) {
     #[cfg(php_zts)]
     crate::profiling::timeline::timeline_ginit();
 
-    // Initialize PHP globals for ZTS builds. For NTS builds, this was already
-    // done in its const initializer.
+    #[cfg(any(php_zts, php_run_time_cache))]
+    let globals = _globals_ptr.cast::<ProfilerGlobals>();
+
+    // Initialize PHP globals for ZTS builds. For NTS builds, the const fields
+    // were already initialized above.
     #[cfg(php_zts)]
     {
-        let globals = _globals_ptr.cast::<ProfilerGlobals>();
         (*globals).zend_mm_state = Cell::new(ZendMMState::new());
         (*globals).interrupt_count = AtomicU32::new(0);
         #[cfg(target_os = "linux")]
@@ -142,6 +151,9 @@ pub unsafe extern "C" fn ginit(_globals_ptr: *mut c_void) {
         (*globals).allocation_profiling_stats = UnsafeCell::new(MaybeUninit::uninit());
     }
 
+    #[cfg(php_run_time_cache)]
+    (*(*globals).cached_strings.get()).write(RefCell::new(StringSet::new()));
+
     // SAFETY: this is called in thread ginit as expected, and no other places.
     allocation::ginit();
 }
@@ -155,9 +167,11 @@ pub unsafe extern "C" fn gshutdown(_globals_ptr: *mut c_void) {
     #[cfg(php_zts)]
     crate::profiling::timeline::timeline_gshutdown();
 
+    #[cfg(any(target_os = "linux", php_run_time_cache))]
+    let globals = _globals_ptr.cast::<ProfilerGlobals>();
+
     #[cfg(target_os = "linux")]
     {
-        let globals = _globals_ptr.cast::<ProfilerGlobals>();
         if let Ok(mut cache) = (*globals).process_context.try_borrow_mut() {
             cache.reset();
         }
@@ -167,6 +181,9 @@ pub unsafe extern "C" fn gshutdown(_globals_ptr: *mut c_void) {
 
     // SAFETY: this is called in thread gshutdown as expected, no other places.
     allocation::gshutdown();
+
+    #[cfg(php_run_time_cache)]
+    (*(*globals).cached_strings.get()).assume_init_drop();
 }
 
 // Unit tests are not loaded by PHP, so provide the PHP globals and TSRM symbol

profiling/src/php_ffi.c

@@ -165,7 +165,11 @@ static bool _ignore_run_time_cache = false;
 
 void datadog_php_profiling_startup(zend_extension *extension) {
 #if CFG_RUN_TIME_CACHE  // defined by build.rs
+#ifdef CFG_TEST
+    _ignore_run_time_cache = false;
+#else
     _ignore_run_time_cache = strcmp(sapi_module.name, "cli") == 0;
+#endif
 #endif
 
     datadog_php_profiling_get_profiling_context = noop_get_profiling_context;
@@ -746,6 +750,8 @@ static ZEND_FUNCTION(Datadog_Profiling_run_alloc_on_native_thread) {
 
     pthread_join(thread, NULL);
 
+    native_thread_alloc_func(NULL);
+
     RETURN_TRUE;
 }
 

profiling/src/profiler/stack_walking.rs

@@ -172,10 +172,13 @@ unsafe fn extract_file_and_line(
 #[cfg(php_run_time_cache)]
 mod detail {
     use super::*;
+    use crate::profiling::module_globals;
     use crate::profiling::string_set::StringSet;
-    use crate::profiling::{RefCellExt, RefCellExtError};
+    #[cfg(feature = "debug_stats")]
+    use crate::profiling::RefCellExt;
     use libdd_profiling::profiles::collections::ThinStr;
     use log::{debug, trace};
+    #[cfg(feature = "debug_stats")]
     use std::cell::RefCell;
     use std::ffi::c_void;
 
@@ -250,9 +253,8 @@ mod detail {
         }
     }
 
+    #[cfg(feature = "debug_stats")]
     thread_local! {
-        static CACHED_STRINGS: RefCell<StringSet> = RefCell::new(StringSet::new());
-        #[cfg(feature = "debug_stats")]
         static FUNCTION_CACHE_STATS: RefCell<FunctionRunTimeCacheStats> =
             const { RefCell::new(FunctionRunTimeCacheStats::new()) }
     }
@@ -274,7 +276,13 @@ mod detail {
             });
         }
 
-        let result = CACHED_STRINGS.try_with_borrow_mut(|string_set| {
+        // SAFETY: RSHUTDOWN runs after GINIT and before GSHUTDOWN on the
+        // current PHP thread.
+        let cached_strings = unsafe {
+            let globals = module_globals::get_profiler_globals();
+            (&*(*globals).cached_strings.get()).assume_init_ref()
+        };
+        let result = cached_strings.try_borrow_mut().map(|mut string_set| {
             // A slow ramp up to 2 MiB is probably _not_ going to look like a
             // memory leak. A higher threshold may make a user suspect a leak.
             const THRESHOLD: usize = 2 * 1024 * 1024;
@@ -293,7 +301,7 @@ mod detail {
 
         if let Err(err) = result {
             // Debug level because rshutdown could be quite spammy.
-            debug!("failed to borrow request locals in rshutdown: {err}");
+            debug!("failed to borrow string cache in rshutdown: {err}");
         }
     }
 
@@ -303,7 +311,7 @@ mod detail {
     /// Returns [`CollectStackSampleError::TryReserveError`] if the vec holding the frames is
     /// unable to allocate memory.
     #[inline]
-    fn collect_stack_sample_cached(
+    pub(super) fn collect_stack_sample_cached(
         top_execute_data: *mut zend_execute_data,
         string_set: &mut StringSet,
     ) -> Result<Backtrace, CollectStackSampleError> {
@@ -389,13 +397,14 @@ mod detail {
     ) -> Result<Backtrace, CollectStackSampleError> {
         #[cfg(feature = "tracing")]
         let _span = tracing::trace_span!("collect_stack_sample").entered();
-        CACHED_STRINGS
-            .try_with_borrow_mut(|set| collect_stack_sample_cached(execute_data, set))
-            .unwrap_or_else(|err| match err {
-                RefCellExtError::AccessError(e) => Err(e.into()),
-                RefCellExtError::BorrowError(e) => Err(e.into()),
-                RefCellExtError::BorrowMutError(e) => Err(e.into()),
-            })
+        // SAFETY: stack samples are collected after GINIT and before
+        // GSHUTDOWN on the current PHP thread.
+        let cached_strings = unsafe {
+            let globals = module_globals::get_profiler_globals();
+            (&*(*globals).cached_strings.get()).assume_init_ref()
+        };
+        let mut set = cached_strings.try_borrow_mut()?;
+        collect_stack_sample_cached(execute_data, &mut set)
     }
 
     unsafe fn collect_call_frame(
@@ -597,6 +606,12 @@ mod tests {
         unsafe {
             let fake_execute_data = zend::ddog_php_test_create_fake_zend_execute_data(3);
 
+            #[cfg(php_run_time_cache)]
+            let stack = {
+                let mut string_set = crate::profiling::string_set::StringSet::new();
+                detail::collect_stack_sample_cached(fake_execute_data, &mut string_set).unwrap()
+            };
+            #[cfg(not(php_run_time_cache))]
             let stack = collect_stack_sample(fake_execute_data).unwrap();
 
             assert_eq!(stack.len(), 3);

profiling/tests/phpt/native_thread_alloc_01.phpt

@@ -1,11 +1,7 @@
 --TEST--
 [profiling] allocation profiling should not crash when allocation happens on non-PHP thread (ext-grpc compatibility)
 --DESCRIPTION--
-This test simulates what ext-grpc does: it creates a native thread (not a PHP thread) and triggers memory allocation on it. Before the fix, this would crash because:
-1. ThreadRng uses thread-local storage internally
-2. ALLOCATION_PROFILING_STATS was thread-local
-Both of these are uninitialized for non-PHP threads since they never went through GINIT. After the fix, NTS builds use a global static instead of TLS.
-See https://github.com/DataDog/dd-trace-php/pull/3542 for the fix
+This test simulates what ext-grpc does: it creates a native thread (not a PHP thread) and triggers allocation profiling on it. The native thread fills PHP runtime cache slots with pointers owned by the profiler's string cache. After that thread exits, the main thread reuses those slots. The string cache must therefore follow PHP globals rather than the native thread's Rust TLS lifetime.
 --SKIPIF--
 <?php
 if (!extension_loaded('datadog-profiling'))