diff --git a/foundations/Cargo.toml b/foundations/Cargo.toml index 22e44d73..f483a8c0 100644 --- a/foundations/Cargo.toml +++ b/foundations/Cargo.toml @@ -233,6 +233,13 @@ rustdoc-args = ["--cfg", "foundations_docsrs", "--cfg", "tokio_unstable", "--cfg # to rustc, or else dependencies will not be enabled, and the docs build will fail. rustc-args = ["--cfg", "tokio_unstable", "--cfg", "foundations_unstable"] +[[bin]] +name = "memory-profiler-test-helper" +path = "tests/helpers/memory_profiler_test_helper.rs" +required-features = ["memory-profiling"] +test = false +bench = false + [lints] workspace = true diff --git a/foundations/src/telemetry/memory_profiler.rs b/foundations/src/telemetry/memory_profiler.rs index 32cb12c3..32455b5b 100644 --- a/foundations/src/telemetry/memory_profiler.rs +++ b/foundations/src/telemetry/memory_profiler.rs @@ -65,8 +65,9 @@ impl MemoryProfiler { /// # Enabling profiling /// /// Note that profiling needs to be explicitly enabled by setting `_RJEM_MALLOC_CONF=prof:true` - /// environment variable for the binary and with [`MemoryProfilerSettings::enabled`] being set - /// to `true`. Otherwise, this method will return `None`. + /// environment variable for the binary. When profiling is available, this method applies the + /// configured sample interval and activation state. It returns `None` when profiling is + /// unavailable or [`MemoryProfilerSettings::enabled`] is `false`. /// /// If syscall sandboxing is being used (see [`crate::security`] for more details), telemetry /// must be initialized prior to syscall sandboxing. @@ -152,23 +153,31 @@ impl MemoryProfiler { } fn init_profiler(settings: &MemoryProfilerSettings) -> BootstrapResult> { - if !settings.enabled || !control::profiling_enabled() { + if !control::profiling_enabled() { return Ok(None); } - let (request_sender, request_receiver) = mpsc::channel(); - HEAP_PROFILE_REQUEST_SENDER - .set(AsyncMutex::new(request_sender)) - .map_err(|_| anyhow::anyhow!("request sender had already been initialized"))?; + // Disable profiling before resetting it so no samples are collected using stale settings. + control::write(control::PROF_ACTIVE, false) + .map_err(|e| BootstrapError::new(e).context("failed to deactivate profiling"))?; - std::thread::spawn(move || heap_profile_thread(request_receiver)); + control::write(control::PROF_RESET, usize::from(settings.sample_interval)) + .map_err(|e| BootstrapError::new(e).context("failed to set sample interval"))?; + + if !settings.enabled { + return Ok(None); + } control::write(control::BACKGROUND_THREAD, true).map_err(|e| { BootstrapError::new(e).context("failed to activate background thread collection") })?; - control::write(control::PROF_RESET, settings.sample_interval as u64) - .map_err(|e| BootstrapError::new(e).context("failed to set sample interval"))?; + let (request_sender, request_receiver) = mpsc::channel(); + HEAP_PROFILE_REQUEST_SENDER + .set(AsyncMutex::new(request_sender)) + .map_err(|_| anyhow::anyhow!("request sender had already been initialized"))?; + + std::thread::spawn(move || heap_profile_thread(request_receiver)); control::write(control::PROF_ACTIVE, true) .map_err(|e| BootstrapError::new(e).context("failed to activate profiling"))?; diff --git a/foundations/src/telemetry/mod.rs b/foundations/src/telemetry/mod.rs index a30bf6b5..cde3e1ce 100644 --- a/foundations/src/telemetry/mod.rs +++ b/foundations/src/telemetry/mod.rs @@ -343,6 +343,12 @@ pub fn init(config: TelemetryConfig) -> BootstrapResult { ) } + // Eagerly init the memory profiler so it gets set up before syscalls are sandboxed with seccomp. + // Apply its settings even when memory_profiler.enabled or server.enabled is false + // to ensure profiling is disabled + #[cfg(all(target_os = "linux", feature = "memory-profiling"))] + let _ = MemoryProfiler::get_or_init_with(&config.settings.memory_profiler)?; + #[cfg(feature = "metrics")] self::metrics::init::init(config.service_info, &config.settings.metrics)?; diff --git a/foundations/src/telemetry/server/mod.rs b/foundations/src/telemetry/server/mod.rs index 70ee7604..f2a5b5b1 100644 --- a/foundations/src/telemetry/server/mod.rs +++ b/foundations/src/telemetry/server/mod.rs @@ -151,13 +151,6 @@ impl TelemetryServerFuture { let settings = Arc::new(settings); - // Eagerly init the memory profiler so it gets set up before syscalls are sandboxed with seccomp. - #[cfg(all(target_os = "linux", feature = "memory-profiling"))] - if settings.memory_profiler.enabled { - memory_profiling::profiler(Arc::clone(&settings)) - .map_err(|err| anyhow::anyhow!(err))?; - } - let router = Router::new(custom_routes, Arc::clone(&settings)) .context("building telemetry server router")?; diff --git a/foundations/tests/helpers/memory_profiler_test_helper.rs b/foundations/tests/helpers/memory_profiler_test_helper.rs new file mode 100644 index 00000000..063c2816 --- /dev/null +++ b/foundations/tests/helpers/memory_profiler_test_helper.rs @@ -0,0 +1,47 @@ +// Run profiler initialization in a fresh process so each test controls jemalloc's startup state. +#[cfg(target_os = "linux")] +fn main() { + use foundations::telemetry::MemoryProfiler; + use foundations::telemetry::settings::MemoryProfilerSettings; + use std::ffi::CStr; + + const PROF_ACTIVE: &CStr = cstr(b"prof.active\0"); + const PROF_LG_SAMPLE: &CStr = cstr(b"prof.lg_sample\0"); + + let enabled = match std::env::args().nth(1).as_deref() { + Some("enabled") => true, + Some("disabled") => false, + _ => panic!("expected `enabled` or `disabled` argument"), + }; + + let profiler = MemoryProfiler::get_or_init_with(&MemoryProfilerSettings { + enabled, + sample_interval: 17, + }) + .expect("failed to initialize memory profiler"); + + let active: bool = read_jemalloc_setting(PROF_ACTIVE); + let sample_interval: usize = read_jemalloc_setting(PROF_LG_SAMPLE); + + println!( + "profiler={} active={active} sample_interval={sample_interval}", + profiler.is_some() + ); +} + +#[cfg(target_os = "linux")] +fn read_jemalloc_setting(name: &std::ffi::CStr) -> T { + unsafe { tikv_jemalloc_ctl::raw::read(name.to_bytes_with_nul()) } + .expect("failed to read jemalloc setting") +} + +#[cfg(target_os = "linux")] +const fn cstr(bytes: &[u8]) -> &std::ffi::CStr { + match std::ffi::CStr::from_bytes_until_nul(bytes) { + Ok(value) => value, + Err(_) => panic!("control name should be null-terminated"), + } +} + +#[cfg(not(target_os = "linux"))] +fn main() {} diff --git a/foundations/tests/memory_profiler.rs b/foundations/tests/memory_profiler.rs new file mode 100644 index 00000000..8bc72135 --- /dev/null +++ b/foundations/tests/memory_profiler.rs @@ -0,0 +1,43 @@ +#![cfg(all(target_os = "linux", feature = "memory-profiling"))] + +use std::process::Command; + +fn run_helper(malloc_conf: &str, enabled: bool) -> String { + let mut command = Command::new(env!("CARGO_BIN_EXE_memory-profiler-test-helper")); + command + .arg(if enabled { "enabled" } else { "disabled" }) + .env("_RJEM_MALLOC_CONF", malloc_conf); + + let output = command + .output() + .expect("failed to run memory profiler test helper"); + + assert!( + output.status.success(), + "memory profiler test helper failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + String::from_utf8(output.stdout) + .expect("helper output should be UTF-8") + .trim() + .to_owned() +} + +#[test] +fn disabled_settings_are_applied() { + // Disabled application settings must override an allocator configured as active at startup. + assert_eq!( + run_helper("prof:true,prof_active:true,lg_prof_sample:19", false), + "profiler=false active=false sample_interval=17" + ); +} + +#[test] +fn enabled_settings_activate_inactive_profiler() { + // Enabled application settings must activate the allocator and replace its sample interval. + assert_eq!( + run_helper("prof:true,prof_active:false,lg_prof_sample:19", true), + "profiler=true active=true sample_interval=17" + ); +}