diff --git a/Cargo.lock b/Cargo.lock index 59966e6180..8b8de48e36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5872,6 +5872,7 @@ dependencies = [ "thiserror 2.0.18", "tokio", "tracing", + "tracing-subscriber", "zeroize", ] diff --git a/packages/rs-sdk-ffi/Cargo.toml b/packages/rs-sdk-ffi/Cargo.toml index 7fa859017e..2502f18729 100644 --- a/packages/rs-sdk-ffi/Cargo.toml +++ b/packages/rs-sdk-ffi/Cargo.toml @@ -42,6 +42,7 @@ thiserror = "2.0" # Logging tracing = "0.1.41" +tracing-subscriber = { version = "0.3.22", features = ["env-filter", "fmt"] } # Encoding bs58 = "0.5" diff --git a/packages/rs-sdk-ffi/src/lib.rs b/packages/rs-sdk-ffi/src/lib.rs index 064f8d7aa5..81b7dc5ad5 100644 --- a/packages/rs-sdk-ffi/src/lib.rs +++ b/packages/rs-sdk-ffi/src/lib.rs @@ -169,12 +169,25 @@ pub extern "C" fn dash_sdk_init() { // Initialize any other subsystems if needed } -/// Enable logging with the specified level +/// Enable logging with the specified level. +/// +/// This function initializes a `tracing` subscriber with the given log level. +/// If the `RUST_LOG` environment variable is set, its directives take +/// precedence (useful for ad-hoc debugging); otherwise per-crate filter +/// directives derived from `level` are used. The env var is only *read*, +/// never written, so the call is safe from any thread context (including +/// after a Tokio runtime has started). +/// +/// The subscriber's built-in `tracing-log` bridge captures output from +/// crates that use the `log` facade, so a separate `env_logger::init()` +/// is not required. +/// +/// If a global subscriber has already been set (e.g., by a previous call), +/// subsequent calls are a no-op and the original level is retained. +/// /// Level values: 0 = Error, 1 = Warn, 2 = Info, 3 = Debug, 4 = Trace #[no_mangle] pub extern "C" fn dash_sdk_enable_logging(level: u8) { - use std::env; - let log_level = match level { 0 => "error", 1 => "warn", @@ -184,19 +197,30 @@ pub extern "C" fn dash_sdk_enable_logging(level: u8) { _ => "info", }; - // Set RUST_LOG environment variable for detailed logging - env::set_var( - "RUST_LOG", - format!( - "dash_sdk={},rs_sdk={},dapi_grpc={},h2={},tower={},hyper={},tonic={}", - log_level, log_level, log_level, log_level, log_level, log_level, log_level - ), + // Build the filter string with per-crate directives -- identical to what + // was previously stored in RUST_LOG, but constructed in-process so there + // is no data-race with concurrent `env::var` reads on other threads. + let filter_string = format!( + "dash_sdk={log_level},rs_sdk={log_level},rs_sdk_ffi={log_level},\ + dapi_grpc={log_level},h2={log_level},tower={log_level},\ + hyper={log_level},tonic={log_level}" ); - // Note: env_logger initialization is done in SDK creation - // We just set the environment variable here + // Honour RUST_LOG when present (read-only, no data-race); fall back + // to the programmatic filter string otherwise. + let filter = tracing_subscriber::EnvFilter::try_from_default_env() + .unwrap_or_else(|_| tracing_subscriber::EnvFilter::new(filter_string)); - tracing::info!(level = log_level, "logging enabled"); + // Initialize the global tracing subscriber. `try_init` returns Err if a + // subscriber is already installed; we intentionally ignore that so that + // calling this function more than once is harmless. + if tracing_subscriber::fmt::fmt() + .with_env_filter(filter) + .try_init() + .is_ok() + { + tracing::info!(level = log_level, "logging enabled"); + } } /// Get the version of the Dash SDK FFI library @@ -205,3 +229,50 @@ pub extern "C" fn dash_sdk_version() -> *const std::os::raw::c_char { static VERSION: &str = concat!(env!("CARGO_PKG_VERSION"), "\0"); VERSION.as_ptr() as *const std::os::raw::c_char } + +#[cfg(test)] +mod tests { + use super::*; + + /// Verify that `dash_sdk_enable_logging` does NOT set the RUST_LOG + /// environment variable. This is the core property that makes the + /// function safe to call from a multi-threaded context. + #[test] + fn enable_logging_does_not_set_env_var() { + // If RUST_LOG is already set by the test harness or environment, + // we cannot reliably detect whether our function sets it, so skip. + if std::env::var_os("RUST_LOG").is_some() { + return; + } + + // Call the function under test with each supported level. + for level in 0..=4 { + dash_sdk_enable_logging(level); + } + + // The function must NOT have set RUST_LOG. + assert!( + std::env::var("RUST_LOG").is_err(), + "RUST_LOG should not be set by dash_sdk_enable_logging; \ + env::set_var must not be used because it is a data race \ + in multi-threaded programs" + ); + } + + /// Verify that the function can be called from multiple threads + /// concurrently without panicking (i.e., no data race). + #[test] + fn enable_logging_is_thread_safe() { + let handles: Vec<_> = (0..4) + .map(|i| { + std::thread::spawn(move || { + dash_sdk_enable_logging(i % 5); + }) + }) + .collect(); + + for h in handles { + h.join().expect("thread should not panic"); + } + } +}