Profiling: fix SystemSettings initialization

profiling/src/allocation/allocation_ge84.rs

@@ -146,15 +146,9 @@ lazy_static! {
 }
 
 pub fn first_rinit_should_disable_due_to_jit() -> bool {
-    if *JIT_ENABLED
+    *JIT_ENABLED
         && zend::PHP_VERSION_ID >= 80400
         && (80400..80406).contains(&crate::RUNTIME_PHP_VERSION_ID.load(Relaxed))
-    {
-        error!("Memory allocation profiling will be disabled as long as JIT is active. To enable allocation profiling disable JIT or upgrade PHP to at least version 8.4.7. See https://github.com/DataDog/dd-trace-php/pull/3199");
-        true
-    } else {
-        false
-    }
 }
 
 /// This initializes the thread locale variable `ZEND_MM_STATE` with respect to the currently

profiling/src/allocation/allocation_le83.rs

@@ -7,7 +7,7 @@ use crate::{RefCellExt, PROFILER_NAME, REQUEST_LOCALS};
 use core::{cell::Cell, ptr};
 use lazy_static::lazy_static;
 use libc::{c_char, c_int, c_void, size_t};
-use log::{debug, error, trace, warn};
+use log::{debug, trace, warn};
 use std::sync::atomic::Ordering::Relaxed;
 
 #[cfg(feature = "debug_stats")]
@@ -132,19 +132,9 @@ pub fn alloc_prof_ginit() {
 }
 
 pub fn first_rinit_should_disable_due_to_jit() -> bool {
-    if NEEDS_RUN_TIME_CHECK_FOR_ENABLED_JIT
+    NEEDS_RUN_TIME_CHECK_FOR_ENABLED_JIT
         && alloc_prof_needs_disabled_for_jit(crate::RUNTIME_PHP_VERSION_ID.load(Relaxed))
         && *JIT_ENABLED
-    {
-        if zend::PHP_VERSION_ID >= 80400 {
-            error!("Memory allocation profiling will be disabled as long as JIT is active. To enable allocation profiling disable JIT or upgrade PHP to at least version 8.4.7. See https://github.com/DataDog/dd-trace-php/pull/3199");
-        } else {
-            error!("Memory allocation profiling will be disabled as long as JIT is active. To enable allocation profiling disable JIT or upgrade PHP to at least version 8.1.21 or 8.2.8. See https://github.com/DataDog/dd-trace-php/pull/2088");
-        }
-        true
-    } else {
-        false
-    }
 }
 
 pub fn alloc_prof_rinit() {

profiling/src/allocation/mod.rs

@@ -11,24 +11,28 @@ use crate::bindings::{self as zend};
 use crate::profiling::Profiler;
 use crate::{RefCellExt, REQUEST_LOCALS};
 use libc::size_t;
-use log::{debug, error, trace};
+use log::{debug, trace};
 use rand_distr::{Distribution, Poisson};
 use std::ffi::c_void;
+use std::num::{NonZero, NonZeroU32, NonZeroU64};
 use std::sync::atomic::{AtomicU64, Ordering};
 
+use crate::config::SystemSettings;
 #[cfg(not(php_zts))]
 use rand::rngs::StdRng;
 #[cfg(php_zts)]
 use rand::rngs::ThreadRng;
 #[cfg(not(php_zts))]
 use rand::SeedableRng;
 
-/// Default sampling interval in bytes (4MB)
-pub const DEFAULT_ALLOCATION_SAMPLING_INTERVAL: u64 = 1024 * 4096;
+/// Default sampling interval in bytes (4 MiB).
+// SAFETY: value is > 0.
+pub const DEFAULT_ALLOCATION_SAMPLING_INTERVAL: NonZeroU32 =
+    unsafe { NonZero::new_unchecked(1024 * 4096) };
 
-/// Sampling distance feed into poison sampling algo
+/// Sampling distance feed into poison sampling algo. This must be > 0.
 pub static ALLOCATION_PROFILING_INTERVAL: AtomicU64 =
-    AtomicU64::new(DEFAULT_ALLOCATION_SAMPLING_INTERVAL);
+    AtomicU64::new(DEFAULT_ALLOCATION_SAMPLING_INTERVAL.get() as u64);
 
 /// This will store the count of allocations (including reallocations) during
 /// a profiling period. This will overflow when doing more than u64::MAX
@@ -53,10 +57,9 @@ pub struct AllocationProfilingStats {
 }
 
 impl AllocationProfilingStats {
-    fn new() -> AllocationProfilingStats {
-        // Safety: this will only error if lambda <= 0
-        let poisson =
-            Poisson::new(ALLOCATION_PROFILING_INTERVAL.load(Ordering::Relaxed) as f64).unwrap();
+    fn new(sampling_distance: NonZeroU64) -> AllocationProfilingStats {
+        // SAFETY: this will only error if lambda <= 0, and it's NonZeroU64.
+        let poisson = unsafe { Poisson::new(sampling_distance.get() as f64).unwrap_unchecked() };
         let mut stats = AllocationProfilingStats {
             next_sample: 0,
             poisson,
@@ -105,30 +108,35 @@ pub fn alloc_prof_startup() {
     allocation_le83::alloc_prof_startup();
 }
 
-pub fn alloc_prof_first_rinit() {
-    let (allocation_enabled, sampling_distance) = REQUEST_LOCALS
-        .try_with_borrow(|locals| {
-            let settings = locals.system_settings();
-            (settings.profiling_allocation_enabled, settings.profiling_allocation_sampling_distance)
-        })
-        .unwrap_or_else(|err| {
-            error!("Allocation profiling first rinit failed because it failed to borrow the request locals. Please report this to Datadog: {err}");
-            (false, DEFAULT_ALLOCATION_SAMPLING_INTERVAL as u32)
-        });
+pub fn first_rinit(settings: &SystemSettings) {
+    if !settings.profiling_allocation_enabled {
+        return;
+    }
 
-    if !allocation_enabled {
+    let sampling_distance = settings.profiling_allocation_sampling_distance;
+    ALLOCATION_PROFILING_INTERVAL.store(sampling_distance.get() as u64, Ordering::Relaxed);
+
+    trace!("Memory allocation profiling initialized with a sampling distance of {sampling_distance} bytes.");
+}
+
+/// # Safety
+///
+/// Must be called exactly once per extension minit.
+pub unsafe fn minit(settings: &SystemSettings) {
+    if !settings.profiling_allocation_enabled {
         return;
     }
 
-    ALLOCATION_PROFILING_INTERVAL.store(sampling_distance as u64, Ordering::Relaxed);
+    let sampling_distance = settings.profiling_allocation_sampling_distance;
+    ALLOCATION_PROFILING_INTERVAL.store(sampling_distance.get() as u64, Ordering::Relaxed);
+
+    // SAFETY: called in minit.
+    unsafe { profiling_stats::minit(sampling_distance.into()) };
 
-    trace!(
-        "Memory allocation profiling initialized with a sampling distance of {} bytes.",
-        ALLOCATION_PROFILING_INTERVAL.load(Ordering::Relaxed)
-    );
+    trace!("Memory allocation profiling initialized with a sampling distance of {sampling_distance} bytes.");
 }
 
-pub fn alloc_prof_rinit() {
+pub fn rinit() {
     let allocation_enabled = REQUEST_LOCALS
         .try_with_borrow(|locals| locals.system_settings().profiling_allocation_enabled)
         .unwrap_or_else(|err| {

profiling/src/allocation/profiling_stats.rs

@@ -3,17 +3,18 @@
 //! performance sensitive. It is encapsulated so that some unsafe techniques
 //! can be used but expose a relatively safe API.
 
-use super::AllocationProfilingStats;
+use super::{AllocationProfilingStats, ALLOCATION_PROFILING_INTERVAL};
 use libc::size_t;
 use std::mem::MaybeUninit;
 
-#[cfg(php_zts)]
-use std::cell::UnsafeCell;
-
 #[cfg(php_zend_mm_set_custom_handlers_ex)]
 use super::allocation_ge84;
 #[cfg(not(php_zend_mm_set_custom_handlers_ex))]
 use super::allocation_le83;
+#[cfg(php_zts)]
+use std::cell::UnsafeCell;
+use std::num::NonZeroU64;
+use std::sync::atomic::Ordering;
 
 #[cfg(php_zts)]
 thread_local! {
@@ -107,12 +108,15 @@ pub fn allocation_profiling_stats_should_collect(len: size_t) -> bool {
 /// Must be called once per PHP thread ginit.
 pub unsafe fn ginit() {
     // SAFETY:
-    //  1. During ginit, there will not be any other borrows.
-    //  2. This closure will not make new borrows.
+    //  1. During ginit, there will not be any other borrows to stats.
+    //  2. This closure will not make new borrows to stats.
     //  3. This is not during the thread-local destructor.
     unsafe {
         allocation_profiling_stats_mut(|uninit| {
-            uninit.write(AllocationProfilingStats::new());
+            let interval = ALLOCATION_PROFILING_INTERVAL.load(Ordering::Relaxed);
+            // SAFETY: ALLOCATION_PROFILING_INTERVAL must always be > 0.
+            let nonzero = NonZeroU64::new_unchecked(interval);
+            uninit.write(AllocationProfilingStats::new(nonzero));
         })
     };
 
@@ -122,6 +126,27 @@ pub unsafe fn ginit() {
     allocation_ge84::alloc_prof_ginit();
 }
 
+/// Initializes the allocation profiler's globals with the provided sampling
+/// distance.
+///
+/// # Safety
+///
+/// Must be called once per PHP thread minit, unless the allocation profiling
+/// is disabled, in which case it can be skipped.
+pub unsafe fn minit(sampling_distance: NonZeroU64) {
+    // SAFETY:
+    //  1. During minit, there will not be any other borrows.
+    //  2. This closure will not make new borrows.
+    //  3. This is not during the thread-local destructor.
+    unsafe {
+        allocation_profiling_stats_mut(|uninit| {
+            // SAFETY: previously initialized in ginit, we're just
+            // re-initializing it because we now have config
+            *uninit.assume_init_mut() = AllocationProfilingStats::new(sampling_distance);
+        })
+    };
+}
+
 /// Shuts down the allocation profiler's globals.
 ///
 /// # Safety

profiling/src/config.rs

@@ -1,4 +1,3 @@
-use crate::allocation;
 use crate::bindings::zai_config_type::*;
 use crate::bindings::{
     datadog_php_profiling_copy_string_view_into_zval, ddog_php_prof_get_memoized_config,
@@ -7,16 +6,18 @@ use crate::bindings::{
     StringError, ZaiStr, IS_FALSE, IS_LONG, IS_TRUE, ZAI_CONFIG_NAME_BUFSIZ, ZEND_INI_DISPLAY_ORIG,
 };
 use crate::zend::zai_str_from_zstr;
+use crate::{allocation, bindings, zend};
 use core::fmt::{Display, Formatter};
 use core::mem::transmute;
 use core::ptr;
 use core::str::FromStr;
 use libc::{c_char, c_int};
 use libdd_common::tag::{parse_tags, Tag};
 pub use libdd_profiling::exporter::Uri;
-use log::{debug, warn, LevelFilter};
+use log::{debug, error, warn, LevelFilter};
 use std::borrow::Cow;
 use std::ffi::CString;
+use std::num::NonZeroU32;
 use std::path::{Path, PathBuf};
 
 #[derive(Copy, Clone, Debug, Default)]
@@ -43,7 +44,7 @@ pub struct SystemSettings {
     pub profiling_endpoint_collection_enabled: bool,
     pub profiling_experimental_cpu_time_enabled: bool,
     pub profiling_allocation_enabled: bool,
-    pub profiling_allocation_sampling_distance: u32,
+    pub profiling_allocation_sampling_distance: NonZeroU32,
     pub profiling_timeline_enabled: bool,
     pub profiling_exception_enabled: bool,
     pub profiling_exception_message_enabled: bool,
@@ -67,7 +68,7 @@ impl SystemSettings {
             profiling_endpoint_collection_enabled: false,
             profiling_experimental_cpu_time_enabled: false,
             profiling_allocation_enabled: false,
-            profiling_allocation_sampling_distance: u32::MAX,
+            profiling_allocation_sampling_distance: NonZeroU32::MAX,
             profiling_timeline_enabled: false,
             profiling_exception_enabled: false,
             profiling_exception_message_enabled: false,
@@ -82,8 +83,7 @@ impl SystemSettings {
 
     /// # Safety
     /// This function must only be called after ZAI config has been
-    /// initialized in first rinit, and before config is uninitialized in
-    /// shutdown.
+    /// initialized, and before config is uninitialized in shutdown.
     unsafe fn new() -> SystemSettings {
         // Select agent URI/UDS.
         let agent_host = agent_host();
@@ -130,22 +130,19 @@ impl SystemSettings {
     unsafe fn on_first_request() {
         let mut system_settings = SystemSettings::new();
 
-        // Initialize logging before allocation's rinit, as it logs.
-        cfg_if::cfg_if! {
-            if #[cfg(debug_assertions)] {
-                log::set_max_level(system_settings.profiling_log_level);
-            } else {
-                crate::logging::log_init(system_settings.profiling_log_level);
-            }
-        }
-
         // Work around version-specific issues.
         #[cfg(not(php_zend_mm_set_custom_handlers_ex))]
         if allocation::allocation_le83::first_rinit_should_disable_due_to_jit() {
+            if zend::PHP_VERSION_ID >= 80400 {
+                error!("Memory allocation profiling will be disabled as long as JIT is active. To enable allocation profiling disable JIT or upgrade PHP to at least version 8.4.7. See https://github.com/DataDog/dd-trace-php/pull/3199");
+            } else {
+                error!("Memory allocation profiling will be disabled as long as JIT is active. To enable allocation profiling disable JIT or upgrade PHP to at least version 8.1.21 or 8.2.8. See https://github.com/DataDog/dd-trace-php/pull/2088");
+            }
             system_settings.profiling_allocation_enabled = false;
         }
         #[cfg(php_zend_mm_set_custom_handlers_ex)]
         if allocation::allocation_ge84::first_rinit_should_disable_due_to_jit() {
+            error!("Memory allocation profiling will be disabled as long as JIT is active. To enable allocation profiling disable JIT or upgrade PHP to at least version 8.4.7. See https://github.com/DataDog/dd-trace-php/pull/3199");
             system_settings.profiling_allocation_enabled = false;
         }
 
@@ -475,7 +472,8 @@ static DEFAULT_SYSTEM_SETTINGS: SystemSettings = SystemSettings {
     profiling_endpoint_collection_enabled: true,
     profiling_experimental_cpu_time_enabled: true,
     profiling_allocation_enabled: true,
-    profiling_allocation_sampling_distance: 1024 * 4096,
+    // SAFETY: value is > 0.
+    profiling_allocation_sampling_distance: unsafe { NonZeroU32::new_unchecked(1024 * 4096) },
     profiling_timeline_enabled: true,
     profiling_exception_enabled: true,
     profiling_exception_message_enabled: false,
@@ -542,11 +540,16 @@ unsafe fn profiling_allocation_enabled() -> bool {
 /// # Safety
 /// This function must only be called after config has been initialized in
 /// rinit, and before it is uninitialized in mshutdown.
-unsafe fn profiling_allocation_sampling_distance() -> u32 {
-    get_system_uint32(
+unsafe fn profiling_allocation_sampling_distance() -> NonZeroU32 {
+    let int = get_system_uint32(
         ProfilingAllocationSamplingDistance,
-        DEFAULT_SYSTEM_SETTINGS.profiling_allocation_sampling_distance,
-    )
+        DEFAULT_SYSTEM_SETTINGS
+            .profiling_allocation_sampling_distance
+            .get(),
+    );
+    // SAFETY: ProfilingAllocationSamplingDistance uses parser that ensures a
+    // non-zero value.
+    unsafe { NonZeroU32::new_unchecked(int) }
 }
 
 /// # Safety
@@ -1233,8 +1236,31 @@ pub(crate) fn minit(module_number: libc::c_int) {
         );
         assert!(tmp); // It's literally return true in the source.
 
-        let system_settings = &mut *ptr::addr_of_mut!(SYSTEM_SETTINGS);
-        *system_settings = SystemSettings::initial();
+        // We set this so that we can access config for system INI settings during
+        // minit, for example for allocation_sampling_distance.
+        let in_request = false;
+        bindings::zai_config_first_time_rinit(in_request);
+
+        // SAFETY: just initialized zai config.
+        let mut system_settings = SystemSettings::new();
+
+        // Initialize logging before allocation's rinit, as it logs.
+        cfg_if::cfg_if! {
+            if #[cfg(debug_assertions)] {
+                log::set_max_level(system_settings.profiling_log_level);
+            } else {
+                crate::logging::log_init(system_settings.profiling_log_level);
+            }
+        }
+
+        SystemSettings::log_state(
+            (*ptr::addr_of!(SYSTEM_SETTINGS)).state,
+            system_settings.state,
+            "the module was initialized",
+        );
+        ptr::addr_of_mut!(SYSTEM_SETTINGS).swap(&mut system_settings);
+
+        allocation::minit(&*ptr::addr_of!(SYSTEM_SETTINGS))
     }
 }
 

profiling/src/lib.rs

@@ -692,7 +692,7 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult {
         #[cfg(all(feature = "io_profiling", target_os = "linux"))]
         io::io_prof_first_rinit();
 
-        allocation::alloc_prof_first_rinit();
+        allocation::first_rinit(system_settings);
     });
 
     Profiler::init(system_settings);
@@ -742,7 +742,7 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult {
         TAGS.set(Arc::default());
     }
 
-    allocation::alloc_prof_rinit();
+    allocation::rinit();
 
     // SAFETY: called after config is initialized.
     unsafe { timeline::timeline_rinit() };