diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index 37ecddedb0..0d6f8670c6 100644 --- a/profiling/src/lib.rs +++ b/profiling/src/lib.rs @@ -490,6 +490,23 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult { locals.system_settings = system_settings; }); + // Preloading happens before zend_post_startup_cb is called for the first + // time. When preloading is enabled and a non-root user is used for + // php-fpm, there is fork that happens. In the past, having the profiler + // enabled at this time would cause php-fpm eventually hang once the + // Profiler's channels were full; this has been fixed. See: + // https://github.com/DataDog/dd-trace-php/issues/1919 + // + // There are a few ways to handle this preloading scenario with the fork, + // but the simplest is to not enable the profiler until the engine's + // startup is complete. This means the preloading will not be profiled, + // but this should be okay. + #[cfg(php_preload)] + if !unsafe { bindings::ddog_php_prof_is_post_startup() } { + debug!("zend_post_startup_cb hasn't happened yet; not enabling profiler."); + return ZendResult::Success; + } + // SAFETY: still safe to access in rinit after first_rinit. let system_settings = unsafe { system_settings.as_mut() }; @@ -530,23 +547,6 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult { exception::exception_profiling_first_rinit(); }); - // Preloading happens before zend_post_startup_cb is called for the first - // time. When preloading is enabled and a non-root user is used for - // php-fpm, there is fork that happens. In the past, having the profiler - // enabled at this time would cause php-fpm eventually hang once the - // Profiler's channels were full; this has been fixed. See: - // https://github.com/DataDog/dd-trace-php/issues/1919 - // - // There are a few ways to handle this preloading scenario with the fork, - // but the simplest is to not enable the profiler until the engine's - // startup is complete. This means the preloading will not be profiled, - // but this should be okay. - #[cfg(php_preload)] - if !unsafe { bindings::ddog_php_prof_is_post_startup() } { - debug!("zend_post_startup_cb hasn't happened yet; not enabling profiler."); - return ZendResult::Success; - } - Profiler::init(system_settings); if system_settings.profiling_enabled { @@ -630,6 +630,11 @@ extern "C" fn rshutdown(_type: c_int, _module_number: c_int) -> ZendResult { #[cfg(debug_assertions)] trace!("RSHUTDOWN({_type}, {_module_number})"); + #[cfg(php_preload)] + if !unsafe { bindings::ddog_php_prof_is_post_startup() } { + return ZendResult::Success; + } + profiling::stack_walking::rshutdown(); REQUEST_LOCALS.with(|cell| { diff --git a/profiling/tests/phpt/preload_01.phpt b/profiling/tests/phpt/preload_01.phpt index 17c4629f2c..30710cf408 100644 --- a/profiling/tests/phpt/preload_01.phpt +++ b/profiling/tests/phpt/preload_01.phpt @@ -12,18 +12,18 @@ See: https://github.com/DataDog/dd-trace-php/issues/1919 Due to limitations of PHPT, this test does not test PHP-FPM or processes run with different permissions, but it makes sure, that profiling is started after preloading is done. +--ENV-- +USE_ZEND_ALLOC=0 --SKIPIF-- = 7.4.0", PHP_EOL; if (!extension_loaded('datadog-profiling')) echo "skip: test requires datadog-profiling", PHP_EOL; ?> --INI-- datadog.profiling.enabled=yes datadog.profiling.log_level=debug -datadog.profiling.allocation_enabled=no -datadog.profiling.experimental_cpu_time_enabled=no zend_extension=opcache opcache.enable_cli=1 opcache.preload={PWD}/preload_01_preload.php