diff --git a/.github/workflows/benchmark-build-comparison.yml b/.github/workflows/benchmark-build-comparison.yml index 6a573d0ac..f078af92c 100644 --- a/.github/workflows/benchmark-build-comparison.yml +++ b/.github/workflows/benchmark-build-comparison.yml @@ -6,7 +6,7 @@ on: branches: [main] paths: - 'bench/blink/**' - - 'bench/fastled-examples/src/build_comparison.rs' + - 'bench/fastled-examples/src/build_comparison*.rs' - 'bench/fastled-examples/Cargo.toml' - '.github/workflows/benchmark-build-comparison.yml' schedule: diff --git a/CLAUDE.md b/CLAUDE.md index 7249c8fa7..e5171efe3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -138,7 +138,7 @@ The current inventory is auto-published to a stable tracking issue every Monday ## Key Constraints -- **No file-based locks** — all locking through daemon's in-memory managers +- **No file-based locks, with one sanctioned exception** — almost all synchronization is through the daemon's in-memory managers. The narrow exception is fbuild-daemon startup/lifetime root-ownership and spawn-herd election (soldr-style, FastLED/fbuild#1159): a version-blind `root-owner.lock` held for the daemon's whole lifetime, plus a `spawn.lock` single-flight election for concurrent CLI spawns. zccache compile/object access keeps its own zccache-internal in-memory synchronization — these locks never gate cache reads/writes. Locks are OS-released on process death and must never be manually broken or deleted. - **Dev mode isolation** — `FBUILD_DEV_MODE=1` → `~/.fbuild/dev/`. The daemon endpoint is no longer a fixed port: it's derived per (backend version + cache identity) in the IANA dynamic range 49152–65535 (`fbuild_paths::default_daemon_port` / `daemon_endpoint_key`), so different-version checkouts get isolated daemons and can't serve each other wrong-version builds (FastLED/fbuild#1009). Override with `FBUILD_DAEMON_PORT`. - **HTTP API compatibility** — same endpoints and JSON schemas as the Python daemon - **Windows USB-CDC** — 30 retries, aggressive buffer drain, DTR/RTS toggling after flash diff --git a/Cargo.lock b/Cargo.lock index 0bd9e7a70..a0e679cc3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1343,6 +1343,7 @@ name = "fbuild-core" version = "2.5.3" dependencies = [ "async-trait", + "fs2", "libc", "prost", "reqwest", @@ -1554,6 +1555,9 @@ name = "fbuild-paths" version = "2.5.3" dependencies = [ "fbuild-core", + "serde", + "serde_json", + "tempfile", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 0399653ba..a4705932b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -111,6 +111,7 @@ bytes = "1" uuid = { version = "1", features = ["v4"] } base64 = "0.22" futures = "0.3" +fs2 = "0.4" tokio-tungstenite = "0.24" async-trait = "0.1" dashmap = "6" diff --git a/agents/docs/commands-reference.md b/agents/docs/commands-reference.md index edcda0b79..d859bf60c 100644 --- a/agents/docs/commands-reference.md +++ b/agents/docs/commands-reference.md @@ -12,7 +12,7 @@ help text). | Command | Use this when | See more | |---|---|---| | `fbuild build` | You want to compile firmware for the env specified by `-e ` and ``. The default path; cache via daemon. | `fbuild help build` | -| `fbuild clean sketch|all` | You want to remove one environment/profile's project outputs, optionally including its exact reusable framework-cache entries, without compiling or deploying. | `fbuild help clean`, FastLED/fbuild#1089 | +| `fbuild clean sketch|all|cache` | You want to remove one target's project outputs, optionally its reusable framework objects; `cache` additionally resets the active mode's global compiler-object cache while retaining installed packages/toolchains. | `fbuild help clean`, FastLED/fbuild#1089, FastLED/fbuild#1154 | | `fbuild deploy` | You want to build AND flash to a connected board. Pass `--monitor` to attach the monitor after flash. | `fbuild help deploy` | | `fbuild monitor` | You want to attach the serial monitor to an already-running board without re-flashing. | `fbuild help monitor` | | `fbuild reset` | You want to reset the device without re-flashing. | `fbuild help reset` | diff --git a/bench/blink/README.md b/bench/blink/README.md index dce332f2a..a0a40c39b 100644 --- a/bench/blink/README.md +++ b/bench/blink/README.md @@ -8,11 +8,17 @@ distribution is pinned independently: `arduino:avr@1.8.8` for Arduino CLI and Each tool is measured for three trials by default: -- **Cold** removes project outputs and matching compiled framework caches before - compiling. Installed toolchains/framework packages and global - download/compiler caches remain available. +- **Cold** removes every tool's project outputs, reusable framework objects, + and compiler-object caches before compiling. Arduino CLI and PlatformIO + download/HTTP caches are also cleared. Installed packages/toolchains and + fbuild package archives remain available. - **Warm** immediately repeats the same build without changing any input. +Cleanup runs before the timer for every cold trial. Arduino CLI runs +`arduino-cli cache clean` and removes its build directory; PlatformIO runs +`pio system prune --cache --force` and its project clean target; fbuild runs +`fbuild clean cache`. None of these operations uninstalls the pinned packages. + The Rust runner records every trial and publishes the median. Cold bars are drawn behind narrower warm overlays using the same GitHub-dark gray, blue, and red palette as the zccache and soldr benchmark graphics. diff --git a/bench/fastled-examples/src/README.md b/bench/fastled-examples/src/README.md index 20da48f10..b44992e2e 100644 --- a/bench/fastled-examples/src/README.md +++ b/bench/fastled-examples/src/README.md @@ -5,6 +5,8 @@ Sources for the repository's end-to-end benchmark binaries: - `main.rs` implements `bench-fastled-examples`. - `build_comparison.rs` implements the nightly Arduino CLI vs PlatformIO vs fbuild Blink build comparison and static-site renderer. +- `build_comparison_tests.rs` covers the comparison runner's cold-cache + sequencing, output metadata, and renderer behavior. See the parent [`README.md`](../README.md) for the FastLED harness and [`../../blink/README.md`](../../blink/README.md) for the whole-build benchmark. diff --git a/bench/fastled-examples/src/build_comparison.rs b/bench/fastled-examples/src/build_comparison.rs index a0b5f3725..afd3e5e62 100644 --- a/bench/fastled-examples/src/build_comparison.rs +++ b/bench/fastled-examples/src/build_comparison.rs @@ -37,13 +37,29 @@ struct Options { raw_base_url: String, } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] enum ToolKind { Arduino, PlatformIo, Fbuild, } +#[derive(Clone, Debug, PartialEq, Eq)] +enum ColdCleanupStep { + Command { + program: OsString, + args: Vec, + }, + RemoveDir(PathBuf), +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum MeasurementStep { + PrepareCold(usize), + ColdBuild(usize), + WarmBuild(usize), +} + #[derive(Clone, Copy, Debug)] struct ToolStyle { key: &'static str, @@ -212,42 +228,50 @@ fn measure_tool( )?; } - for trial in 1..=options.trials { - writeln!( - log, - "\n===== {} trial {trial}/{} =====", - kind.style().label, - options.trials - )?; - clean_tool( - kind, - options, - repo_root, - project_dir, - fbuild, - arduino_build_dir, - log, - )?; - let cold = timed_build( - kind, - options, - repo_root, - project_dir, - fbuild, - arduino_build_dir, - log, - )?; - let warm = timed_build( - kind, - options, - repo_root, - project_dir, - fbuild, - arduino_build_dir, - log, - )?; - cold_trials_ms.push(round_millis(cold)); - warm_trials_ms.push(round_millis(warm)); + for step in measurement_plan(options.trials) { + match step { + MeasurementStep::PrepareCold(trial) => { + writeln!( + log, + "\n===== {} trial {trial}/{} =====", + kind.style().label, + options.trials + )?; + prepare_cold( + kind, + options, + repo_root, + project_dir, + fbuild, + arduino_build_dir, + log, + )?; + } + MeasurementStep::ColdBuild(_) => { + let elapsed = timed_build( + kind, + options, + repo_root, + project_dir, + fbuild, + arduino_build_dir, + log, + )?; + cold_trials_ms.push(round_millis(elapsed)); + } + MeasurementStep::WarmBuild(_) => { + let elapsed = timed_build( + kind, + options, + repo_root, + project_dir, + fbuild, + arduino_build_dir, + log, + )?; + warm_trials_ms.push(round_millis(elapsed)); + } + } } let cold_ms = round_millis(median(&cold_trials_ms)); @@ -270,7 +294,19 @@ fn measure_tool( }) } -fn clean_tool( +fn measurement_plan(trials: usize) -> Vec { + (1..=trials) + .flat_map(|trial| { + [ + MeasurementStep::PrepareCold(trial), + MeasurementStep::ColdBuild(trial), + MeasurementStep::WarmBuild(trial), + ] + }) + .collect() +} + +fn prepare_cold( kind: ToolKind, options: &Options, repo_root: &Path, @@ -279,31 +315,72 @@ fn clean_tool( arduino_build_dir: &Path, log: &mut File, ) -> AppResult<()> { - match kind { - ToolKind::Arduino => remove_dir_within(repo_root, arduino_build_dir), - ToolKind::PlatformIo => { - let args = os_args(&[ - "run", - "--project-dir", - &project_dir.to_string_lossy(), - "--environment", - "uno", - "--target", - "clean", - ]); - run_logged(&options.platformio, &args, repo_root, log).map(|_| ()) + writeln!(log, "----- untimed cold-cache preparation -----")?; + for step in cold_cleanup_steps( + kind, + &options.arduino_cli, + &options.platformio, + project_dir, + fbuild, + arduino_build_dir, + ) { + match step { + ColdCleanupStep::Command { program, args } => { + run_logged(&program, &args, repo_root, log)?; + } + ColdCleanupStep::RemoveDir(path) => remove_dir_within(repo_root, &path)?, } - ToolKind::Fbuild => { - let args = os_args(&[ + } + writeln!(log, "----- timed cold build follows -----")?; + Ok(()) +} + +fn cold_cleanup_steps( + kind: ToolKind, + arduino_cli: &OsStr, + platformio: &OsStr, + project_dir: &Path, + fbuild: &Path, + arduino_build_dir: &Path, +) -> Vec { + let command = |program: &OsStr, args: Vec| ColdCleanupStep::Command { + program: program.to_os_string(), + args, + }; + match kind { + ToolKind::Arduino => vec![ + command(arduino_cli, os_args(&["cache", "clean"])), + ColdCleanupStep::RemoveDir(arduino_build_dir.to_path_buf()), + ], + ToolKind::PlatformIo => vec![ + command( + platformio, + os_args(&["system", "prune", "--cache", "--force"]), + ), + command( + platformio, + os_args(&[ + "run", + "--project-dir", + &project_dir.to_string_lossy(), + "--environment", + "uno", + "--target", + "clean", + ]), + ), + ], + ToolKind::Fbuild => vec![command( + fbuild.as_os_str(), + os_args(&[ "clean", - "all", + "cache", &project_dir.to_string_lossy(), "--environment", "uno", "--release", - ]); - run_logged(fbuild.as_os_str(), &args, repo_root, log).map(|_| ()) - } + ]), + )], } } @@ -497,7 +574,7 @@ fn latest_payload(metadata: &Metadata, results: &[ToolResult]) -> Value { }, "trials": metadata.trials, "statistic": "median", - "cold_definition": "project outputs and matching compiled framework caches removed; installed packages and global download/compiler caches retained", + "cold_definition": "project outputs, reusable framework objects, compiler-object caches, and Arduino/PlatformIO download/HTTP caches removed; installed packages/toolchains and fbuild package archives retained", "warm_definition": "immediate no-change rebuild after the cold build", }, "results": results, @@ -717,7 +794,7 @@ fn render_html(metadata: &Metadata, results: &[ToolResult]) -> String {

fbuild Blink build benchmark

Generated {generated_at} from {sha}. Median of {trials} trials on {os}/{arch}.

-

All three tools compile the same Arduino Uno bench/blink/blink.ino. Cold removes project outputs and matching compiled framework caches while retaining installed packages and global download/compiler caches. Warm is the immediate no-change rebuild. The narrower warm bar overlays the cold bar.

+

All three tools compile the same Arduino Uno bench/blink/blink.ino. Cold removes project outputs, reusable framework objects, compiler-object caches, and Arduino/PlatformIO download/HTTP caches while retaining installed packages/toolchains and fbuild package archives. Warm is the immediate no-change rebuild. The narrower warm bar overlays the cold bar.

Arduino CLI vs PlatformIO vs fbuild cold and warm Blink build timings
@@ -861,132 +938,5 @@ fn print_help() { } #[cfg(test)] -mod tests { - use super::*; - - fn sample_results() -> Vec { - vec![ - ToolResult { - tool: "arduino".into(), - display_name: "Arduino CLI".into(), - version: "arduino-cli 1.5.0".into(), - cold_ms: 1200.0, - warm_ms: 800.0, - speedup: 1.5, - cold_trials_ms: vec![1100.0, 1200.0, 1300.0], - warm_trials_ms: vec![750.0, 800.0, 850.0], - }, - ToolResult { - tool: "platformio".into(), - display_name: "PlatformIO".into(), - version: "PlatformIO Core 6.1.19".into(), - cold_ms: 900.0, - warm_ms: 300.0, - speedup: 3.0, - cold_trials_ms: vec![850.0, 900.0, 950.0], - warm_trials_ms: vec![280.0, 300.0, 320.0], - }, - ToolResult { - tool: "fbuild".into(), - display_name: "fbuild".into(), - version: "fbuild 0.1.0".into(), - cold_ms: 600.0, - warm_ms: 40.0, - speedup: 15.0, - cold_trials_ms: vec![580.0, 600.0, 620.0], - warm_trials_ms: vec![38.0, 40.0, 42.0], - }, - ] - } - - fn sample_metadata() -> Metadata { - Metadata { - generated_at: "2026-07-22T12:00:00Z".into(), - git_sha: "0123456789abcdef".into(), - repository: DEFAULT_REPOSITORY.into(), - run_url: "https://github.com/FastLED/fbuild/actions/runs/1".into(), - project: "bench/blink".into(), - trials: 3, - } - } - - #[test] - fn median_handles_odd_and_even_trial_counts() { - assert_eq!(median(&[9.0, 1.0, 5.0]), 5.0); - assert_eq!(median(&[9.0, 1.0, 7.0, 3.0]), 5.0); - } - - #[test] - fn remove_dir_within_guards_boundaries() { - let sandbox = tempfile::tempdir().unwrap(); - let root = sandbox.path().join("root"); - let nested = root.join("nested"); - let sibling = sandbox.path().join("sibling"); - fs::create_dir_all(&nested).unwrap(); - fs::create_dir_all(&sibling).unwrap(); - - assert!(remove_dir_within(&root, &root).is_err()); - assert!(remove_dir_within(&root, &sibling).is_err()); - assert!(root.is_dir()); - assert!(sibling.is_dir()); - - remove_dir_within(&root, &nested).unwrap(); - assert!(!nested.exists()); - } - - #[test] - fn svg_uses_reference_palette_and_warm_overlay() { - let svg = render_svg(&sample_metadata(), &sample_results()); - for color in [ - "#3b4046", "#8b949e", "#1f3a7a", "#79c0ff", "#5b1f1c", "#f85149", - ] { - assert!(svg.contains(color), "missing {color}"); - } - assert!(svg.contains("height=\"28\"")); - assert!(svg.contains("height=\"14\"")); - assert!(svg.contains("cold (back) + warm (front overlay)")); - } - - #[test] - fn outputs_include_agent_discovery_and_bounded_history() { - let temp = tempfile::tempdir().unwrap(); - let history = (0..HISTORY_MAX_LINES) - .map(|index| format!(r#"{{"old":{index}}}"#)) - .collect::>() - .join("\n") - + "\n"; - fs::write(temp.path().join("history.jsonl"), history).unwrap(); - write_outputs( - temp.path(), - &sample_metadata(), - &sample_results(), - DEFAULT_PAGES_URL, - DEFAULT_RAW_BASE_URL, - ) - .unwrap(); - - for file in [ - "manifest.json", - "latest.json", - "history.jsonl", - "benchmark.svg", - "index.html", - ".nojekyll", - ] { - assert!(temp.path().join(file).is_file(), "missing {file}"); - } - let manifest: Value = - serde_json::from_str(&fs::read_to_string(temp.path().join("manifest.json")).unwrap()) - .unwrap(); - assert_eq!(manifest["branch"], "benchmark-stats"); - assert_eq!( - manifest["artifacts"]["history"]["max_lines"], - HISTORY_MAX_LINES - ); - let history = fs::read_to_string(temp.path().join("history.jsonl")).unwrap(); - assert_eq!(history.lines().count(), HISTORY_MAX_LINES); - assert!(history.lines().last().unwrap().contains("0123456789abcdef")); - let html = fs::read_to_string(temp.path().join("index.html")).unwrap(); - assert!(html.contains("stable discovery index for agents")); - } -} +#[path = "build_comparison_tests.rs"] +mod tests; diff --git a/bench/fastled-examples/src/build_comparison_tests.rs b/bench/fastled-examples/src/build_comparison_tests.rs new file mode 100644 index 000000000..1ff08ffde --- /dev/null +++ b/bench/fastled-examples/src/build_comparison_tests.rs @@ -0,0 +1,230 @@ +use super::*; + +fn sample_results() -> Vec { + vec![ + ToolResult { + tool: "arduino".into(), + display_name: "Arduino CLI".into(), + version: "arduino-cli 1.5.0".into(), + cold_ms: 1200.0, + warm_ms: 800.0, + speedup: 1.5, + cold_trials_ms: vec![1100.0, 1200.0, 1300.0], + warm_trials_ms: vec![750.0, 800.0, 850.0], + }, + ToolResult { + tool: "platformio".into(), + display_name: "PlatformIO".into(), + version: "PlatformIO Core 6.1.19".into(), + cold_ms: 900.0, + warm_ms: 300.0, + speedup: 3.0, + cold_trials_ms: vec![850.0, 900.0, 950.0], + warm_trials_ms: vec![280.0, 300.0, 320.0], + }, + ToolResult { + tool: "fbuild".into(), + display_name: "fbuild".into(), + version: "fbuild 0.1.0".into(), + cold_ms: 600.0, + warm_ms: 40.0, + speedup: 15.0, + cold_trials_ms: vec![580.0, 600.0, 620.0], + warm_trials_ms: vec![38.0, 40.0, 42.0], + }, + ] +} + +fn sample_metadata() -> Metadata { + Metadata { + generated_at: "2026-07-22T12:00:00Z".into(), + git_sha: "0123456789abcdef".into(), + repository: DEFAULT_REPOSITORY.into(), + run_url: "https://github.com/FastLED/fbuild/actions/runs/1".into(), + project: "bench/blink".into(), + trials: 3, + } +} + +fn command(program: &str, args: &[&str]) -> ColdCleanupStep { + ColdCleanupStep::Command { + program: OsString::from(program), + args: os_args(args), + } +} + +#[test] +fn median_handles_odd_and_even_trial_counts() { + assert_eq!(median(&[9.0, 1.0, 5.0]), 5.0); + assert_eq!(median(&[9.0, 1.0, 7.0, 3.0]), 5.0); +} + +#[test] +fn remove_dir_within_guards_boundaries() { + let sandbox = tempfile::tempdir().unwrap(); + let root = sandbox.path().join("root"); + let nested = root.join("nested"); + let sibling = sandbox.path().join("sibling"); + fs::create_dir_all(&nested).unwrap(); + fs::create_dir_all(&sibling).unwrap(); + + assert!(remove_dir_within(&root, &root).is_err()); + assert!(remove_dir_within(&root, &sibling).is_err()); + assert!(root.is_dir()); + assert!(sibling.is_dir()); + + remove_dir_within(&root, &nested).unwrap(); + assert!(!nested.exists()); +} + +#[test] +fn every_trial_prepares_cold_once_and_never_prepares_warm() { + assert_eq!( + measurement_plan(3), + vec![ + MeasurementStep::PrepareCold(1), + MeasurementStep::ColdBuild(1), + MeasurementStep::WarmBuild(1), + MeasurementStep::PrepareCold(2), + MeasurementStep::ColdBuild(2), + MeasurementStep::WarmBuild(2), + MeasurementStep::PrepareCold(3), + MeasurementStep::ColdBuild(3), + MeasurementStep::WarmBuild(3), + ] + ); +} + +#[test] +fn each_tool_has_the_complete_cold_cleanup_sequence() { + let project = Path::new("bench/blink"); + let arduino_build = Path::new("benchmark-output/arduino-build"); + let fbuild = Path::new("target/release/fbuild"); + + assert_eq!( + cold_cleanup_steps( + ToolKind::Arduino, + OsStr::new("arduino-cli"), + OsStr::new("pio"), + project, + fbuild, + arduino_build, + ), + vec![ + command("arduino-cli", &["cache", "clean"]), + ColdCleanupStep::RemoveDir(arduino_build.to_path_buf()), + ] + ); + assert_eq!( + cold_cleanup_steps( + ToolKind::PlatformIo, + OsStr::new("arduino-cli"), + OsStr::new("pio"), + project, + fbuild, + arduino_build, + ), + vec![ + command("pio", &["system", "prune", "--cache", "--force"]), + command( + "pio", + &[ + "run", + "--project-dir", + "bench/blink", + "--environment", + "uno", + "--target", + "clean", + ], + ), + ] + ); + assert_eq!( + cold_cleanup_steps( + ToolKind::Fbuild, + OsStr::new("arduino-cli"), + OsStr::new("pio"), + project, + fbuild, + arduino_build, + ), + vec![command( + "target/release/fbuild", + &[ + "clean", + "cache", + "bench/blink", + "--environment", + "uno", + "--release", + ], + )] + ); +} + +#[test] +fn svg_uses_reference_palette_and_warm_overlay() { + let svg = render_svg(&sample_metadata(), &sample_results()); + for color in [ + "#3b4046", "#8b949e", "#1f3a7a", "#79c0ff", "#5b1f1c", "#f85149", + ] { + assert!(svg.contains(color), "missing {color}"); + } + assert!(svg.contains("height=\"28\"")); + assert!(svg.contains("height=\"14\"")); + assert!(svg.contains("cold (back) + warm (front overlay)")); +} + +#[test] +fn outputs_include_agent_discovery_and_bounded_history() { + let temp = tempfile::tempdir().unwrap(); + let history = (0..HISTORY_MAX_LINES) + .map(|index| format!(r#"{{"old":{index}}}"#)) + .collect::>() + .join("\n") + + "\n"; + fs::write(temp.path().join("history.jsonl"), history).unwrap(); + write_outputs( + temp.path(), + &sample_metadata(), + &sample_results(), + DEFAULT_PAGES_URL, + DEFAULT_RAW_BASE_URL, + ) + .unwrap(); + + for file in [ + "manifest.json", + "latest.json", + "history.jsonl", + "benchmark.svg", + "index.html", + ".nojekyll", + ] { + assert!(temp.path().join(file).is_file(), "missing {file}"); + } + let manifest: Value = + serde_json::from_str(&fs::read_to_string(temp.path().join("manifest.json")).unwrap()) + .unwrap(); + assert_eq!(manifest["branch"], "benchmark-stats"); + assert_eq!( + manifest["artifacts"]["history"]["max_lines"], + HISTORY_MAX_LINES + ); + let history = fs::read_to_string(temp.path().join("history.jsonl")).unwrap(); + assert_eq!(history.lines().count(), HISTORY_MAX_LINES); + assert!(history.lines().last().unwrap().contains("0123456789abcdef")); + let latest: Value = + serde_json::from_str(&fs::read_to_string(temp.path().join("latest.json")).unwrap()) + .unwrap(); + assert_eq!( + latest["metadata"]["cold_definition"], + "project outputs, reusable framework objects, compiler-object caches, and Arduino/PlatformIO download/HTTP caches removed; installed packages/toolchains and fbuild package archives retained" + ); + let html = fs::read_to_string(temp.path().join("index.html")).unwrap(); + assert!(html.contains("stable discovery index for agents")); + assert!(html.contains("compiler-object caches")); + assert!(html.contains("Arduino/PlatformIO download/HTTP caches")); + assert!(html.contains("fbuild package archives")); +} diff --git a/crates/fbuild-build-engine/src/framework_core_cache.rs b/crates/fbuild-build-engine/src/framework_core_cache.rs index 484281a57..1a2e40546 100644 --- a/crates/fbuild-build-engine/src/framework_core_cache.rs +++ b/crates/fbuild-build-engine/src/framework_core_cache.rs @@ -55,6 +55,40 @@ impl FrameworkCoreCache { Self { key, path } } + /// Test seam: like [`FrameworkCoreCache::new`], but rooted at an explicit + /// cache root instead of the process-global `~/.fbuild/{dev|prod}/cache`. + /// Without this, unit tests write into (and read back from) the REAL user + /// cache, so leftover artifacts from earlier runs change hydrate counts + /// and make the tests both flaky and cache-polluting. + /// Mirrors [`FrameworkCoreCache::new`]'s full signature plus the cache + /// root, so the argument count is inherent to the seam. + #[cfg(test)] + #[allow(clippy::too_many_arguments)] + fn new_with_cache_root( + cache_root: &Path, + project_dir: &Path, + platform_label: &str, + env_name: &str, + profile: BuildProfile, + compiler: &dyn Compiler, + core_sources: &[PathBuf], + extra_flags: &LanguageExtraFlags, + ) -> Self { + let key = core_cache_key( + project_dir, + platform_label, + env_name, + profile, + compiler, + core_sources, + extra_flags, + ); + let path = fbuild_packages::Cache::with_cache_root(project_dir, cache_root) + .core_artifacts_dir() + .join(&key); + Self { key, path } + } + pub fn key(&self) -> &str { &self.key } @@ -479,7 +513,13 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let project = tmp.path().join("project"); std::fs::create_dir_all(&project).unwrap(); - let cache = FrameworkCoreCache::new( + // Hermetic cache root: rooting at the real user cache made this test + // accumulate one obj+dep pair per run (the object filename hashes the + // per-run tempdir source path while the cache key stays constant), so + // hydrate's copied count grew 3, 5, 7, ... across repeated runs. + let cache_root = tmp.path().join("cache-root"); + let cache = FrameworkCoreCache::new_with_cache_root( + &cache_root, &project, "avr", "uno", @@ -565,7 +605,11 @@ mod tests { let source = project.join("src/main.cpp"); std::fs::create_dir_all(source.parent().unwrap()).unwrap(); std::fs::write(&source, b"same").unwrap(); - let selected = FrameworkCoreCache::new( + // Hermetic cache root (see hydrate_and_store_only_core_artifact_files): + // never create/remove entries under the real user cache from a test. + let cache_root = tmp.path().join("cache-root"); + let selected = FrameworkCoreCache::new_with_cache_root( + &cache_root, &project, "avr", "uno", @@ -574,7 +618,8 @@ mod tests { std::slice::from_ref(&source), &flags, ); - let sibling = FrameworkCoreCache::new( + let sibling = FrameworkCoreCache::new_with_cache_root( + &cache_root, &project, "avr", "mega", diff --git a/crates/fbuild-cli/src/cli/args.rs b/crates/fbuild-cli/src/cli/args.rs index 178b2771e..d5eb2e679 100644 --- a/crates/fbuild-cli/src/cli/args.rs +++ b/crates/fbuild-cli/src/cli/args.rs @@ -39,8 +39,12 @@ impl From for ShrinkMode { #[derive(ValueEnum, Clone, Copy, Debug, PartialEq, Eq)] #[clap(rename_all = "kebab-case")] pub enum CleanScope { + /// Remove the selected project build outputs. Sketch, + /// Also remove the selected target's reusable framework objects. All, + /// Also clear the active fbuild mode's global compiler-object cache. + Cache, } #[derive(Parser)] @@ -260,9 +264,9 @@ pub enum Commands { #[arg(long)] bloat_analysis: bool, }, - /// Remove project outputs and optionally reusable framework caches. + /// Remove project outputs and optionally reusable or compiler caches. Clean { - /// Cleanup scope: project outputs only, or outputs plus matching caches. + /// Cleanup scope: project outputs only, matching caches, or all compiler objects. #[arg(value_enum)] scope: CleanScope, project_dir: Option, diff --git a/crates/fbuild-cli/src/cli/clean.rs b/crates/fbuild-cli/src/cli/clean.rs index 11065fbd9..56b6de477 100644 --- a/crates/fbuild-cli/src/cli/clean.rs +++ b/crates/fbuild-cli/src/cli/clean.rs @@ -2,8 +2,15 @@ use crate::daemon_client::{self, BuildRequest, DaemonClient}; use crate::output; +use fbuild_core::process_identity::{pid_exe_stem_matches, pid_is_alive, terminate_pid, wait_for_pid_exit}; +use fbuild_paths::daemon_ownership::{self, DAEMON_EXE_STEM, RootOwnershipGuard, SpawnLockGuard}; +use std::future::Future; +use std::io; +use std::path::Path; +use std::time::{Duration, Instant}; -use super::args::CleanScope; +use super::args::{CleanScope, DaemonAction}; +use super::daemon_cmd::run_daemon; use super::deploy::print_operation_streams; pub async fn run_clean( @@ -13,6 +20,9 @@ pub async fn run_clean( quick: bool, release: bool, ) -> fbuild_core::Result<()> { + if matches!(scope, CleanScope::Cache) { + reset_compiler_cache().await?; + } let profile = if release { Some("release".to_string()) } else if quick { @@ -27,7 +37,7 @@ pub async fn run_clean( project_dir, environment, clean_build: true, - clean_all: matches!(scope, CleanScope::All), + clean_all: matches!(scope, CleanScope::All | CleanScope::Cache), clean_only: true, verbose: false, jobs: None, @@ -57,3 +67,521 @@ pub async fn run_clean( } Ok(()) } + +/// Reset the global compiler cache (`/zccache`). +/// +/// soldr-style ownership orchestration (FastLED/fbuild#1159): a spawn-herd +/// single-flight lock is held across the whole reset so no concurrent daemon +/// spawner races the deletion; a legacy/rollout sweep hunts down every other +/// live `fbuild-daemon` that might still hold the cache root (via HTTP for +/// discoverable daemons and verified-PID signalling for stale/legacy ones); +/// and the exclusive `RootOwnershipGuard` is the final, positive proof that +/// every owning daemon has actually exited before the directory is removed. +async fn reset_compiler_cache() -> fbuild_core::Result<()> { + let compiler_cache = fbuild_paths::get_fbuild_root().join("zccache"); + + // 1) Spawn-herd single flight, held across the whole reset. + let spawn_guard = acquire_reset_spawn_lock().await; + + let client = DaemonClient::new(); + + // 2) Identity validation: refuse if the daemon at the resolved port + // belongs to a different mode/cache root. + validate_running_daemon_identity(&client).await?; + + // 3) Graceful stop of the current-version daemon. + let stop_result = run_daemon(DaemonAction::Stop).await; + if let Err(stop_error) = stop_result { + let daemon_stayed_healthy = daemon_stays_healthy(&client).await; + accept_stop_outcome(Err(stop_error), daemon_stayed_healthy)?; + } + + // 4) Legacy/rollout sweep: discover and shut down every other live + // fbuild-daemon that could own this cache root. + if let Err(sweep_error) = sweep_legacy_daemons().await { + drop(spawn_guard); + let restore_result = daemon_client::ensure_daemon_running().await; + return combine_reset_results(Err(sweep_error), restore_result); + } + + // 5) Exclusive root ownership: the positive proof every owner exited. + let ownership_guard = match acquire_root_ownership_before_reset().await { + Ok(guard) => guard, + Err(ownership_error) => { + drop(spawn_guard); + let restore_result = daemon_client::ensure_daemon_running().await; + return combine_reset_results(Err(ownership_error), restore_result); + } + }; + + // 6) Delete the cache. + let cleanup_result = remove_compiler_cache_at(&compiler_cache).await; + + // 7) Drop ownership + spawn guards *before* restarting so the new daemon + // (and its own spawn-lock acquisition) isn't blocked by our own guards. + drop(ownership_guard); + drop(spawn_guard); + finish_cache_reset(cleanup_result, || run_daemon(DaemonAction::Restart)).await?; + + output::result(format!( + "global compiler cache cleared: {}", + compiler_cache.display() + )); + Ok(()) +} + +/// Acquire the spawn-herd single-flight lock, retrying briefly if another +/// process is currently mid-spawn. Root ownership (step 5) is the final +/// gate, so failing to acquire this lock after the retry budget is not +/// fatal — the reset proceeds anyway. +async fn acquire_reset_spawn_lock() -> Option { + const RETRIES: usize = 50; + const POLL_INTERVAL: Duration = Duration::from_millis(100); + + for attempt in 0..RETRIES { + if let Some(guard) = daemon_ownership::try_acquire_spawn_lock() { + return Some(guard); + } + if attempt + 1 < RETRIES { + tokio::time::sleep(POLL_INTERVAL).await; + } + } + None +} + +async fn validate_running_daemon_identity(client: &DaemonClient) -> fbuild_core::Result<()> { + if !client.health().await { + return Ok(()); + } + + let info = client.daemon_info().await?; + if let Some(error) = daemon_client::daemon_cache_identity_error(&info) { + return Err(fbuild_core::FbuildError::DaemonError(format!( + "refusing compiler cache reset: {error}" + ))); + } + Ok(()) +} + +async fn daemon_stays_healthy(client: &DaemonClient) -> bool { + const CHECKS: usize = 20; + const POLL_INTERVAL: Duration = Duration::from_millis(100); + + for check in 0..CHECKS { + if !client.health().await { + return false; + } + if check + 1 < CHECKS { + tokio::time::sleep(POLL_INTERVAL).await; + } + } + true +} + +fn accept_stop_outcome( + stop_result: fbuild_core::Result<()>, + daemon_stayed_healthy: bool, +) -> fbuild_core::Result<()> { + match stop_result { + Ok(()) => Ok(()), + Err(_) if !daemon_stayed_healthy => Ok(()), + Err(error) => Err(error), + } +} + +// --- Legacy/rollout sweep (FastLED/fbuild#1159 step 4) --- + +/// Discover and shut down every other live `fbuild-daemon` that could own +/// this cache root: any daemon reachable via a `daemon-*.port` file whose +/// reported cache identity matches ours (HTTP shutdown), then any PID +/// recorded via the owner claim, the stable legacy PID file, or +/// `daemon_status.json` (verified-PID signal, never an unverified/recycled +/// PID). +async fn sweep_legacy_daemons() -> fbuild_core::Result<()> { + shutdown_matching_port_daemons().await?; + + let mut candidates: Vec = Vec::new(); + if let Some(claim) = daemon_ownership::read_owner_claim() { + candidates.push(claim.pid); + } + if let Some(pid) = read_stable_pid_file() { + candidates.push(pid); + } + if let Some(pid) = read_status_json_daemon_pid() { + candidates.push(pid); + } + candidates.sort_unstable(); + candidates.dedup(); + + for pid in candidates { + terminate_legacy_pid_if_verified(pid).await; + } + Ok(()) +} + +/// Enumerate `daemon-*.port` files under `get_daemon_dir()`. For each port +/// that answers `/api/daemon/info` with a matching cache identity, request a +/// graceful (non-force) shutdown. A busy daemon (still healthy after the +/// shutdown request errors) is a hard refusal of the whole reset. A daemon +/// reporting a *different* cache identity is left alone. +async fn shutdown_matching_port_daemons() -> fbuild_core::Result<()> { + let daemon_dir = fbuild_paths::get_daemon_dir(); + let entries = match std::fs::read_dir(&daemon_dir) { + Ok(entries) => entries, + Err(_) => return Ok(()), + }; + + let expected_identity = fbuild_paths::running_process::DaemonCacheIdentity::discover(); + let expected_label = expected_identity.label_value(); + + for entry in entries.flatten() { + let path = entry.path(); + let Some(file_name) = path.file_name().and_then(|n| n.to_str()) else { + continue; + }; + if !is_daemon_port_file_name(file_name) { + continue; + } + let Some(port) = std::fs::read_to_string(&path) + .ok() + .and_then(|contents| parse_port_file_contents(&contents)) + else { + continue; + }; + + let legacy_client = DaemonClient::with_port(port); + let Ok(info) = legacy_client.daemon_info().await else { + // Did not answer: not live, or already stopped. Nothing to do. + continue; + }; + if info.cache_identity.as_deref() != Some(expected_label.as_str()) { + // Different cache identity: leave it alone. + continue; + } + + if let Err(shutdown_error) = legacy_client.shutdown().await { + let still_healthy = daemon_stays_healthy(&legacy_client).await; + legacy_shutdown_outcome(port, shutdown_error, still_healthy)?; + } + } + Ok(()) +} + +/// Decide the outcome of a failed legacy-daemon shutdown request: a daemon +/// still healthy after the request errored is busy and refuses the whole +/// reset; a daemon that is gone (connection lost because it actually shut +/// down) is accepted. +fn legacy_shutdown_outcome( + port: u16, + shutdown_error: fbuild_core::FbuildError, + still_healthy: bool, +) -> fbuild_core::Result<()> { + if still_healthy { + Err(fbuild_core::FbuildError::DaemonError(format!( + "refusing compiler cache reset: legacy daemon on port {port} refused shutdown ({shutdown_error})" + ))) + } else { + Ok(()) + } +} + +fn is_daemon_port_file_name(name: &str) -> bool { + name.starts_with("daemon-") && name.ends_with(".port") +} + +fn parse_port_file_contents(contents: &str) -> Option { + let port: u16 = contents.trim().parse().ok()?; + (port > 0).then_some(port) +} + +fn parse_stable_pid_contents(contents: &str) -> Option { + contents.trim().parse().ok() +} + +fn read_stable_pid_file() -> Option { + std::fs::read_to_string(fbuild_paths::get_daemon_pid_file()) + .ok() + .and_then(|contents| parse_stable_pid_contents(&contents)) +} + +fn parse_status_json_daemon_pid(contents: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(contents).ok()?; + value.get("daemon_pid")?.as_u64().map(|pid| pid as u32) +} + +fn read_status_json_daemon_pid() -> Option { + std::fs::read_to_string(fbuild_paths::get_daemon_status_file()) + .ok() + .and_then(|contents| parse_status_json_daemon_pid(&contents)) +} + +/// The verified-PID identity gate: only signal a PID that is alive AND +/// whose running executable stem matches `fbuild-daemon`. A recycled PID +/// running an unrelated program must never be signalled. +fn should_signal_legacy_pid(is_alive: bool, exe_stem_matches: bool) -> bool { + is_alive && exe_stem_matches +} + +async fn terminate_legacy_pid_if_verified(pid: u32) { + let verified = should_signal_legacy_pid(pid_is_alive(pid), pid_exe_stem_matches(pid, DAEMON_EXE_STEM)); + if !verified { + return; + } + if wait_for_pid_exit(pid, Duration::from_secs(5)) { + return; + } + terminate_pid(pid); + wait_for_pid_exit(pid, Duration::from_secs(5)); +} + +// --- Exclusive root ownership (FastLED/fbuild#1159 step 5) --- + +async fn acquire_root_ownership_before_reset() -> fbuild_core::Result { + const TIMEOUT: Duration = Duration::from_secs(30); + const POLL_INTERVAL: Duration = Duration::from_millis(100); + let started = Instant::now(); + + loop { + match RootOwnershipGuard::try_acquire() { + Ok(Some(guard)) => return Ok(guard), + Ok(None) => {} + Err(error) => { + return Err(fbuild_core::FbuildError::DaemonError(format!( + "failed while waiting for exclusive cache-root ownership: {error}" + ))); + } + } + if started.elapsed() >= TIMEOUT { + return Err(fbuild_core::FbuildError::DaemonError( + "a live fbuild-daemon still owns the cache root; reset aborted".to_string(), + )); + } + tokio::time::sleep(POLL_INTERVAL).await; + } +} + +async fn finish_cache_reset( + cleanup_result: fbuild_core::Result<()>, + restart: Restart, +) -> fbuild_core::Result<()> +where + Restart: FnOnce() -> RestartFuture, + RestartFuture: Future>, +{ + let restart_result = restart().await; + combine_reset_results(cleanup_result, restart_result) +} + +fn combine_reset_results( + cleanup_result: fbuild_core::Result<()>, + restart_result: fbuild_core::Result<()>, +) -> fbuild_core::Result<()> { + match (cleanup_result, restart_result) { + (Ok(()), Ok(())) => Ok(()), + (Err(cleanup_error), Ok(())) => Err(cleanup_error), + (Ok(()), Err(restart_error)) => Err(restart_error), + (Err(cleanup_error), Err(restart_error)) => Err(fbuild_core::FbuildError::Other(format!( + "compiler cache cleanup failed: {cleanup_error}; daemon restart also failed: {restart_error}" + ))), + } +} + +async fn remove_compiler_cache_at(compiler_cache: &Path) -> fbuild_core::Result<()> { + if compiler_cache.file_name() != Some(std::ffi::OsStr::new("zccache")) { + return Err(fbuild_core::FbuildError::Other(format!( + "refusing to remove unexpected compiler cache path: {}", + compiler_cache.display() + ))); + } + + const RETRIES: usize = 20; + for attempt in 0..=RETRIES { + match fbuild_core::fs::remove_dir_all(compiler_cache).await { + Ok(()) => return Ok(()), + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(error) if attempt < RETRIES => { + tracing::debug!( + path = %compiler_cache.display(), + attempt = attempt + 1, + error = %error, + "compiler cache removal retry" + ); + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + Err(error) => return Err(error.into()), + } + } + unreachable!("compiler cache retry loop always returns") +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, + }; + + #[tokio::test] + async fn compiler_cache_cleanup_removes_only_requested_root_and_is_idempotent() { + let temp = tempfile::tempdir().unwrap(); + let compiler_cache = temp.path().join("zccache"); + let sibling_cache = temp.path().join("cache"); + fbuild_core::fs::create_dir_all(compiler_cache.join("objects")) + .await + .unwrap(); + fbuild_core::fs::create_dir_all(&sibling_cache) + .await + .unwrap(); + fbuild_core::fs::write(compiler_cache.join("objects/cached.o"), b"object") + .await + .unwrap(); + fbuild_core::fs::write(sibling_cache.join("package.tar"), b"package") + .await + .unwrap(); + + remove_compiler_cache_at(&compiler_cache).await.unwrap(); + remove_compiler_cache_at(&compiler_cache).await.unwrap(); + + assert!(!compiler_cache.exists()); + assert!(sibling_cache.join("package.tar").is_file()); + } + + #[tokio::test] + async fn cleanup_failure_still_attempts_daemon_restart() { + let restart_calls = Arc::new(AtomicUsize::new(0)); + let observed_calls = Arc::clone(&restart_calls); + + let result = finish_cache_reset( + Err(fbuild_core::FbuildError::Other( + "synthetic cleanup failure".to_string(), + )), + move || { + let observed_calls = Arc::clone(&observed_calls); + async move { + observed_calls.fetch_add(1, Ordering::SeqCst); + Ok(()) + } + }, + ) + .await; + + assert!(result.is_err()); + assert_eq!(restart_calls.load(Ordering::SeqCst), 1); + } + + #[test] + fn daemon_stop_refusal_is_preserved_while_daemon_stays_healthy() { + let result = accept_stop_outcome( + Err(fbuild_core::FbuildError::DaemonError( + "operation in progress".to_string(), + )), + true, + ); + + assert!(result.is_err()); + } + + #[test] + fn lost_stop_response_is_accepted_after_daemon_exits() { + let result = accept_stop_outcome( + Err(fbuild_core::FbuildError::DaemonError( + "connection closed".to_string(), + )), + false, + ); + + assert!(result.is_ok()); + } + + #[test] + fn legacy_shutdown_busy_daemon_is_hard_refusal() { + let result = legacy_shutdown_outcome( + 49200, + fbuild_core::FbuildError::DaemonError("operation in progress".to_string()), + true, + ); + + assert!(result.is_err()); + } + + #[test] + fn legacy_shutdown_lost_connection_after_daemon_exit_is_accepted() { + let result = legacy_shutdown_outcome( + 49200, + fbuild_core::FbuildError::DaemonError("connection closed".to_string()), + false, + ); + + assert!(result.is_ok()); + } + + #[test] + fn is_daemon_port_file_name_matches_expected_pattern() { + assert!(is_daemon_port_file_name("daemon-0123456789abcdef.port")); + assert!(!is_daemon_port_file_name("daemon.log")); + assert!(!is_daemon_port_file_name("daemon_status.json")); + assert!(!is_daemon_port_file_name("fbuild_daemon.pid")); + } + + #[test] + fn parse_port_file_contents_rejects_garbage_and_zero() { + assert_eq!(parse_port_file_contents("49200"), Some(49200)); + assert_eq!(parse_port_file_contents("49200\n"), Some(49200)); + assert_eq!(parse_port_file_contents("0"), None); + assert_eq!(parse_port_file_contents("not-a-port"), None); + assert_eq!(parse_port_file_contents(""), None); + } + + #[test] + fn parse_stable_pid_contents_parses_valid_and_rejects_garbage() { + assert_eq!(parse_stable_pid_contents("1234"), Some(1234)); + assert_eq!(parse_stable_pid_contents("1234\n"), Some(1234)); + assert_eq!(parse_stable_pid_contents("not-a-pid"), None); + assert_eq!(parse_stable_pid_contents(""), None); + } + + #[test] + fn parse_status_json_daemon_pid_round_trips_and_rejects_malformed() { + assert_eq!( + parse_status_json_daemon_pid(r#"{"daemon_pid": 4321}"#), + Some(4321) + ); + assert_eq!(parse_status_json_daemon_pid(r#"{"daemon_pid": null}"#), None); + assert_eq!(parse_status_json_daemon_pid(r#"{}"#), None); + assert_eq!(parse_status_json_daemon_pid("not json"), None); + } + + #[test] + fn should_signal_legacy_pid_requires_both_gates() { + assert!(should_signal_legacy_pid(true, true)); + assert!(!should_signal_legacy_pid(true, false)); + assert!(!should_signal_legacy_pid(false, true)); + assert!(!should_signal_legacy_pid(false, false)); + } + + /// A recycled/unrelated PID must never be signalled: `pid_exe_stem_matches` + /// fails closed for a dead PID, and (separately) for a live PID whose + /// image is not `fbuild-daemon`. Uses the current process's own PID + /// (guaranteed alive) with a deliberately wrong expected stem, and a + /// dead PID (`i32::MAX as u32` — NOT `u32::MAX`, which is `-1` on unix + /// and would signal the calling process's own process group). + #[test] + fn recycled_or_unrelated_pid_is_never_signalled() { + let own_pid = std::process::id(); + assert!(pid_is_alive(own_pid)); + assert!(!pid_exe_stem_matches(own_pid, "definitely-not-fbuild-daemon")); + assert!(!should_signal_legacy_pid( + pid_is_alive(own_pid), + pid_exe_stem_matches(own_pid, "definitely-not-fbuild-daemon") + )); + + let dead_pid = i32::MAX as u32; + assert!(!pid_is_alive(dead_pid)); + assert!(!pid_exe_stem_matches(dead_pid, DAEMON_EXE_STEM)); + assert!(!should_signal_legacy_pid( + pid_is_alive(dead_pid), + pid_exe_stem_matches(dead_pid, DAEMON_EXE_STEM) + )); + } +} diff --git a/crates/fbuild-cli/src/cli/tests.rs b/crates/fbuild-cli/src/cli/tests.rs index 1c29d0f41..e36bfc337 100644 --- a/crates/fbuild-cli/src/cli/tests.rs +++ b/crates/fbuild-cli/src/cli/tests.rs @@ -293,6 +293,33 @@ fn clean_scope_and_profile_are_accepted() { } } +#[test] +fn clean_cache_scope_is_accepted() { + let argv = [ + "fbuild", + "clean", + "cache", + "tests/platform/uno", + "--release", + ]; + let cli = Cli::try_parse_from(argv).expect("parse"); + match cli.command { + Some(Commands::Clean { + scope, + project_dir, + quick, + release, + .. + }) => { + assert_eq!(scope, super::args::CleanScope::Cache); + assert_eq!(project_dir.as_deref(), Some("tests/platform/uno")); + assert!(!quick); + assert!(release); + } + _ => panic!("expected Clean subcommand"), + } +} + #[test] fn clean_quick_and_release_conflict() { let argv = ["fbuild", "clean", "sketch", "--quick", "--release"]; diff --git a/crates/fbuild-cli/src/daemon_client.rs b/crates/fbuild-cli/src/daemon_client.rs index f5de0f034..6594ba914 100644 --- a/crates/fbuild-cli/src/daemon_client.rs +++ b/crates/fbuild-cli/src/daemon_client.rs @@ -42,7 +42,7 @@ fn stream_status_message(event: &StreamEvent) -> Option { }) } -fn daemon_cache_identity_error(info: &DaemonInfoResponse) -> Option { +pub(crate) fn daemon_cache_identity_error(info: &DaemonInfoResponse) -> Option { let expected = fbuild_paths::running_process::DaemonCacheIdentity::discover(); let expected_label = expected.label_value(); if info.cache_identity.as_deref() != Some(expected_label.as_str()) { @@ -222,6 +222,21 @@ impl DaemonClient { } } + /// Construct a client targeting an explicit port, bypassing the normal + /// endpoint-key port resolution in [`fbuild_paths::get_daemon_url`]. + /// + /// Used by the legacy/rollout sweep in `fbuild clean cache` + /// (FastLED/fbuild#1159) to probe and shut down other live + /// `fbuild-daemon` processes discovered via `daemon-*.port` files, which + /// may not be at this process's own resolved endpoint. + pub fn with_port(port: u16) -> Self { + let client = fbuild_core::http::client().clone(); + Self { + base_url: format!("http://127.0.0.1:{port}"), + client, + } + } + pub fn websocket_url(&self, path: &str) -> String { let scheme = if self.base_url.starts_with("https://") { "wss://" @@ -864,6 +879,25 @@ async fn ensure_direct_daemon_running() -> fbuild_core::Result<()> { tracing::info!("daemon not running, starting..."); + // Spawn-herd election (FastLED/fbuild#1159): a fan-out of concurrent CLI + // invocations that all observe "daemon not running" must not each spawn + // their own daemon. Only the winner of this single-flight lock actually + // spawns; losers wait for the winner's daemon to become healthy instead. + let Some(_spawn_guard) = daemon_client_spawn_lock() else { + for _ in 0..50 { + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + if client.health().await { + return Ok(()); + } + } + return Err(fbuild_core::FbuildError::DaemonError( + "daemon did not become healthy while another process was spawning it".to_string(), + )); + }; + + // Winner: `_spawn_guard` is held alive through the spawn + readiness poll + // below so no other caller starts a redundant daemon while we're mid-spawn. + // Retry daemon spawn up to 3 times with exponential backoff // (matches Python behavior: [0.0s, 0.5s, 2.0s] delays between attempts) let backoff_delays = [0.0, 0.5, 2.0]; @@ -918,6 +952,14 @@ async fn ensure_direct_daemon_running() -> fbuild_core::Result<()> { )) } +/// Acquire the spawn-herd single-flight lock (FastLED/fbuild#1159). Thin +/// wrapper kept local to this module so the call site above reads naturally; +/// `try_acquire_spawn_lock` itself already fails safely to `None` on lock +/// errors (never gates progress on a broken filesystem). +fn daemon_client_spawn_lock() -> Option { + fbuild_paths::daemon_ownership::try_acquire_spawn_lock() +} + /// Spawn a single daemon process instance. async fn spawn_daemon_process() -> fbuild_core::Result<()> { // FastLED/fbuild#830: prefer a `fbuild-daemon` binary sitting next diff --git a/crates/fbuild-core/Cargo.toml b/crates/fbuild-core/Cargo.toml index 4ec758371..43c101ade 100644 --- a/crates/fbuild-core/Cargo.toml +++ b/crates/fbuild-core/Cargo.toml @@ -38,6 +38,9 @@ async-trait = { workspace = true } # in the workspace has one source of truth. See `src/http.rs` and the # matching `ban_bare_reqwest` dylint. reqwest = { workspace = true } +# Cross-process shared/exclusive fbuild-daemon lifecycle gates. These are not +# used for zccache object access. Locks are released if a process exits. +fs2 = { workspace = true } # `libc::setpgid` / `libc::prctl` for the Unix `pre_exec` hook used by # `containment::tokio_spawn::configure`. See FastLED/fbuild#32. diff --git a/crates/fbuild-core/README.md b/crates/fbuild-core/README.md index 7d8a7ede1..8df1812de 100644 --- a/crates/fbuild-core/README.md +++ b/crates/fbuild-core/README.md @@ -18,6 +18,8 @@ Core types, errors, and utilities shared across all fbuild crates. - **build_log** -- Centralized build output log with optional `mpsc::Sender` streaming - **compiler_flags** -- Platform-correct escaping for GCC `-D` define flags +- **file_lock** -- Generic OS-released shared/exclusive file-lock primitives, used by `fbuild-paths::daemon_ownership` for fbuild-daemon startup/lifetime ownership (not object-cache access, which stays zccache-internal) +- **process_identity** -- PID liveness, exe-identity, and terminate helpers (`pid_is_alive`, `pid_executable_path`, `pid_exe_stem_matches`, `terminate_pid`, `wait_for_pid_exit`) used to safely displace stale/legacy fbuild-daemon processes; fails closed when a process image can't be read or verified - **response_file** -- GCC `@file` response file writer for Windows command-line length limits - **shell_split** -- Quote-aware string splitting that treats backslashes as literal (Windows-safe) - **subprocess** -- Command runner with timeout, `CREATE_NO_WINDOW` on Windows, and MSYS environment stripping diff --git a/crates/fbuild-core/src/file_lock.rs b/crates/fbuild-core/src/file_lock.rs new file mode 100644 index 000000000..eb9277628 --- /dev/null +++ b/crates/fbuild-core/src/file_lock.rs @@ -0,0 +1,157 @@ +//! Cross-process locks for fbuild-daemon startup/lifecycle coordination. +//! +//! These locks are deliberately outside zccache's compile/object hot path, +//! whose synchronization remains internal to zccache. + +use fs2::FileExt; +use std::fs::{File, OpenOptions}; +use std::io; +use std::path::Path; +use std::time::{Duration, Instant}; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum FileLockMode { + Shared, + Exclusive, +} + +#[derive(Debug)] +pub struct FileLockGuard { + _file: File, +} + +/// Try to acquire an OS-released lock on `path`. +/// +/// Returns `Ok(None)` when another process holds a conflicting lock. The lock +/// is released automatically when the guard is dropped or the process exits. +pub fn try_acquire(path: &Path, mode: FileLockMode) -> io::Result> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let file = OpenOptions::new() + .create(true) + .truncate(false) + .read(true) + .write(true) + .open(path)?; + let result = match mode { + FileLockMode::Shared => FileExt::try_lock_shared(&file), + FileLockMode::Exclusive => FileExt::try_lock_exclusive(&file), + }; + match result { + Ok(()) => Ok(Some(FileLockGuard { _file: file })), + Err(error) if lock_is_held(&error) => Ok(None), + Err(error) => Err(error), + } +} + +/// Does this error mean "another process holds a conflicting lock"? +/// +/// Unix reports contention as `EWOULDBLOCK` (kind `WouldBlock`), but Windows +/// `LockFileEx(LOCKFILE_FAIL_IMMEDIATELY)` reports `ERROR_LOCK_VIOLATION` +/// (os error 33), which std maps to an uncategorized kind — so a kind check +/// alone misclassifies contention as a hard error on Windows. Also compare +/// against `fs2::lock_contended_error()` (the canonical per-platform +/// contention error), mirroring soldr's `lock_is_held`. +fn lock_is_held(error: &io::Error) -> bool { + error.kind() == io::ErrorKind::WouldBlock + || error.raw_os_error() == fs2::lock_contended_error().raw_os_error() +} + +/// Wait up to `timeout` for a cross-process file lock. +pub async fn acquire( + path: &Path, + mode: FileLockMode, + timeout: Duration, + poll: Duration, +) -> io::Result { + let started = Instant::now(); + loop { + if let Some(guard) = try_acquire(path, mode)? { + return Ok(guard); + } + if started.elapsed() >= timeout { + return Err(io::Error::new( + io::ErrorKind::TimedOut, + format!( + "timed out after {:.1}s waiting for {} lock on {}", + timeout.as_secs_f64(), + match mode { + FileLockMode::Shared => "shared", + FileLockMode::Exclusive => "exclusive", + }, + path.display() + ), + )); + } + tokio::time::sleep(poll).await; + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn shared_holders_block_exclusive_until_all_release() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("cache.lock"); + let first = try_acquire(&path, FileLockMode::Shared) + .unwrap() + .expect("first shared lock"); + let second = try_acquire(&path, FileLockMode::Shared) + .unwrap() + .expect("second shared lock"); + + assert!( + try_acquire(&path, FileLockMode::Exclusive) + .unwrap() + .is_none() + ); + drop(first); + assert!( + try_acquire(&path, FileLockMode::Exclusive) + .unwrap() + .is_none() + ); + drop(second); + assert!( + try_acquire(&path, FileLockMode::Exclusive) + .unwrap() + .is_some() + ); + } + + #[test] + fn exclusive_holder_blocks_shared_until_release() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("cache.lock"); + let exclusive = try_acquire(&path, FileLockMode::Exclusive) + .unwrap() + .expect("exclusive lock"); + + assert!(try_acquire(&path, FileLockMode::Shared).unwrap().is_none()); + drop(exclusive); + assert!(try_acquire(&path, FileLockMode::Shared).unwrap().is_some()); + } + + #[tokio::test] + async fn timed_acquire_fails_closed_while_conflicting_lock_is_held() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("cache.lock"); + let _exclusive = try_acquire(&path, FileLockMode::Exclusive) + .unwrap() + .expect("exclusive lock"); + + let error = acquire( + &path, + FileLockMode::Shared, + Duration::from_millis(20), + Duration::from_millis(5), + ) + .await + .unwrap_err(); + + assert_eq!(error.kind(), io::ErrorKind::TimedOut); + } +} diff --git a/crates/fbuild-core/src/lib.rs b/crates/fbuild-core/src/lib.rs index 9a617a7c2..f318de3f3 100644 --- a/crates/fbuild-core/src/lib.rs +++ b/crates/fbuild-core/src/lib.rs @@ -13,10 +13,12 @@ pub mod containment; pub mod elapsed; pub mod emulator; pub mod env_namespace; +pub mod file_lock; pub mod fs; pub mod http; pub mod install_status; pub mod path; +pub mod process_identity; pub mod response_file; pub mod shell_split; pub mod subprocess; diff --git a/crates/fbuild-core/src/process_identity.rs b/crates/fbuild-core/src/process_identity.rs new file mode 100644 index 000000000..02539707a --- /dev/null +++ b/crates/fbuild-core/src/process_identity.rs @@ -0,0 +1,267 @@ +//! Cross-platform, PID-recycling-safe process identity primitives. +//! +//! Copied from soldr's daemon lifecycle semantics +//! (`.extern-repos/soldr/crates/soldr-daemon/src/daemon/lifecycle.rs`): +//! before ever signalling a PID recorded in a file, verify (a) the PID is +//! still alive, AND (b) its running executable image's file stem matches +//! what we expect. A PID can be recycled by the OS between when a file was +//! written and when it's read back, so liveness alone is not enough — an +//! unrelated process could receive a `SIGKILL` / `TerminateProcess` meant +//! for a long-dead daemon. +//! +//! [`pid_exe_stem_matches`] **fails closed**: if the process image can't be +//! inspected (permission denied, already exited, platform probe failure), +//! it returns `false` rather than assuming a match. Callers must never +//! signal a PID whose identity they could not positively verify. + +use std::path::PathBuf; +use std::time::{Duration, Instant}; + +/// Is `pid` currently alive? +/// +/// Unix: `kill(pid, 0)` — delivers no signal, just probes existence + +/// permission. Windows: `OpenProcess` + `GetExitCodeProcess`, checking for +/// `STILL_ACTIVE`. +#[cfg(unix)] +pub fn pid_is_alive(pid: u32) -> bool { + // SAFETY: kill(pid, 0) is a well-defined liveness probe — no signal is + // delivered, the syscall just returns 0 if the pid exists and the + // caller has permission to signal it. + unsafe { libc::kill(pid as libc::pid_t, 0) == 0 } +} + +#[cfg(windows)] +#[allow(clippy::upper_case_acronyms, non_snake_case)] +pub fn pid_is_alive(pid: u32) -> bool { + use std::os::windows::raw::HANDLE; + #[allow(clippy::upper_case_acronyms)] + type DWORD = u32; + #[allow(clippy::upper_case_acronyms)] + type BOOL = i32; + const PROCESS_QUERY_LIMITED_INFORMATION: DWORD = 0x1000; + const STILL_ACTIVE: DWORD = 259; + extern "system" { + fn OpenProcess(desired_access: DWORD, inherit: BOOL, pid: DWORD) -> HANDLE; + fn CloseHandle(h: HANDLE) -> BOOL; + fn GetExitCodeProcess(h: HANDLE, code: *mut DWORD) -> BOOL; + } + let h = unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid) }; + if h.is_null() { + return false; + } + let mut code: DWORD = 0; + let ok = unsafe { GetExitCodeProcess(h, &mut code) }; + unsafe { CloseHandle(h) }; + ok != 0 && code == STILL_ACTIVE +} + +#[cfg(not(any(unix, windows)))] +pub fn pid_is_alive(_pid: u32) -> bool { + false +} + +/// Read the executable image path of a running process. +/// +/// Linux: `/proc//exe` (symlink read). macOS/BSD: `ps -o comm=` +/// (portable, no extra crate). Windows: `QueryFullProcessImageNameW`. Any +/// probe failure returns `None` — the caller treats that as "identity +/// unverified", never as "assume match". +#[cfg(target_os = "linux")] +pub fn pid_executable_path(pid: u32) -> Option { + let link = PathBuf::from(format!("/proc/{pid}/exe")); + std::fs::read_link(link).ok() +} + +#[cfg(all(unix, not(target_os = "linux")))] +pub fn pid_executable_path(pid: u32) -> Option { + let output = std::process::Command::new("/bin/ps") + .args(["-p", &pid.to_string(), "-o", "comm="]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let image = String::from_utf8(output.stdout).ok()?; + let image = image.trim(); + (!image.is_empty()).then(|| PathBuf::from(image)) +} + +#[cfg(windows)] +#[allow(clippy::upper_case_acronyms, non_snake_case)] +pub fn pid_executable_path(pid: u32) -> Option { + use std::os::windows::raw::HANDLE; + #[allow(clippy::upper_case_acronyms)] + type DWORD = u32; + #[allow(clippy::upper_case_acronyms)] + type BOOL = i32; + type WCHAR = u16; + const PROCESS_QUERY_LIMITED_INFORMATION: DWORD = 0x1000; + extern "system" { + fn OpenProcess(desired_access: DWORD, inherit: BOOL, pid: DWORD) -> HANDLE; + fn CloseHandle(h: HANDLE) -> BOOL; + fn QueryFullProcessImageNameW( + h: HANDLE, + flags: DWORD, + buf: *mut WCHAR, + size: *mut DWORD, + ) -> BOOL; + } + let h = unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid) }; + if h.is_null() { + return None; + } + let mut buf: Vec = vec![0; 1024]; + let mut size: DWORD = buf.len() as DWORD; + let ok = unsafe { QueryFullProcessImageNameW(h, 0, buf.as_mut_ptr(), &mut size) }; + unsafe { CloseHandle(h) }; + if ok == 0 { + return None; + } + let s = String::from_utf16_lossy(&buf[..size as usize]); + (!s.is_empty()).then(|| PathBuf::from(s)) +} + +#[cfg(not(any(unix, windows)))] +pub fn pid_executable_path(_pid: u32) -> Option { + None +} + +/// PID-recycling-safe identity gate: does `pid`'s running executable image +/// have file stem `expected_stem`? +/// +/// **Fails closed**: an uninspectable image (permission denied, process +/// already gone, probe error) returns `false`. Case-insensitive on Windows +/// (`file.EXE` vs `file.exe`), case-sensitive elsewhere. Callers must gate +/// every signal (`terminate_pid`) on this check to avoid killing an +/// unrelated process that happens to have inherited a stale PID. +pub fn pid_exe_stem_matches(pid: u32, expected_stem: &str) -> bool { + let Some(path) = pid_executable_path(pid) else { + return false; + }; + let Some(stem) = path.file_stem().and_then(|s| s.to_str()) else { + return false; + }; + if cfg!(windows) { + stem.eq_ignore_ascii_case(expected_stem) + } else { + stem == expected_stem + } +} + +/// Terminate `pid`: SIGTERM then, if still alive after ~3s, SIGKILL (Unix). +/// `TerminateProcess` (Windows, no graceful signal available). Callers MUST +/// have already verified [`pid_exe_stem_matches`] before calling this — +/// this function does not re-check identity, it just signals. +#[cfg(unix)] +pub fn terminate_pid(pid: u32) { + // SAFETY: kill(2) with SIGTERM then (if needed) SIGKILL. The caller is + // responsible for having verified the PID's identity beforehand. + unsafe { + libc::kill(pid as libc::pid_t, libc::SIGTERM); + } + if wait_for_pid_exit(pid, Duration::from_secs(3)) { + return; + } + unsafe { + libc::kill(pid as libc::pid_t, libc::SIGKILL); + } +} + +#[cfg(windows)] +#[allow(clippy::upper_case_acronyms, non_snake_case)] +pub fn terminate_pid(pid: u32) { + use std::os::windows::raw::HANDLE; + #[allow(clippy::upper_case_acronyms)] + type DWORD = u32; + #[allow(clippy::upper_case_acronyms)] + type BOOL = i32; + const PROCESS_TERMINATE: DWORD = 0x0001; + extern "system" { + fn OpenProcess(desired_access: DWORD, inherit: BOOL, pid: DWORD) -> HANDLE; + fn TerminateProcess(h: HANDLE, exit_code: DWORD) -> BOOL; + fn CloseHandle(h: HANDLE) -> BOOL; + } + // SAFETY: OpenProcess for a caller-verified PID; TerminateProcess is the + // Windows equivalent of SIGKILL — there is no graceful-signal analog. + let h = unsafe { OpenProcess(PROCESS_TERMINATE, 0, pid) }; + if h.is_null() { + return; + } + unsafe { + TerminateProcess(h, 1); + CloseHandle(h); + } +} + +#[cfg(not(any(unix, windows)))] +pub fn terminate_pid(_pid: u32) {} + +/// Poll until `pid` exits or `timeout` elapses. Returns `true` if the +/// process was observed dead within the timeout. +pub fn wait_for_pid_exit(pid: u32, timeout: Duration) -> bool { + let deadline = Instant::now() + timeout; + while Instant::now() < deadline { + if !pid_is_alive(pid) { + return true; + } + let remaining = deadline.saturating_duration_since(Instant::now()); + if remaining.is_zero() { + break; + } + std::thread::sleep(Duration::from_millis(50).min(remaining)); + } + !pid_is_alive(pid) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn own_pid_is_alive() { + assert!(pid_is_alive(std::process::id())); + } + + #[test] + fn dead_pid_is_not_alive() { + // A large positive PID that is almost certainly not a running + // process. NOT `u32::MAX`, which casts to `-1` (the "all + // processes" wildcard) on Unix and would spuriously look alive. + let dead = i32::MAX as u32; + assert!(!pid_is_alive(dead)); + } + + #[test] + fn exe_stem_fails_closed_for_wrong_stem() { + // Our own PID is alive, but its exe stem is the test binary, not + // some arbitrary expected name — must fail closed (false), never + // assume a match. + assert!(!pid_exe_stem_matches( + std::process::id(), + "definitely-not-our-test-binary-stem" + )); + } + + #[test] + fn exe_stem_matches_current_exe_stem() { + let current_exe = std::env::current_exe().expect("current exe"); + let stem = current_exe + .file_stem() + .and_then(|s| s.to_str()) + .expect("current exe stem") + .to_string(); + assert!(pid_exe_stem_matches(std::process::id(), &stem)); + } + + #[test] + fn exe_stem_fails_closed_for_dead_pid() { + let dead = i32::MAX as u32; + assert!(!pid_exe_stem_matches(dead, "anything")); + } + + #[test] + fn wait_for_pid_exit_returns_true_for_already_dead_pid() { + let dead = i32::MAX as u32; + assert!(wait_for_pid_exit(dead, Duration::from_millis(50))); + } +} diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index 302917a5e..b3c1895cf 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -106,6 +106,40 @@ async fn main() { // identity-dependent behavior still fails closed. let _ = tokio::task::spawn_blocking(populate_usb_overlay_best_effort).await; + // Soldr-style root ownership (FastLED/fbuild#1154 / #1159): this lock + // covers fbuild-daemon startup/lifetime only — zccache continues to + // synchronize object-cache access internally. The daemon holds this + // exclusively for its entire lifetime; `fbuild clean cache` takes it + // exclusively (after stopping every daemon it can find) as the final, + // version-blind proof that no daemon still owns the cache root before + // deleting it. Poll briefly rather than blocking forever: a wedged + // holder must produce a clear fatal error, not a silent hang. + let _root_ownership_guard = { + const POLL: std::time::Duration = std::time::Duration::from_millis(100); + const WAIT: std::time::Duration = std::time::Duration::from_secs(10); + let deadline = std::time::Instant::now() + WAIT; + loop { + match fbuild_paths::daemon_ownership::RootOwnershipGuard::try_acquire() { + Ok(Some(guard)) => break guard, + Ok(None) => { + if std::time::Instant::now() >= deadline { + eprintln!( + "fatal: another fbuild-daemon owns the cache root (root-owner.lock busy)" + ); + std::process::exit(1); + } + tokio::time::sleep(POLL).await; + } + Err(error) => { + eprintln!( + "fatal: failed to acquire fbuild-daemon root ownership lock: {error}" + ); + std::process::exit(1); + } + } + } + }; + // FastLED/fbuild#800 (Phase 4 stage 2 of #789): start the embedded // zccache service inside this tokio runtime and install the global // handle BEFORE any compile work begins. The wrapper-binary path is @@ -208,6 +242,36 @@ async fn main() { tracing::warn!("failed to write port file: {}", e); } + // Write the soldr-style owner claim (FastLED/fbuild#1159) now that we + // know our port. This is advisory only — readers must always re-verify + // pid liveness + exe stem before trusting it — but it lets a + // `fbuild clean cache` sweep target this exact daemon by port without + // enumerating every `daemon-*.port` file. `current_exe()` failing is + // not treated as fatal: skip the claim write and keep the daemon + // running under the root-ownership lock alone. + match std::env::current_exe() { + Ok(exe) => { + let identity = fbuild_paths::running_process::DaemonCacheIdentity::discover(); + let claim = fbuild_paths::daemon_ownership::OwnerClaim { + pid: std::process::id(), + exe, + version: env!("CARGO_PKG_VERSION").to_string(), + mode: identity.mode.to_string(), + cache_root_key: identity.cache_root_key.clone(), + port, + }; + if let Err(e) = fbuild_paths::daemon_ownership::write_owner_claim(&claim) { + tracing::warn!("failed to write owner claim: {}", e); + } + } + Err(e) => { + tracing::warn!( + "cannot resolve current_exe() for owner claim; skipping claim write: {}", + e + ); + } + } + // Install the running-process `fbuild.servicedef` so the broker can // discover this daemon binary in subsequent fbuild invocations. Without // this file the broker stays in `Mode: direct-fallback` even after @@ -445,9 +509,10 @@ async fn main() { tracing::error!("server error: {}", e); }); - // Clean up PID and port files + // Clean up PID and port files, and the soldr-style owner claim. let _ = fbuild_core::fs::remove_file(&pid_file).await; let _ = fbuild_core::fs::remove_file(&port_file).await; + fbuild_paths::daemon_ownership::remove_owner_claim(); tracing::info!("daemon exiting"); std::process::exit(0); diff --git a/crates/fbuild-daemon/tests/legacy_daemon_transition.rs b/crates/fbuild-daemon/tests/legacy_daemon_transition.rs new file mode 100644 index 000000000..9c90c2e73 --- /dev/null +++ b/crates/fbuild-daemon/tests/legacy_daemon_transition.rs @@ -0,0 +1,256 @@ +//! Real-process regression test for the pre-#1159 legacy daemon transition +//! hazard (FastLED/fbuild#1159). +//! +//! Two things are exercised end-to-end with REAL child processes rather than +//! in-process fakes: +//! +//! 1. A stand-in for a pre-#1159 daemon: a real, long-lived child process +//! whose executable stem is NOT `fbuild-daemon` gets its PID written into +//! a temp-dir stand-in for the legacy stable pid file +//! (`fbuild_paths::get_daemon_pid_file()`'s layout). Any code path that +//! would read that PID and consider signalling it MUST refuse, because +//! `fbuild_core::process_identity::pid_exe_stem_matches(pid, +//! "fbuild-daemon")` fails closed on the stem mismatch. This test proves +//! the gate stays closed and the process survives, and that liveness +//! detection correctly flips to `false` once the process is actually +//! killed (by the test itself, not through the gated path). +//! +//! 2. The REAL `fbuild-daemon` binary is spawned with an isolated temp +//! HOME/USERPROFILE (+ `FBUILD_DEV_MODE=1`), and +//! `fbuild_paths::daemon_ownership::RootOwnershipGuard::try_acquire_at` +//! on its `root-owner.lock` is shown to be blocked while the daemon is +//! alive and healthy, and acquirable again once the daemon process is +//! killed — abrupt owner death releases ownership, exactly like an OS +//! file lock is supposed to. + +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; + +use fbuild_core::process_identity::{pid_exe_stem_matches, pid_is_alive}; +use fbuild_paths::daemon_ownership::{DAEMON_EXE_STEM, RootOwnershipGuard}; + +/// Bounded `Child::wait()` (mirrors `tests/port_recovery.rs`): a missed +/// signal/kill must never hang the test suite. +fn wait_with_timeout(child: &mut Child, budget: Duration) -> bool { + let deadline = Instant::now() + budget; + loop { + match child.try_wait() { + Ok(Some(_)) => return true, + Ok(None) => { + if Instant::now() >= deadline { + let _ = child.kill(); + let _ = child.wait(); + return false; + } + std::thread::sleep(Duration::from_millis(100)); + } + Err(_) => return false, + } + } +} + +#[cfg(unix)] +fn hard_kill(child: &Child) { + // SAFETY: kill(2) with a PID this test process owns (a child it spawned + // itself). No signal handler runs in our own process; this only affects + // the child. + unsafe { + libc::kill(child.id() as i32, libc::SIGKILL); + } +} + +#[cfg(windows)] +fn hard_kill(child: &Child) { + // allow-direct-spawn: test driver hard-killing a process it spawned under test. + let _ = Command::new("taskkill") + .args(["/F", "/PID", &child.id().to_string()]) + .status(); +} + +/// Helper mode: NOT `fbuild-daemon` (this is the integration-test binary, +/// re-invoked as a subprocess), so the parent test has a real, long-lived, +/// non-daemon PID to probe. Runs until killed. `--ignored` because it must +/// never run as part of the normal suite — only the parent test below +/// spawns it explicitly. +#[test] +#[ignore = "subprocess helper for legacy_pid_stand_in_is_never_signaled (#1159)"] +fn subprocess_sleep_helper() { + std::thread::sleep(Duration::from_secs(120)); +} + +#[test] +fn legacy_pid_stand_in_is_never_signaled() { + let bin = std::env::current_exe().expect("current test exe"); + // allow-direct-spawn: test driver spawns its own test binary in helper mode. + let mut helper = Command::new(&bin) + .args([ + "--ignored", + "--exact", + "subprocess_sleep_helper", + "--nocapture", + ]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("spawn non-daemon helper process"); + + // Give it a moment to actually start running. + let start_deadline = Instant::now() + Duration::from_secs(5); + while !pid_is_alive(helper.id()) && Instant::now() < start_deadline { + std::thread::sleep(Duration::from_millis(50)); + } + + let pid = helper.id(); + assert!( + pid_is_alive(pid), + "helper process must be alive after spawn" + ); + + // The exe-stem gate must fail closed: this PID is very much alive, but + // its image is the *test* binary, not `fbuild-daemon` — any legacy pid + // file / claim reader that found this PID must refuse to signal it. + assert!( + !pid_exe_stem_matches(pid, DAEMON_EXE_STEM), + "a non-daemon process must never pass the fbuild-daemon exe-stem gate" + ); + + // Write the PID into a temp-dir stand-in for the legacy stable pid file + // (`fbuild_paths::get_daemon_pid_file()`), mirroring what a pre-#1159 + // daemon (or an unrelated process that inherited a recycled PID) would + // leave behind. + let temp = tempfile::tempdir().expect("tempdir"); + let legacy_pid_file = temp.path().join("fbuild_daemon.pid"); + std::fs::write(&legacy_pid_file, pid.to_string()).expect("write legacy pid file"); + + // Simulate the legacy-sweep read path: read the pid back, verify + // liveness + exe stem BEFORE ever considering a signal. Because the + // stem check fails, nothing here is allowed to call `terminate_pid`. + let read_back: u32 = std::fs::read_to_string(&legacy_pid_file) + .expect("read legacy pid file") + .trim() + .parse() + .expect("pid file contains a u32"); + assert_eq!(read_back, pid); + let should_signal = pid_is_alive(read_back) && pid_exe_stem_matches(read_back, DAEMON_EXE_STEM); + assert!( + !should_signal, + "gate must refuse to signal a pid whose exe stem doesn't match fbuild-daemon" + ); + + // Positive proof: we did NOT call terminate_pid, so the process must + // still be alive. + assert!( + pid_is_alive(pid), + "helper process must remain alive — the exe-stem gate must have kept it un-signalled" + ); + + // Now kill it ourselves (test cleanup, NOT through the gated path) and + // confirm liveness correctly flips to false afterward. + hard_kill(&helper); + let exited = wait_with_timeout(&mut helper, Duration::from_secs(15)); + assert!( + exited, + "helper process did not exit within 15s of hard-kill" + ); + assert!( + !pid_is_alive(pid), + "helper process must be reported dead after being killed" + ); + assert!( + !pid_exe_stem_matches(pid, DAEMON_EXE_STEM), + "gate must remain closed for a dead pid too (fails closed either way)" + ); +} + +fn free_port() -> u16 { + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind ephemeral port"); + listener.local_addr().expect("local addr").port() +} + +/// Mirrors `fbuild_paths::get_fbuild_root()` / `get_daemon_dir()` / +/// `daemon_ownership::root_owner_lock_path()` layout (`/.fbuild/dev/ +/// daemon/root-owner.lock`) under an isolated HOME, computed WITHOUT +/// touching this test process's own environment — env vars are +/// process-global and the test suite runs multi-threaded, so mutating +/// `std::env` here would race other tests. The child `fbuild-daemon` +/// process gets `HOME`/`USERPROFILE` via `Command::env` instead, which is +/// scoped to just that child. +fn root_owner_lock_path_for(temp_home: &Path) -> PathBuf { + temp_home + .join(".fbuild") + .join("dev") + .join("daemon") + .join("root-owner.lock") +} + +async fn wait_for_http_health(port: u16, timeout: Duration) -> bool { + let deadline = Instant::now() + timeout; + let url = format!("http://127.0.0.1:{port}/health"); + while Instant::now() < deadline { + if let Ok(resp) = reqwest::get(&url).await { + if resp.status().is_success() { + return true; + } + } + tokio::time::sleep(Duration::from_millis(200)).await; + } + false +} + +#[tokio::test] +#[ignore = "spawns real fbuild-daemon (#1159)"] +async fn real_daemon_root_ownership_released_on_kill() { + let temp_home = tempfile::tempdir().expect("temp home"); + let port = free_port(); + let bin = env!("CARGO_BIN_EXE_fbuild-daemon"); + let home_key = if cfg!(windows) { "USERPROFILE" } else { "HOME" }; + + // allow-direct-spawn: test driver spawns the real fbuild-daemon binary under test. + let mut daemon = Command::new(bin) + .env(home_key, temp_home.path()) + .env("FBUILD_DEV_MODE", "1") + .env("FBUILD_DAEMON_PORT", port.to_string()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("spawn real fbuild-daemon"); + + let lock_path = root_owner_lock_path_for(temp_home.path()); + + // Generous bound: cold zccache init under a fresh, isolated HOME can + // take several seconds. + let healthy = wait_for_http_health(port, Duration::from_secs(30)).await; + assert!(healthy, "daemon never became healthy on port {port}"); + + // By the time /health answers, main.rs has already passed root- + // ownership acquisition: it happens before `CompileBackend::start()`, + // which in turn happens before the router (and therefore `/health`) + // is even constructed. + let blocked = RootOwnershipGuard::try_acquire_at(&lock_path).expect("try_acquire_at io"); + assert!( + blocked.is_none(), + "root-owner.lock must be held by the live daemon" + ); + + hard_kill(&daemon); + let exited = wait_with_timeout(&mut daemon, Duration::from_secs(30)); + assert!(exited, "daemon did not exit within 30s of hard-kill"); + + // Abrupt owner death must release ownership — the OS releases the file + // lock on process exit (including SIGKILL/TerminateProcess) — poll with + // a deadline rather than assuming instantaneous release. + let deadline = Instant::now() + Duration::from_secs(15); + let mut reacquired = None; + while Instant::now() < deadline { + if let Ok(Some(guard)) = RootOwnershipGuard::try_acquire_at(&lock_path) { + reacquired = Some(guard); + break; + } + std::thread::sleep(Duration::from_millis(200)); + } + assert!( + reacquired.is_some(), + "root-owner.lock must become acquirable again after the owning daemon is killed" + ); +} diff --git a/crates/fbuild-paths/Cargo.toml b/crates/fbuild-paths/Cargo.toml index c4711eb22..64fb28598 100644 --- a/crates/fbuild-paths/Cargo.toml +++ b/crates/fbuild-paths/Cargo.toml @@ -11,3 +11,8 @@ publish = false [dependencies] fbuild-core = { path = "../fbuild-core" } +serde = { workspace = true } +serde_json = { workspace = true } + +[dev-dependencies] +tempfile = { workspace = true } diff --git a/crates/fbuild-paths/README.md b/crates/fbuild-paths/README.md index 1934290fc..2edd54f46 100644 --- a/crates/fbuild-paths/README.md +++ b/crates/fbuild-paths/README.md @@ -8,6 +8,7 @@ Single source of truth for all `.fbuild` directory paths, with dev/prod isolatio - `get_fbuild_root()` -- Returns `~/.fbuild/dev` or `~/.fbuild/prod` based on mode - `get_other_fbuild_root()` -- Returns the opposite mode's root (for cross-mode daemon discovery) - `get_daemon_dir()` / `get_daemon_pid_file()` / `get_daemon_port_file()` / `get_daemon_log_file()` / `get_daemon_status_file()` -- Daemon file paths +- `daemon_ownership` module -- `RootOwnershipGuard` (version-blind, per-cache-root exclusive lock at `root-owner.lock`, held by the daemon for its whole lifetime; `fbuild clean cache` takes it exclusively before deleting the zccache store), `SpawnLockGuard` (`spawn.lock` single-flight election so concurrent CLI spawns don't race), and `OwnerClaim`/`write_owner_claim()`/`read_owner_claim()`/`remove_owner_claim()` (a `root-owner.json` claim recording pid/exe/version/mode/cache_root_key/port, written after the daemon acquires ownership and knows its port; never authoritative on its own — always verified against pid liveness + exe identity before acting on it) - `get_daemon_port()` -- Port resolution with four-level priority: env var, current mode port file, cross-mode port file, default (8865 dev / 8765 prod) - `get_daemon_url()` -- Daemon HTTP URL (`http://127.0.0.1:{port}`) - `get_cache_root()` -- Global cache dir (`FBUILD_CACHE_DIR` override or `~/.fbuild/{mode}/cache`) @@ -19,3 +20,4 @@ Single source of truth for all `.fbuild` directory paths, with dev/prod isolatio ## Modules - **lib.rs** -- All path functions in a single module +- **daemon_ownership** -- `RootOwnershipGuard`, `SpawnLockGuard`, and `OwnerClaim` (root-owner.lock / spawn.lock / root-owner.json) — see Key Types above diff --git a/crates/fbuild-paths/src/daemon_ownership.rs b/crates/fbuild-paths/src/daemon_ownership.rs new file mode 100644 index 000000000..c56ac7024 --- /dev/null +++ b/crates/fbuild-paths/src/daemon_ownership.rs @@ -0,0 +1,335 @@ +//! Soldr-style daemon lifecycle ownership for the compiler cache root +//! (FastLED/fbuild#1154 / #1159). +//! +//! Two cross-process file locks, both scoped to `get_daemon_dir()` and +//! deliberately outside zccache's own object-cache synchronization: +//! +//! - [`RootOwnershipGuard`] — version-blind, per-cache-root exclusive +//! ownership. A `fbuild-daemon` holds this for its entire lifetime. +//! `fbuild clean cache` takes it exclusively (after stopping every daemon +//! it can find) as positive proof no daemon is touching the cache root +//! before deleting it. +//! - [`SpawnLockGuard`] — spawn-herd single flight, so N concurrent `fbuild` +//! invocations that all decide to spawn a daemon don't stampede. +//! +//! Semantics copied from soldr's `RootOwnershipGuard` / `acquire_spawn_lock` +//! (`.extern-repos/soldr/crates/soldr-daemon/src/daemon/lifecycle.rs`). +//! Locking itself is NOT reimplemented here — both guards are thin wrappers +//! around [`fbuild_core::file_lock::try_acquire`] so there is exactly one +//! cross-process file-lock primitive in the workspace. +//! +//! NEVER delete or truncate a lock file to "unlock" it — the OS releases +//! the lock when the holding process exits (including a hard kill); the +//! file itself is not the lock. + +use std::path::{Path, PathBuf}; + +use fbuild_core::file_lock::{self, FileLockGuard, FileLockMode}; + +use crate::get_daemon_dir; + +/// Expected executable stem of the fbuild daemon binary. Used with +/// `fbuild_core::process_identity::pid_exe_stem_matches` to verify a PID +/// found in a legacy pid file / claim / status file is actually an +/// fbuild-daemon before it is ever signalled. +pub const DAEMON_EXE_STEM: &str = "fbuild-daemon"; + +const ROOT_OWNER_LOCK_NAME: &str = "root-owner.lock"; +const SPAWN_LOCK_NAME: &str = "spawn.lock"; +const OWNER_CLAIM_NAME: &str = "root-owner.json"; + +/// Version-blind, per-cache-root exclusive ownership lock. +/// +/// Lives at [`root_owner_lock_path`]. A daemon acquires this once at +/// startup (before doing any cache-mutating work) and holds it for its +/// entire lifetime; `fbuild clean cache` takes it exclusively — after +/// stopping every daemon it can find — as the final, positive proof that +/// no daemon (of ANY version) still owns the cache root, before deleting +/// it. +#[derive(Debug)] +pub struct RootOwnershipGuard { + _guard: FileLockGuard, +} + +impl RootOwnershipGuard { + /// Try to acquire root ownership at the default path + /// ([`root_owner_lock_path`]). `Ok(None)` means another process + /// currently holds it. + pub fn try_acquire() -> std::io::Result> { + Self::try_acquire_at(&root_owner_lock_path()) + } + + /// Test seam: try to acquire root ownership at an arbitrary path. + pub fn try_acquire_at(path: &Path) -> std::io::Result> { + Ok(file_lock::try_acquire(path, FileLockMode::Exclusive)?.map(|_guard| Self { _guard })) + } +} + +/// Spawn-herd single-flight lock. Lives at [`spawn_lock_path`]. +/// +/// Errors while opening/locking the file are treated as "no lock +/// available" (`None`) — a broken filesystem must never gate progress; the +/// caller falls back to the herd-spawn path (poll health, retry) rather +/// than blocking forever on a lock that can't be taken. +#[derive(Debug)] +pub struct SpawnLockGuard { + _guard: FileLockGuard, +} + +/// Try to acquire the spawn-herd lock at the default path +/// ([`spawn_lock_path`]). +pub fn try_acquire_spawn_lock() -> Option { + try_acquire_spawn_lock_at(&spawn_lock_path()) +} + +/// Test seam: try to acquire the spawn-herd lock at an arbitrary path. +pub fn try_acquire_spawn_lock_at(path: &Path) -> Option { + file_lock::try_acquire(path, FileLockMode::Exclusive) + .ok() + .flatten() + .map(|_guard| SpawnLockGuard { _guard }) +} + +/// Path to the root-ownership lock file. +pub fn root_owner_lock_path() -> PathBuf { + get_daemon_dir().join(ROOT_OWNER_LOCK_NAME) +} + +/// Path to the spawn-herd lock file. +pub fn spawn_lock_path() -> PathBuf { + get_daemon_dir().join(SPAWN_LOCK_NAME) +} + +/// Path to the [`OwnerClaim`] JSON file. +pub fn owner_claim_path() -> PathBuf { + get_daemon_dir().join(OWNER_CLAIM_NAME) +} + +/// Advisory claim written by the daemon after it has acquired root +/// ownership and knows its bound port. Removed at graceful shutdown. +/// +/// **Never authoritative on its own.** This file can go stale (crash, +/// `SIGKILL`, power loss) exactly like any other PID file — always verify +/// `pid` liveness AND its exe stem (via +/// `fbuild_core::process_identity::pid_exe_stem_matches(pid, DAEMON_EXE_STEM)`) +/// before treating a claim as describing a live daemon, and never signal a +/// PID from a claim without that verification. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct OwnerClaim { + pub pid: u32, + pub exe: PathBuf, + /// `env!("CARGO_PKG_VERSION")` of the daemon that wrote the claim. + pub version: String, + /// `"dev"` | `"prod"`. + pub mode: String, + /// `DaemonCacheIdentity.cache_root_key` of the daemon that wrote the + /// claim — lets a reader confirm the claim actually describes the + /// cache root it's about to touch. + pub cache_root_key: String, + pub port: u16, +} + +/// Write (or overwrite) the owner claim file. +pub fn write_owner_claim(claim: &OwnerClaim) -> std::io::Result<()> { + let path = owner_claim_path(); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let json = serde_json::to_string_pretty(claim) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + std::fs::write(path, json) +} + +/// Read the owner claim file. Absent or malformed (bad JSON, missing +/// field, wrong shape) => `None`. Does NOT verify liveness — see the +/// [`OwnerClaim`] doc comment. +pub fn read_owner_claim() -> Option { + let raw = std::fs::read_to_string(owner_claim_path()).ok()?; + serde_json::from_str(&raw).ok() +} + +/// Remove the owner claim file. Best-effort: a missing file is not an +/// error (idempotent). +pub fn remove_owner_claim() { + let _ = std::fs::remove_file(owner_claim_path()); +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn root_ownership_is_exclusive_within_process() { + let temp = TempDir::new().expect("tempdir"); + let path = temp.path().join("root-owner.lock"); + + let first = RootOwnershipGuard::try_acquire_at(&path) + .expect("first acquire io") + .expect("first acquire must succeed"); + let second = RootOwnershipGuard::try_acquire_at(&path).expect("second acquire io"); + assert!( + second.is_none(), + "a second exclusive acquire while the first is held must return None" + ); + drop(first); + let third = RootOwnershipGuard::try_acquire_at(&path) + .expect("third acquire io") + .expect("lock must be available again after the holder drops"); + drop(third); + } + + #[test] + fn spawn_lock_is_exclusive_within_process() { + let temp = TempDir::new().expect("tempdir"); + let path = temp.path().join("spawn.lock"); + + let first = try_acquire_spawn_lock_at(&path).expect("first acquire"); + let second = try_acquire_spawn_lock_at(&path); + assert!(second.is_none(), "second acquire while held must be None"); + drop(first); + let third = try_acquire_spawn_lock_at(&path); + assert!(third.is_some(), "lock must be available after release"); + } + + /// Soldr pattern (`spawn_lock_serializes_concurrent_threads`): fire a + /// pile of threads at the same lock file and confirm the lock actually + /// serializes them — not all of them, and not zero of them, acquire. + #[test] + fn spawn_lock_serializes_concurrent_threads() { + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::{Arc, Barrier}; + + let temp = TempDir::new().expect("tempdir"); + let path = Arc::new(temp.path().join("spawn.lock")); + const THREADS: usize = 16; + let barrier = Arc::new(Barrier::new(THREADS)); + let success_count = Arc::new(AtomicUsize::new(0)); + + let mut handles = Vec::with_capacity(THREADS); + for _ in 0..THREADS { + let path = path.clone(); + let barrier = barrier.clone(); + let counter = success_count.clone(); + handles.push(std::thread::spawn(move || { + barrier.wait(); + if let Some(guard) = try_acquire_spawn_lock_at(&path) { + counter.fetch_add(1, Ordering::Relaxed); + std::thread::sleep(std::time::Duration::from_millis(10)); + drop(guard); + } + })); + } + for h in handles { + h.join().expect("thread join"); + } + let count = success_count.load(Ordering::Relaxed); + assert!(count >= 1, "at least one thread must acquire; got {count}"); + assert!( + count < THREADS, + "lock must serialize acquisition; got {count} of {THREADS}" + ); + } + + #[test] + fn owner_claim_round_trips() { + let temp = TempDir::new().expect("tempdir"); + // Point HOME/USERPROFILE-independent paths at a temp cache dir by + // writing/reading through the raw serde round trip directly rather + // than the real `owner_claim_path()` (which is process-global via + // `get_daemon_dir`), keeping this test independent of env state. + let path = temp.path().join(OWNER_CLAIM_NAME); + let claim = OwnerClaim { + pid: 4242, + exe: PathBuf::from("/usr/local/bin/fbuild-daemon"), + version: "9.9.9".to_string(), + mode: "dev".to_string(), + cache_root_key: "abc123".to_string(), + port: 54321, + }; + let json = serde_json::to_string_pretty(&claim).expect("serialize"); + std::fs::write(&path, json).expect("write"); + + let raw = std::fs::read_to_string(&path).expect("read"); + let round_tripped: OwnerClaim = serde_json::from_str(&raw).expect("deserialize"); + assert_eq!(round_tripped.pid, claim.pid); + assert_eq!(round_tripped.exe, claim.exe); + assert_eq!(round_tripped.version, claim.version); + assert_eq!(round_tripped.mode, claim.mode); + assert_eq!(round_tripped.cache_root_key, claim.cache_root_key); + assert_eq!(round_tripped.port, claim.port); + } + + #[test] + fn malformed_owner_claim_json_fails_closed() { + let temp = TempDir::new().expect("tempdir"); + let path = temp.path().join(OWNER_CLAIM_NAME); + std::fs::write(&path, "not valid json").expect("write"); + let raw = std::fs::read_to_string(&path).expect("read"); + assert!( + serde_json::from_str::(&raw).is_err(), + "malformed claim JSON must fail to parse, never silently default" + ); + } + + #[test] + fn read_owner_claim_absent_file_is_none() { + // `read_owner_claim()` reads from the process-global + // `owner_claim_path()`; simulate the "absent" branch directly + // against a path we know doesn't exist, mirroring the function's + // own absent/malformed => None contract. + let temp = TempDir::new().expect("tempdir"); + let missing = temp.path().join("does-not-exist.json"); + assert!(std::fs::read_to_string(&missing).is_err()); + } + + /// Soldr pattern (`root_ownership_is_version_blind_across_processes`): + /// verify exclusivity holds across REAL processes, not just within one. + /// The driver acquires the lock, spawns this test binary re-invoked as + /// a subprocess probe (`--ignored --exact `), confirms it is + /// blocked, drops the lock, then confirms a second subprocess probe + /// succeeds. + #[test] + #[ignore = "subprocess helper for root_ownership_is_version_blind_across_processes"] + fn subprocess_probe_root_owner() { + let root = std::env::var_os("FBUILD_TEST_ROOT_OWNER_PATH").expect("test lock path"); + let expected = std::env::var("FBUILD_TEST_ROOT_OWNER_EXPECT").expect("expectation"); + let path = PathBuf::from(root); + let acquired = RootOwnershipGuard::try_acquire_at(&path) + .expect("root ownership probe io") + .is_some(); + assert_eq!(acquired, expected == "acquired"); + } + + #[test] + fn root_ownership_is_version_blind_across_processes() { + let temp = TempDir::new().expect("tempdir"); + let path = temp.path().join("root-owner.lock"); + + let run_probe = |expected: &str| { + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--ignored", + "--exact", + "daemon_ownership::tests::subprocess_probe_root_owner", + "--nocapture", + ]) + .env("FBUILD_TEST_ROOT_OWNER_PATH", &path) + .env("FBUILD_TEST_ROOT_OWNER_EXPECT", expected) + .output() + .expect("run subprocess probe"); + assert!( + output.status.success(), + "subprocess root-owner probe failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + }; + + let owner = RootOwnershipGuard::try_acquire_at(&path) + .expect("acquire io") + .expect("parent must own the fresh lock file"); + run_probe("blocked"); + drop(owner); + run_probe("acquired"); + } +} diff --git a/crates/fbuild-paths/src/lib.rs b/crates/fbuild-paths/src/lib.rs index ed962d6dc..affd4df2a 100644 --- a/crates/fbuild-paths/src/lib.rs +++ b/crates/fbuild-paths/src/lib.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use fbuild_core::BuildProfile; +pub mod daemon_ownership; pub mod running_process; /// Check if running in development mode. diff --git a/crates/fbuild-serial/src/ports.rs b/crates/fbuild-serial/src/ports.rs index b32608ac5..62db717c5 100644 --- a/crates/fbuild-serial/src/ports.rs +++ b/crates/fbuild-serial/src/ports.rs @@ -274,7 +274,7 @@ mod imp { use serialport::{SerialPortInfo, SerialPortType, UsbPortInfo}; use windows_sys::Win32::Devices::DeviceAndDriverInstallation::{ CM_Get_DevNode_Status, CM_Get_Device_IDW, CM_Get_Parent, CR_NO_SUCH_DEVINST, CR_SUCCESS, - DICS_FLAG_GLOBAL, DIGCF_ALLCLASSES, DIGCF_PRESENT, DIREG_DEV, GUID_DEVCLASS_USB, HDEVINFO, + DICS_FLAG_GLOBAL, DIGCF_ALLCLASSES, DIGCF_PRESENT, DIREG_DEV, HDEVINFO, MAX_DEVICE_ID_LEN, SP_DEVINFO_DATA, SPDRP_CLASS, SPDRP_FRIENDLYNAME, SPDRP_HARDWAREID, SPDRP_LOCATION_INFORMATION, SPDRP_MFG, SetupDiClassGuidsFromNameW, SetupDiDestroyDeviceInfoList, SetupDiEnumDeviceInfo, SetupDiGetClassDevsW, diff --git a/docs/architecture/runtime.md b/docs/architecture/runtime.md index 2db5d6885..109338e13 100644 --- a/docs/architecture/runtime.md +++ b/docs/architecture/runtime.md @@ -11,7 +11,8 @@ The daemon runs on a multi-threaded tokio runtime: ## Lock Strategy -All synchronization is in-memory within the daemon process. No file-based locks. +Request-level synchronization is in-memory within the daemon process — no +file-based locks are used for builds, serial, deploy, or config state. | Resource | Lock Type | Scope | |----------|-----------|-------| @@ -21,6 +22,19 @@ All synchronization is in-memory within the daemon process. No file-based locks. | Device lease | Per-device RwLock | Exclusive (deploy) or shared (monitor) | | Config lock | Per-project Mutex | Prevents concurrent config changes | +The one exception is daemon startup/lifetime ownership itself, which is +process-level rather than request-level and so can't be arbitrated by an +in-memory manager (there may be no daemon process yet). `fbuild-paths`'s +`daemon_ownership` module (soldr-style, FastLED/fbuild#1159) provides a +version-blind `root-owner.lock` that a daemon holds for its whole lifetime, +plus a `spawn.lock` single-flight election so multiple CLI invocations racing +to start a daemon don't spawn duplicates. `fbuild clean cache` acquires +`root-owner.lock` exclusively (after verifying and displacing any daemon, +including legacy ones, that still owns it) before deleting the zccache store. +These are OS-released file locks, never manually broken or deleted, and they +never gate zccache object reads/writes — those stay in-memory-synchronized +inside zccache itself. + ## Error Recovery - **Daemon crash**: CLI detects connection failure, restarts daemon automatically diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 2aa6d3ed2..c39a548b6 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -42,17 +42,30 @@ Common options include `--clean`, `--jobs`, `--quick`, `--release`, Remove project outputs without compiling or deploying. `sketch` removes only the selected environment/profile build directory. `all` also removes the exact -matching reusable framework-cache entries. The operation is coordinated by the -daemon, so it waits for any build holding the project lock. +matching reusable framework-cache entries. `cache` stops the current-version +daemon and any other live `fbuild-daemon` processes that verifiably own the +same cache root — including legacy pre-#1159 daemons — then takes exclusive +ownership of the cache root, deletes only the active dev/prod mode's +`/zccache` compiler-object store, and restarts the daemon. +Installed packages, platforms, frameworks, toolchains, and downloaded fbuild +archives are retained. + +The command refuses `cache` while a build operation is active, so the compiler +store is never removed from underneath a build, and refuses if the running +daemon's identity or mode doesn't match the target cache root. It attempts to +restore daemon availability even if cache removal fails. ```bash fbuild clean sketch fbuild clean sketch examples/Blink -e uno --quick fbuild clean all -e esp32dev --release +fbuild clean cache examples/Blink -e uno --release ``` The scope is required; `--quick` and `--release` are mutually exclusive and -default to the release profile. +default to the release profile. Unlike `sketch` and `all`, `cache` has global +scope within the active fbuild mode and affects compiler-cache hits for every +project using that mode. ### `fbuild deploy`