diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index b772c459d..48c41aef7 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -29,6 +29,7 @@ workspace = true [features] default = [] +test-util = [] hotpath = ["hotpath/hotpath", "hotpath/tokio"] hotpath-alloc = ["hotpath", "hotpath/hotpath-alloc"] hotpath-cpu = ["hotpath", "hotpath/hotpath-cpu"] diff --git a/crates/common/src/globals.rs b/crates/common/src/globals.rs index 680ac8c2a..6cc2f173c 100644 --- a/crates/common/src/globals.rs +++ b/crates/common/src/globals.rs @@ -97,6 +97,18 @@ pub fn try_get_global_local_node_name() -> Option { .filter(|name| !name.is_empty()) } +/// Run a synchronous test callback while publishing a name under its writer lock. +/// Call from a blocking worker; returning or unwinding releases the writer. +#[cfg(any(test, feature = "test-util"))] +#[doc(hidden)] +pub fn with_local_node_name_write_lock_for_test(name: &str, work: impl FnOnce() -> T) -> T { + let mut writer = GLOBAL_LOCAL_NODE_NAME.blocking_write(); + *writer = name.to_string(); + let result = work(); + drop(writer); + result +} + #[cfg(test)] mod node_identity_tests { use super::*; @@ -104,15 +116,23 @@ mod node_identity_tests { #[tokio::test] async fn local_node_name_nonblocking_read_recovers_after_writer_release() { let previous = get_global_local_node_name().await; - let mut writer = GLOBAL_LOCAL_NODE_NAME.write().await; - *writer = "published-node".to_string(); - let unavailable = try_get_global_local_node_name(); - drop(writer); - let published = try_get_global_local_node_name(); + let (unavailable, published, panicked) = tokio::task::spawn_blocking(|| { + let unavailable = with_local_node_name_write_lock_for_test("published-node", try_get_global_local_node_name); + let published = try_get_global_local_node_name(); + let panicked = std::panic::catch_unwind(|| { + with_local_node_name_write_lock_for_test("unwound-node", || panic!("identity callback failed")); + }); + (unavailable, published, panicked) + }) + .await + .expect("identity writer worker must complete"); + let after_unwind = try_get_global_local_node_name(); set_global_local_node_name(&previous).await; assert!(unavailable.is_none(), "a held writer must make the nonblocking getter unavailable"); assert_eq!(published.as_deref(), Some("published-node")); + assert!(panicked.is_err(), "the callback panic must propagate after releasing the lock"); + assert_eq!(after_unwind.as_deref(), Some("unwound-node")); } } diff --git a/crates/obs/Cargo.toml b/crates/obs/Cargo.toml index 03f1a098b..5819894b8 100644 --- a/crates/obs/Cargo.toml +++ b/crates/obs/Cargo.toml @@ -173,3 +173,6 @@ rustls = { workspace = true, default-features = false, features = ["aws-lc-rs", rustls-pki-types.workspace = true tokio = { workspace = true, features = ["io-util", "net"] } tokio-rustls = { workspace = true, default-features = false, features = ["aws-lc-rs"] } + +[target.'cfg(all(target_os = "linux", target_arch = "x86_64"))'.dev-dependencies] +rustfs-common = { workspace = true, features = ["test-util"] } diff --git a/crates/obs/src/node_identity/linux_fault_tests.rs b/crates/obs/src/node_identity/linux_fault_tests.rs index 0f093c20c..c1d40a0f3 100644 --- a/crates/obs/src/node_identity/linux_fault_tests.rs +++ b/crates/obs/src/node_identity/linux_fault_tests.rs @@ -149,3 +149,41 @@ async fn missing_identity_recovers_after_publication_with_netlink_denied() { assert_eq!(fallback, "127.0.0.1"); assert_eq!(published, "node-b:9000"); } + +#[tokio::test] +async fn contended_identity_uses_denied_netlink_fallback_then_recovers() { + use std::sync::mpsc; + use std::time::Duration; + + let action = libc::SECCOMP_RET_ERRNO | u32::try_from(libc::EPERM).expect("EPERM fits seccomp errno data"); + let (unavailable, resolved_while_contended, reader_result, published) = with_faulted_worker("node-a:9000", action, || { + let (unavailable, resolved_while_contended, reader) = + rustfs_common::with_local_node_name_write_lock_for_test("node-b:9000", || { + let unavailable = rustfs_common::try_get_global_local_node_name().is_none(); + let (resolved_tx, resolved_rx) = mpsc::channel(); + let reader = std::thread::spawn(move || { + let address_missing = rustfs_utils::get_local_ip().is_none(); + let identity = current_local_node_identity(); + resolved_tx + .send((address_missing, identity)) + .expect("identity test must retain its result receiver"); + }); + + let resolved_while_contended = resolved_rx.recv_timeout(Duration::from_secs(30)); + (unavailable, resolved_while_contended, reader) + }); + // The helper releases the writer before joining even if the reader + // times out, so restoration and worker cleanup can still complete. + let reader_result = reader.join(); + (unavailable, resolved_while_contended, reader_result, current_local_node_identity()) + }) + .await; + + reader_result.expect("faulted identity reader should complete after writer release"); + assert!(unavailable, "the real nonblocking getter must observe writer contention"); + let (address_missing, fallback) = + resolved_while_contended.expect("identity resolution must finish while the name writer is still held"); + assert!(address_missing, "the child must inherit the worker's real netlink denial"); + assert_eq!(fallback, "127.0.0.1"); + assert_eq!(published, "node-b:9000"); +}