Add missing "in-preloading" check to RSHUTDOWN (#3037)

Author: realFlowControl

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| {

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--
 <?php
 if (PHP_VERSION_ID < 70400)
-    echo "skip: need preloading and therefore PHP", PHP_EOL;
+    echo "skip: need preloading and therefore PHP >= 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