diff --git a/crates/fbuild-cli/src/daemon_client.rs b/crates/fbuild-cli/src/daemon_client.rs index 22d0467d..50b89dfc 100644 --- a/crates/fbuild-cli/src/daemon_client.rs +++ b/crates/fbuild-cli/src/daemon_client.rs @@ -1032,6 +1032,11 @@ async fn ensure_direct_daemon_running() -> fbuild_core::Result<()> { DaemonProbe::Restarted => true, DaemonProbe::NotRunning => false, }; + if !restarted { + if let Some(error) = foreign_root_owner_error() { + return Err(fbuild_core::FbuildError::DaemonError(error)); + } + } let spawned_pid = start_daemon(&client).await?; if restarted { // FastLED/fbuild#1476: whichever branch `start_daemon` took (spawned, @@ -1042,6 +1047,40 @@ async fn ensure_direct_daemon_running() -> fbuild_core::Result<()> { Ok(()) } +/// A live daemon of another version or identity owns this cache root on a +/// port this CLI does not use. A daemon spawned now would only exit on +/// `root-owner.lock busy`, and three such spawns used to end in a misleading +/// "daemon did not become healthy" error while the owner was merely busy. +fn foreign_root_owner_error() -> Option { + let claim = fbuild_paths::daemon_ownership::read_owner_claim()?; + let own_port = fbuild_paths::get_daemon_port(); + let live = fbuild_core::platform::process::pid_is_alive(claim.pid) + && fbuild_core::platform::process::pid_exe_stem_matches( + claim.pid, + fbuild_paths::daemon_ownership::DAEMON_EXE_STEM, + ); + (live && claim.port != own_port).then(|| foreign_root_owner_message(&claim, own_port)) +} + +fn foreign_root_owner_message( + claim: &fbuild_paths::daemon_ownership::OwnerClaim, + own_port: u16, +) -> String { + format!( + "the fbuild cache root is owned by another fbuild-daemon (pid {}, version {}, mode {}, \ + port {}, exe {}), which this fbuild {} cannot share; it would use port {}. \ + No compilation was attempted. Wait for that daemon's work to finish and stop it \ + (`fbuild daemon stop`), or run the same fbuild build as the client that started it.", + claim.pid, + claim.version, + claim.mode, + claim.port, + claim.exe.display(), + env!("CARGO_PKG_VERSION"), + own_port + ) +} + enum DaemonProbe { /// A daemon is running and should be kept. Keep, diff --git a/crates/fbuild-cli/src/daemon_client/tests.rs b/crates/fbuild-cli/src/daemon_client/tests.rs index 2430bc59..1f39fcb7 100644 --- a/crates/fbuild-cli/src/daemon_client/tests.rs +++ b/crates/fbuild-cli/src/daemon_client/tests.rs @@ -3,8 +3,9 @@ use super::{ DaemonAcquisition, DaemonClient, DaemonInfoResponse, HealthResponseFull, - broker_refusal_is_fatal, daemon_cache_identity_error, launcher_path, - restart_diag::SiblingDaemon, same_running_image, should_restart_daemon, wedged_daemon_note, + broker_refusal_is_fatal, daemon_cache_identity_error, foreign_root_owner_message, + launcher_path, restart_diag::SiblingDaemon, same_running_image, should_restart_daemon, + wedged_daemon_note, }; use running_process::broker::client::RefusalKind::{VersionBlocked, VersionUnsupported}; @@ -222,3 +223,28 @@ fn a_live_unresponsive_daemon_is_named_along_with_its_recovery() { fn no_live_daemon_produces_no_hint() { assert_eq!(wedged_daemon_note(None), ""); } + +/// A busy daemon of another build that owns the cache root must be named, +/// not reported as an unhealthy daemon after three doomed spawns. +#[test] +fn foreign_root_owner_message_names_the_owner() { + let claim = fbuild_paths::daemon_ownership::OwnerClaim { + pid: 98258, + exe: fbuild_core::path::NormalizedPath::new("/venv/bin/fbuild-daemon"), + version: "2.5.44".to_string(), + mode: "prod".to_string(), + cache_root_key: "/c".to_string(), + port: 60903, + }; + let message = foreign_root_owner_message(&claim, 19142); + for needle in [ + "98258", + "2.5.44", + "60903", + "19142", + "/venv/bin/fbuild-daemon", + "No compilation", + ] { + assert!(message.contains(needle), "{needle} missing from: {message}"); + } +} diff --git a/crates/fbuild-paths/src/lib.rs b/crates/fbuild-paths/src/lib.rs index 4992ae19..4be3aaf2 100644 --- a/crates/fbuild-paths/src/lib.rs +++ b/crates/fbuild-paths/src/lib.rs @@ -13,6 +13,9 @@ pub mod executable_hash; pub mod long_paths; pub mod running_process; +#[cfg(test)] +mod owner_port_tests; + /// The project-local and home-local fbuild directory segment: `.fbuild`. /// /// This is the canonical spelling. Nothing outside `fbuild-paths` / @@ -386,7 +389,10 @@ fn read_port_from_file(path: &Path) -> Option { /// Priority: /// 1. `FBUILD_DAEMON_PORT` environment variable (if set and valid 1–65535) /// 2. Port file for this endpoint (if it exists and is valid) -/// 3. [`default_daemon_port`] — a deterministic per-(version, cache-identity) +/// 3. The port of the live daemon that owns this cache root (its +/// `root-owner.json` claim), when it is the same version, mode and cache +/// root +/// 4. [`default_daemon_port`] — a deterministic per-(version, cache-identity) /// port /// /// FastLED/fbuild#1009: the endpoint is keyed by version+identity (via @@ -417,10 +423,57 @@ pub fn get_daemon_port() -> u16 { // `recorded_daemon_owner_is_dead`. } - // Priority 3: deterministic per-endpoint default. + // Priority 3: the live daemon that owns this cache root, when it is the + // same version and identity. FastLED/fbuild#1009 keys the endpoint on + // version+identity, but two builds that both report the same version can + // still derive different ports (e.g. a 2.5.44 wheel built before the + // #1700 port-window change and a 2.5.44 dev build after it). The + // version-blind root-ownership lock lets only one of them run, so a + // client that ignored the owner would spawn daemons that exit on + // `root-owner.lock busy` and then report the busy owner as unhealthy. + if let Some(port) = daemon_ownership::read_owner_claim().and_then(|claim| { + let identity = crate::running_process::DaemonCacheIdentity::discover(); + adoptable_owner_port( + &claim, + env!("CARGO_PKG_VERSION"), + identity.mode, + &identity.cache_root_key, + owner_claim_is_live, + ) + }) { + return port; + } + + // Priority 4: deterministic per-endpoint default. default_daemon_port() } +/// The owner claim's port, if the claim describes a live daemon this client +/// may share: same version, mode and cache root. Pure apart from `is_live`. +fn adoptable_owner_port( + claim: &daemon_ownership::OwnerClaim, + version: &str, + mode: &str, + cache_root_key: &str, + is_live: impl FnOnce(&daemon_ownership::OwnerClaim) -> bool, +) -> Option { + (claim.port > 0 + && claim.version == version + && claim.mode == mode + && claim.cache_root_key == cache_root_key + && is_live(claim)) + .then_some(claim.port) +} + +/// Positive evidence that the claim's pid is a running `fbuild-daemon`. +fn owner_claim_is_live(claim: &daemon_ownership::OwnerClaim) -> bool { + fbuild_core::platform::process::pid_is_alive(claim.pid) + && fbuild_core::platform::process::pid_exe_stem_matches( + claim.pid, + daemon_ownership::DAEMON_EXE_STEM, + ) +} + /// Can we *prove* the daemon that wrote this endpoint's records is gone? /// /// Fails safe: anything short of positive evidence of death returns `false`, diff --git a/crates/fbuild-paths/src/owner_port_tests.rs b/crates/fbuild-paths/src/owner_port_tests.rs new file mode 100644 index 00000000..3f0bb97e --- /dev/null +++ b/crates/fbuild-paths/src/owner_port_tests.rs @@ -0,0 +1,51 @@ +//! Tests for `adoptable_owner_port` (split from lib.rs for the 1000-LOC gate). + +use super::{adoptable_owner_port, daemon_ownership}; + +fn claim(version: &str, mode: &str, cache: &str, port: u16) -> daemon_ownership::OwnerClaim { + daemon_ownership::OwnerClaim { + pid: std::process::id(), + exe: fbuild_core::path::NormalizedPath::new("/x/fbuild-daemon"), + version: version.to_string(), + mode: mode.to_string(), + cache_root_key: cache.to_string(), + port, + } +} + +/// Two same-version builds that derive different default ports must +/// still find the live daemon that owns their shared cache root. +#[test] +fn owner_claim_port_is_adopted_for_same_version_identity() { + let c = claim("2.5.44", "prod", "/c", 60903); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/c", |_| true), + Some(60903) + ); +} + +#[test] +fn owner_claim_port_is_not_adopted_across_identity_or_when_dead() { + let c = claim("2.5.44", "prod", "/c", 60903); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/c", |_| false), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.45", "prod", "/c", |_| true), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "dev", "/c", |_| true), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/d", |_| true), + None + ); + let zero = claim("2.5.44", "prod", "/c", 0); + assert_eq!( + adoptable_owner_port(&zero, "2.5.44", "prod", "/c", |_| true), + None + ); +}