refactor(profiling): simplify SystemSettings

morrisonlevi · morrisonlevi · commit 8afb20c7649a · 2026-01-14T22:28:23.000-07:00
diff --git a/profiling/src/config.rs b/profiling/src/config.rs
@@ -8,7 +8,7 @@ use crate::bindings::{
 };
 use crate::zend::zai_str_from_zstr;
 use core::fmt::{Display, Formatter};
-use core::mem::{swap, transmute, MaybeUninit};
+use core::mem::transmute;
 use core::ptr;
 use core::str::FromStr;
 use libc::{c_char, c_int};
@@ -41,16 +41,25 @@ pub struct SystemSettings {
 }
 
 impl SystemSettings {
-    pub fn disable_all(&mut self) {
-        self.profiling_enabled = false;
-        self.profiling_experimental_features_enabled = false;
-        self.profiling_endpoint_collection_enabled = false;
-        self.profiling_experimental_cpu_time_enabled = false;
-        self.profiling_allocation_enabled = false;
-        self.profiling_timeline_enabled = false;
-        self.profiling_exception_enabled = false;
-        self.profiling_exception_message_enabled = false;
-        self.profiling_io_enabled = false;
+    /// Provides "initial" settings, which are all "off"-like values.
+    pub const fn initial() -> SystemSettings {
+        SystemSettings {
+            profiling_enabled: false,
+            profiling_experimental_features_enabled: false,
+            profiling_endpoint_collection_enabled: false,
+            profiling_experimental_cpu_time_enabled: false,
+            profiling_allocation_enabled: false,
+            profiling_allocation_sampling_distance: u32::MAX,
+            profiling_timeline_enabled: false,
+            profiling_exception_enabled: false,
+            profiling_exception_message_enabled: false,
+            profiling_wall_time_enabled: false,
+            profiling_io_enabled: false,
+            output_pprof: None,
+            profiling_exception_sampling_distance: u32::MAX,
+            profiling_log_level: LevelFilter::Off,
+            uri: AgentEndpoint::Socket(Cow::Borrowed(AgentEndpoint::DEFAULT_UNIX_SOCKET_PATH)),
+        }
     }
 
     /// # Safety
@@ -83,15 +92,16 @@ impl SystemSettings {
     }
 }
 
-static mut SYSTEM_SETTINGS: MaybeUninit<SystemSettings> = MaybeUninit::uninit();
+static mut SYSTEM_SETTINGS: SystemSettings = SystemSettings::initial();
 
 impl SystemSettings {
-    /// # Safety
-    /// Must be called after [first_rinit] and before [shutdown].
-    pub unsafe fn get() -> ptr::NonNull<SystemSettings> {
-        // SAFETY: required by this function's own safety requirements.
-        let addr = unsafe { (*ptr::addr_of_mut!(SYSTEM_SETTINGS)).assume_init_mut() };
-        ptr::NonNull::from(addr)
+    /// Returns the "current" system settings, which are always memory-safe
+    /// but may point to "initial" values rather than the configured ones,
+    /// depending on what point in the lifecycle we're at.
+    pub const fn get() -> ptr::NonNull<SystemSettings> {
+        let addr = ptr::addr_of_mut!(SYSTEM_SETTINGS);
+        // SAFETY: it's derived from a static variable, it's not null.
+        unsafe { ptr::NonNull::new_unchecked(addr) }
     }
 
     /// # Safety
@@ -110,10 +120,6 @@ impl SystemSettings {
             }
         }
 
-        // Initialize the lazy lock holding the env var for new origin
-        // detection in a safe place.
-        _ = std::sync::LazyLock::force(&libdd_common::entity_id::DD_EXTERNAL_ENV);
-
         // 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() {
@@ -123,54 +129,21 @@ impl SystemSettings {
         if allocation::allocation_ge84::first_rinit_should_disable_due_to_jit() {
             system_settings.profiling_allocation_enabled = false;
         }
-        swap(
-            &mut system_settings,
-            (*ptr::addr_of_mut!(SYSTEM_SETTINGS)).assume_init_mut(),
-        );
-    }
 
-    /// # Safety
-    /// Must be called exactly once each startup in either minit or startup,
-    /// whether profiling is enabled or not.
-    unsafe fn on_startup() {
-        (*ptr::addr_of_mut!(SYSTEM_SETTINGS)).write(INITIAL_SYSTEM_SETTINGS.clone());
+        ptr::addr_of_mut!(SYSTEM_SETTINGS).swap(&mut system_settings);
     }
 
     /// # Safety
     /// Must be called exactly once per shutdown in either mshutdown or
     /// shutdown, before zai config is shutdown.
     unsafe fn on_shutdown() {
-        let system_settings = (*ptr::addr_of_mut!(SYSTEM_SETTINGS)).assume_init_mut();
-        *system_settings = SystemSettings {
-            profiling_enabled: false,
-            profiling_experimental_features_enabled: false,
-            profiling_endpoint_collection_enabled: false,
-            profiling_experimental_cpu_time_enabled: false,
-            profiling_allocation_enabled: false,
-            profiling_allocation_sampling_distance: 0,
-            profiling_timeline_enabled: false,
-            profiling_exception_enabled: false,
-            profiling_exception_message_enabled: false,
-            profiling_wall_time_enabled: false,
-            profiling_io_enabled: false,
-            output_pprof: None,
-            profiling_exception_sampling_distance: 0,
-            profiling_log_level: LevelFilter::Off,
-            uri: Default::default(),
-        };
+        let system_settings = &mut *ptr::addr_of_mut!(SYSTEM_SETTINGS);
+        *system_settings = SystemSettings::initial();
     }
 
     unsafe fn on_fork_in_child() {
-        let system_settings = (*ptr::addr_of_mut!(SYSTEM_SETTINGS)).assume_init_mut();
-        system_settings.profiling_enabled = false;
-        system_settings.profiling_experimental_features_enabled = false;
-        system_settings.profiling_endpoint_collection_enabled = false;
-        system_settings.profiling_experimental_cpu_time_enabled = false;
-        system_settings.profiling_allocation_enabled = false;
-        system_settings.profiling_timeline_enabled = false;
-        system_settings.profiling_exception_enabled = false;
-        system_settings.profiling_exception_message_enabled = false;
-        system_settings.profiling_io_enabled = false;
+        let system_settings = &mut *ptr::addr_of_mut!(SYSTEM_SETTINGS);
+        *system_settings = SystemSettings::initial();
     }
 }
 
@@ -448,55 +421,24 @@ impl ConfigId {
     }
 }
 
-lazy_static::lazy_static! {
-    /// In some SAPIs, full configuration is not known until the first request
-    /// is served. This is ripe for edge cases. Consider this order of events:
-    ///  1. Worker is created.
-    ///  2. No requests are served.
-    ///  3. As the worker shuts down, the timeline profiler will attempt to
-    ///     add idle time to timeline.
-    /// What state should the configuration be in?
-    ///
-    /// Since the real configuration was never learned, assume everything is
-    /// disabled, which should cause fewer issues for customers than assuming
-    /// defaults.
-    pub static ref INITIAL_SYSTEM_SETTINGS: SystemSettings = SystemSettings {
-        profiling_enabled: false,
-        profiling_experimental_features_enabled: false,
-        profiling_endpoint_collection_enabled: false,
-        profiling_experimental_cpu_time_enabled: false,
-        profiling_allocation_enabled: false,
-        profiling_allocation_sampling_distance: u32::MAX,
-        profiling_timeline_enabled: false,
-        profiling_exception_enabled: false,
-        profiling_exception_message_enabled: false,
-        profiling_wall_time_enabled: false,
-        profiling_io_enabled: false,
-        output_pprof: None,
-        profiling_exception_sampling_distance: u32::MAX,
-        profiling_log_level: LevelFilter::Off,
-        uri: Default::default(),
-    };
-
-    /// Keep these in sync with the INI defaults.
-    static ref DEFAULT_SYSTEM_SETTINGS: SystemSettings = SystemSettings {
-        profiling_enabled: true,
-        profiling_experimental_features_enabled: false,
-        profiling_endpoint_collection_enabled: true,
-        profiling_experimental_cpu_time_enabled: true,
-        profiling_allocation_enabled: true,
-        profiling_allocation_sampling_distance: 1024 * 4096,
-        profiling_timeline_enabled: true,
-        profiling_exception_enabled: true,
-        profiling_exception_message_enabled: false,
-        profiling_wall_time_enabled: true,
-        profiling_io_enabled: false,
-        output_pprof: None,
-        profiling_exception_sampling_distance: 100,
-        profiling_log_level: LevelFilter::Off,
-        uri: Default::default(),
-    };
-}
+/// Keep these in sync with the INI defaults.
+static DEFAULT_SYSTEM_SETTINGS: SystemSettings = SystemSettings {
+    profiling_enabled: true,
+    profiling_experimental_features_enabled: false,
+    profiling_endpoint_collection_enabled: true,
+    profiling_experimental_cpu_time_enabled: true,
+    profiling_allocation_enabled: true,
+    profiling_allocation_sampling_distance: 1024 * 4096,
+    profiling_timeline_enabled: true,
+    profiling_exception_enabled: true,
+    profiling_exception_message_enabled: false,
+    profiling_wall_time_enabled: true,
+    profiling_io_enabled: false,
+    output_pprof: None,
+    profiling_exception_sampling_distance: 100,
+    profiling_log_level: LevelFilter::Off,
+    uri: AgentEndpoint::Socket(Cow::Borrowed(AgentEndpoint::DEFAULT_UNIX_SOCKET_PATH)),
+};
 
 /// # Safety
 /// This function must only be called after config has been initialized in
@@ -1244,7 +1186,8 @@ pub(crate) fn minit(module_number: libc::c_int) {
         );
         assert!(tmp); // It's literally return true in the source.
 
-        SystemSettings::on_startup();
+        let system_settings = &mut *ptr::addr_of_mut!(SYSTEM_SETTINGS);
+        *system_settings = SystemSettings::initial();
     }
 }
 
@@ -1268,6 +1211,7 @@ pub(crate) unsafe fn on_fork_in_child() {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use core::mem::MaybeUninit;
     use libc::memcmp;
 
     #[test]
diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs
@@ -22,7 +22,7 @@ mod exception;
 mod timeline;
 mod vec_ext;
 
-use crate::config::{SystemSettings, INITIAL_SYSTEM_SETTINGS};
+use crate::config::SystemSettings;
 use crate::zend::datadog_sapi_globals_request_info;
 use bindings::{
     self as zend, ddog_php_prof_php_version, ddog_php_prof_php_version_id, ZendExtension,
@@ -323,6 +323,9 @@ extern "C" fn minit(_type: c_int, module_number: c_int) -> ZendResult {
         let _connector = libdd_common::connector::Connector::default();
     }
 
+    // Initialize the lazy lock holding the env var for new origin detection.
+    _ = std::sync::LazyLock::force(&libdd_common::entity_id::DD_EXTERNAL_ENV);
+
     // Use a hybrid extension hack to load as a module but have the
     // zend_extension hooks available:
     // https://www.phpinternalsbook.com/php7/extensions_design/zend_extensions.html#hybrid-extensions
@@ -423,7 +426,7 @@ pub struct RequestLocals {
     /// conditions such as in mshutdown when there were no requests served,
     /// then the settings are still memory safe, but they may not have the
     /// real configuration. Instead, they have a best-effort values such as
-    /// INITIAL_SYSTEM_SETTINGS, or possibly the values which were available
+    /// the initial settings, or possibly the values which were available
     /// in MINIT.
     pub system_settings: ptr::NonNull<SystemSettings>,
 
@@ -434,8 +437,9 @@ pub struct RequestLocals {
 impl RequestLocals {
     #[track_caller]
     pub fn system_settings(&self) -> &SystemSettings {
-        // SAFETY: it should always be valid, either set to the
-        // INITIAL_SYSTEM_SETTINGS or to the SYSTEM_SETTINGS.
+        // SAFETY: it should always be valid, just maybe "stale", such as
+        // having only the initial values, or only the ones available in minit,
+        // rather than the fully configured values.
         unsafe { self.system_settings.as_ref() }
     }
 }
@@ -449,7 +453,7 @@ impl Default for RequestLocals {
             git_commit_sha: None,
             git_repository_url: None,
             tags: vec![],
-            system_settings: ptr::NonNull::from(INITIAL_SYSTEM_SETTINGS.deref()),
+            system_settings: SystemSettings::get(),
             interrupt_count: AtomicU32::new(0),
             vm_interrupt_addr: ptr::null_mut(),
         }
@@ -572,8 +576,7 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult {
 
     unsafe { bindings::zai_config_rinit() };
 
-    // SAFETY: We are after first rinit and before config mshutdown.
-    let mut system_settings = unsafe { SystemSettings::get() };
+    let mut system_settings = SystemSettings::get();
 
     // initialize the thread local storage and cache some items
     let result = REQUEST_LOCALS.try_with_borrow_mut(|locals| {
