From bc8ba93b57592df07d797479dff0079a30e5db2c Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 22:31:06 -0700 Subject: [PATCH 01/12] feat(cache): add two-phase disk cache with LRU GC and SQLite index (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `disk_cache` module to fbuild-packages implementing: - Two-phase layout: archives/ (downloads) + installed/ (extracted) - Crash-safe SQLite index in WAL mode with schema migrations - LRU garbage collection with configurable size budgets - Auto-scaled budgets: min(15 GiB, 5% disk) per phase - RAII Lease guard to pin entries during builds - Filesystem reconciliation for crash recovery - 47 tests covering index CRUD, GC eviction, leases, budgets, paths Closes #20 (initial implementation — callers not yet migrated) --- Cargo.lock | 68 ++ Cargo.toml | 1 + crates/fbuild-packages/Cargo.toml | 1 + .../fbuild-packages/src/disk_cache/README.md | 15 + .../fbuild-packages/src/disk_cache/budget.rs | 257 ++++++ crates/fbuild-packages/src/disk_cache/gc.rs | 413 +++++++++ .../fbuild-packages/src/disk_cache/index.rs | 797 ++++++++++++++++++ .../fbuild-packages/src/disk_cache/lease.rs | 117 +++ crates/fbuild-packages/src/disk_cache/mod.rs | 343 ++++++++ .../fbuild-packages/src/disk_cache/paths.rs | 223 +++++ crates/fbuild-packages/src/lib.rs | 2 + 11 files changed, 2237 insertions(+) create mode 100644 crates/fbuild-packages/src/disk_cache/README.md create mode 100644 crates/fbuild-packages/src/disk_cache/budget.rs create mode 100644 crates/fbuild-packages/src/disk_cache/gc.rs create mode 100644 crates/fbuild-packages/src/disk_cache/index.rs create mode 100644 crates/fbuild-packages/src/disk_cache/lease.rs create mode 100644 crates/fbuild-packages/src/disk_cache/mod.rs create mode 100644 crates/fbuild-packages/src/disk_cache/paths.rs diff --git a/Cargo.lock b/Cargo.lock index fb8d9fbe..6c32b6ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,6 +8,18 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" +[[package]] +name = "ahash" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" +dependencies = [ + "cfg-if", + "once_cell", + "version_check", + "zerocopy", +] + [[package]] name = "aho-corasick" version = "1.1.4" @@ -488,6 +500,18 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "fallible-iterator" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" + +[[package]] +name = "fallible-streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" + [[package]] name = "fastrand" version = "2.3.0" @@ -626,6 +650,7 @@ dependencies = [ "fbuild-paths", "flate2", "reqwest", + "rusqlite", "semver", "serde", "serde_json", @@ -883,6 +908,9 @@ name = "hashbrown" version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" +dependencies = [ + "ahash", +] [[package]] name = "hashbrown" @@ -899,6 +927,15 @@ version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" +[[package]] +name = "hashlink" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ba4ff7128dee98c7dc9794b6a411377e1404dba1c97deb8d1a55297bd25d8af" +dependencies = [ + "hashbrown 0.14.5", +] + [[package]] name = "heck" version = "0.5.0" @@ -1260,6 +1297,17 @@ dependencies = [ "redox_syscall 0.7.3", ] +[[package]] +name = "libsqlite3-sys" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c10584274047cb335c23d3e61bcef8e323adae7c5c8c760540f73610177fc3f" +dependencies = [ + "cc", + "pkg-config", + "vcpkg", +] + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -1830,6 +1878,20 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rusqlite" +version = "0.31.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b838eba278d213a8beaf485bd313fd580ca4505a00d5871caeb1457c55322cae" +dependencies = [ + "bitflags 2.11.0", + "fallible-iterator", + "fallible-streaming-iterator", + "hashlink", + "libsqlite3-sys", + "smallvec", +] + [[package]] name = "rustc-hash" version = "2.1.2" @@ -2551,6 +2613,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" +[[package]] +name = "vcpkg" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" + [[package]] name = "version_check" version = "0.9.5" diff --git a/Cargo.toml b/Cargo.toml index f9fab831..54838493 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,3 +57,4 @@ dashmap = "6" blake3 = "1" mimalloc = "0.1" object = "0.36" +rusqlite = { version = "0.31", features = ["bundled"] } diff --git a/crates/fbuild-packages/Cargo.toml b/crates/fbuild-packages/Cargo.toml index 634beb98..ad9c7d8f 100644 --- a/crates/fbuild-packages/Cargo.toml +++ b/crates/fbuild-packages/Cargo.toml @@ -25,6 +25,7 @@ xz2 = { workspace = true } bzip2 = { workspace = true } zstd = { workspace = true } semver = { workspace = true } +rusqlite = { workspace = true } [dev-dependencies] tempfile = { workspace = true } diff --git a/crates/fbuild-packages/src/disk_cache/README.md b/crates/fbuild-packages/src/disk_cache/README.md new file mode 100644 index 00000000..9b627e78 --- /dev/null +++ b/crates/fbuild-packages/src/disk_cache/README.md @@ -0,0 +1,15 @@ +# disk_cache + +Two-phase disk cache with LRU garbage collection, sized budgets, and crash-safe SQLite index. + +Separates downloaded archives from installed (extracted) content, allowing cheap-to-rehydrate +installed directories to be evicted before expensive-to-fetch archive blobs. + +## Modules + +- `mod.rs` - Public `DiskCache` facade +- `paths.rs` - Sole source of cache path construction (archives, installed, staging) +- `index.rs` - SQLite open/migrate/query/touch/reconcile +- `budget.rs` - Size accounting, watermark math, auto-scaling from disk space +- `gc.rs` - Eviction loop, lease reaping, lock handling +- `lease.rs` - RAII `Lease` guard that pins entries during builds diff --git a/crates/fbuild-packages/src/disk_cache/budget.rs b/crates/fbuild-packages/src/disk_cache/budget.rs new file mode 100644 index 00000000..87e08116 --- /dev/null +++ b/crates/fbuild-packages/src/disk_cache/budget.rs @@ -0,0 +1,257 @@ +//! Size budgets and auto-scaling for the disk cache GC. +//! +//! Budgets auto-scale at startup based on available disk space, +//! capped at absolute maximums. All values are overridable via environment variables. + +use std::path::Path; + +/// Default absolute caps. +const DEFAULT_ARCHIVE_BUDGET: u64 = 15 * 1024 * 1024 * 1024; // 15 GiB +const DEFAULT_INSTALLED_BUDGET: u64 = 15 * 1024 * 1024 * 1024; // 15 GiB +const DEFAULT_HIGH_WATERMARK: u64 = 30 * 1024 * 1024 * 1024; // 30 GiB + +/// Percentage of total disk for per-phase budgets. +const PHASE_DISK_PERCENT: f64 = 0.05; // 5% + +/// Percentage of total disk for combined high watermark. +const COMBINED_DISK_PERCENT: f64 = 0.10; // 10% + +/// Low watermark is 80% of high watermark — GC stops here. +const LOW_WATERMARK_RATIO: f64 = 0.80; + +/// Computed cache budgets. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CacheBudget { + pub archive_budget: u64, + pub installed_budget: u64, + pub high_watermark: u64, + pub low_watermark: u64, +} + +impl CacheBudget { + /// Compute budgets based on disk size, respecting env overrides. + pub fn compute(cache_root: &Path) -> Self { + let total_disk = get_total_disk_space(cache_root); + Self::compute_with_disk_size(total_disk) + } + + /// Compute budgets with a known disk size (for testing). + pub fn compute_with_disk_size(total_disk: u64) -> Self { + let phase_by_disk = (total_disk as f64 * PHASE_DISK_PERCENT) as u64; + let combined_by_disk = (total_disk as f64 * COMBINED_DISK_PERCENT) as u64; + + let archive_budget = parse_env_size("FBUILD_CACHE_ARCHIVE_BUDGET") + .unwrap_or_else(|| DEFAULT_ARCHIVE_BUDGET.min(phase_by_disk)); + + let installed_budget = parse_env_size("FBUILD_CACHE_INSTALLED_BUDGET") + .unwrap_or_else(|| DEFAULT_INSTALLED_BUDGET.min(phase_by_disk)); + + let high_watermark = parse_env_size("FBUILD_CACHE_HIGH_WATERMARK") + .unwrap_or_else(|| DEFAULT_HIGH_WATERMARK.min(combined_by_disk)); + + let low_watermark = (high_watermark as f64 * LOW_WATERMARK_RATIO) as u64; + + Self { + archive_budget, + installed_budget, + high_watermark, + low_watermark, + } + } +} + +/// Parse a human-readable size from an environment variable. +/// Supports suffixes: B, K, KB, M, MB, G, GB, T, TB (case-insensitive). +fn parse_env_size(var: &str) -> Option { + let val = std::env::var(var).ok()?; + parse_human_size(&val) +} + +/// Parse a human-readable size string like "15G", "500M", "1024". +pub fn parse_human_size(s: &str) -> Option { + let s = s.trim(); + if s.is_empty() { + return None; + } + + // Find where digits end and suffix begins + let (num_part, suffix) = { + let idx = s + .find(|c: char| !c.is_ascii_digit() && c != '.') + .unwrap_or(s.len()); + (&s[..idx], s[idx..].trim().to_uppercase()) + }; + + let num: f64 = num_part.parse().ok()?; + + let multiplier: u64 = match suffix.as_str() { + "" | "B" => 1, + "K" | "KB" | "KIB" => 1024, + "M" | "MB" | "MIB" => 1024 * 1024, + "G" | "GB" | "GIB" => 1024 * 1024 * 1024, + "T" | "TB" | "TIB" => 1024 * 1024 * 1024 * 1024, + _ => return None, + }; + + Some((num * multiplier as f64) as u64) +} + +/// Get total disk space for the filesystem containing the given path. +fn get_total_disk_space(path: &Path) -> u64 { + #[cfg(windows)] + { + get_total_disk_space_windows(path) + } + #[cfg(unix)] + { + get_total_disk_space_unix(path) + } + #[cfg(not(any(unix, windows)))] + { + let _ = path; + // Fallback: assume 500 GB + 500 * 1024 * 1024 * 1024 + } +} + +#[cfg(windows)] +fn get_total_disk_space_windows(path: &Path) -> u64 { + use std::process::Command; + // Use PowerShell to get disk info + let path_str = path.to_string_lossy(); + let drive = if path_str.len() >= 2 && path_str.as_bytes()[1] == b':' { + &path_str[..2] + } else { + "C:" + }; + + Command::new("powershell") + .args([ + "-NoProfile", + "-Command", + &format!( + "(Get-PSDrive -Name '{}').Used + (Get-PSDrive -Name '{}').Free", + &drive[..1], + &drive[..1] + ), + ]) + .output() + .ok() + .and_then(|o| { + String::from_utf8_lossy(&o.stdout) + .trim() + .parse::() + .ok() + }) + .unwrap_or(500 * 1024 * 1024 * 1024) // fallback 500 GB +} + +#[cfg(unix)] +fn get_total_disk_space_unix(path: &Path) -> u64 { + use std::process::Command; + // Use `df` to get total space + let output = Command::new("df") + .args(["-B1", "--output=size"]) + .arg(path) + .output(); + + output + .ok() + .and_then(|o| { + let stdout = String::from_utf8_lossy(&o.stdout); + stdout + .lines() + .nth(1) // skip header + .and_then(|line| line.trim().parse::().ok()) + }) + .unwrap_or(500 * 1024 * 1024 * 1024) // fallback 500 GB +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_human_size_bytes() { + assert_eq!(parse_human_size("1024"), Some(1024)); + assert_eq!(parse_human_size("1024B"), Some(1024)); + } + + #[test] + fn test_parse_human_size_kilobytes() { + assert_eq!(parse_human_size("1K"), Some(1024)); + assert_eq!(parse_human_size("1KB"), Some(1024)); + assert_eq!(parse_human_size("2k"), Some(2048)); + } + + #[test] + fn test_parse_human_size_megabytes() { + assert_eq!(parse_human_size("1M"), Some(1024 * 1024)); + assert_eq!(parse_human_size("500M"), Some(500 * 1024 * 1024)); + assert_eq!(parse_human_size("1mb"), Some(1024 * 1024)); + } + + #[test] + fn test_parse_human_size_gigabytes() { + assert_eq!(parse_human_size("1G"), Some(1024 * 1024 * 1024)); + assert_eq!(parse_human_size("15G"), Some(15 * 1024 * 1024 * 1024)); + assert_eq!(parse_human_size("1gb"), Some(1024 * 1024 * 1024)); + } + + #[test] + fn test_parse_human_size_terabytes() { + assert_eq!(parse_human_size("1T"), Some(1024 * 1024 * 1024 * 1024)); + } + + #[test] + fn test_parse_human_size_empty() { + assert_eq!(parse_human_size(""), None); + assert_eq!(parse_human_size(" "), None); + } + + #[test] + fn test_parse_human_size_invalid() { + assert_eq!(parse_human_size("abc"), None); + assert_eq!(parse_human_size("10X"), None); + } + + #[test] + fn test_budget_autoscales_to_disk() { + // Small disk: 100 GB + let small_disk = 100 * 1024 * 1024 * 1024_u64; + let budget = CacheBudget::compute_with_disk_size(small_disk); + + // 5% of 100 GB = 5 GB < 15 GB cap, so should be 5 GB + assert_eq!(budget.archive_budget, 5 * 1024 * 1024 * 1024); + assert_eq!(budget.installed_budget, 5 * 1024 * 1024 * 1024); + + // 10% of 100 GB = 10 GB < 30 GB cap + assert_eq!(budget.high_watermark, 10 * 1024 * 1024 * 1024); + + // Low watermark = 80% of high + assert_eq!(budget.low_watermark, 8 * 1024 * 1024 * 1024); + } + + #[test] + fn test_budget_caps_on_large_disk() { + // Very large disk: 2 TB + let large_disk = 2 * 1024 * 1024 * 1024 * 1024_u64; + let budget = CacheBudget::compute_with_disk_size(large_disk); + + // 5% of 2 TB = 102 GB > 15 GB cap, so capped at 15 GB + assert_eq!(budget.archive_budget, 15 * 1024 * 1024 * 1024); + assert_eq!(budget.installed_budget, 15 * 1024 * 1024 * 1024); + + // 10% of 2 TB = 204 GB > 30 GB cap + assert_eq!(budget.high_watermark, 30 * 1024 * 1024 * 1024); + } + + #[test] + fn test_low_watermark_is_80_percent() { + let budget = CacheBudget::compute_with_disk_size(200 * 1024 * 1024 * 1024); + assert_eq!( + budget.low_watermark, + (budget.high_watermark as f64 * 0.80) as u64 + ); + } +} diff --git a/crates/fbuild-packages/src/disk_cache/gc.rs b/crates/fbuild-packages/src/disk_cache/gc.rs new file mode 100644 index 00000000..9ee0ff7c --- /dev/null +++ b/crates/fbuild-packages/src/disk_cache/gc.rs @@ -0,0 +1,413 @@ +//! Garbage collection for the two-phase disk cache. +//! +//! Eviction order (cheap to expensive): +//! 1. Installed directories (LRU-first, skip pinned/leased) +//! 2. Archive files (LRU-first, skip leased) +//! +//! GC stops at LOW_WATERMARK to avoid over-evicting. + +use super::budget::CacheBudget; +use super::index::CacheIndex; +use std::path::Path; + +/// Report of a GC run. +#[derive(Debug, Clone, Default)] +pub struct GcReport { + pub installed_evicted: u64, + pub installed_bytes_freed: u64, + pub archives_evicted: u64, + pub archive_bytes_freed: u64, + pub leases_reaped: usize, + pub orphan_files_removed: usize, + pub orphan_rows_cleaned: usize, +} + +impl GcReport { + pub fn total_bytes_freed(&self) -> u64 { + self.installed_bytes_freed + self.archive_bytes_freed + } +} + +impl std::fmt::Display for GcReport { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "GC: freed {} installed ({} bytes), {} archives ({} bytes), \ + reaped {} leases, {} orphan files, {} orphan rows", + self.installed_evicted, + self.installed_bytes_freed, + self.archives_evicted, + self.archive_bytes_freed, + self.leases_reaped, + self.orphan_files_removed, + self.orphan_rows_cleaned, + ) + } +} + +/// Run a full GC pass against the index and on-disk state. +pub fn run_gc(index: &CacheIndex, budget: &CacheBudget) -> rusqlite::Result { + let leases_reaped = index.reap_dead_leases()?; + let mut report = GcReport { + leases_reaped, + ..Default::default() + }; + + // Step 1: evict installed directories if over budget + let mut installed_bytes = index.total_installed_bytes()? as u64; + let total_bytes = index.total_archive_bytes()? as u64 + installed_bytes; + + if installed_bytes > budget.installed_budget || total_bytes > budget.high_watermark { + let target = budget.low_watermark.min(budget.installed_budget); + let entries = index.lru_installed_entries(1000)?; + + for entry in entries { + if installed_bytes <= target { + break; + } + let bytes = entry.installed_bytes.unwrap_or(0) as u64; + + // Remove the directory from disk + if let Some(ref path) = entry.installed_path { + let full_path = index.cache_root().join(path); + if full_path.exists() { + if let Err(e) = std::fs::remove_dir_all(&full_path) { + tracing::warn!("GC: failed to remove {}: {}", full_path.display(), e); + continue; + } + } + } + + index.clear_installed(entry.id)?; + installed_bytes = installed_bytes.saturating_sub(bytes); + report.installed_evicted += 1; + report.installed_bytes_freed += bytes; + } + } + + // Step 2: evict archives if over budget + let mut archive_bytes = index.total_archive_bytes()? as u64; + + if archive_bytes > budget.archive_budget { + let entries = index.lru_archive_entries(1000)?; + + for entry in entries { + if archive_bytes <= budget.archive_budget { + break; + } + let bytes = entry.archive_bytes.unwrap_or(0) as u64; + + // Remove the archive file from disk + if let Some(ref path) = entry.archive_path { + let full_path = index.cache_root().join(path); + if full_path.exists() { + if let Err(e) = remove_path(&full_path) { + tracing::warn!("GC: failed to remove {}: {}", full_path.display(), e); + continue; + } + } + } + + index.clear_archive(entry.id)?; + archive_bytes = archive_bytes.saturating_sub(bytes); + report.archives_evicted += 1; + report.archive_bytes_freed += bytes; + + // If both archive and installed are gone, delete the row + if entry.installed_path.is_none() { + index.delete_entry(entry.id)?; + } + } + } + + Ok(report) +} + +/// Reconcile the index against the filesystem. +/// +/// - Entries pointing to missing paths → null out the column +/// - Files on disk with no index entry → delete (orphans from crashes) +pub fn reconcile(index: &CacheIndex) -> rusqlite::Result { + let mut report = GcReport::default(); + let cache_root = index.cache_root().to_path_buf(); + + // Phase 1: check all entries, null out paths that don't exist on disk + let entries = index.all_entries()?; + for entry in &entries { + if let Some(ref path) = entry.archive_path { + let full = cache_root.join(path); + if !full.exists() { + index.clear_archive(entry.id)?; + report.orphan_rows_cleaned += 1; + } + } + if let Some(ref path) = entry.installed_path { + let full = cache_root.join(path); + if !full.exists() { + index.clear_installed(entry.id)?; + report.orphan_rows_cleaned += 1; + } else { + // Check for .install_complete sentinel + let sentinel = super::paths::install_complete_sentinel(&full); + if !sentinel.exists() { + // Partial install — remove it + let _ = std::fs::remove_dir_all(&full); + index.clear_installed(entry.id)?; + report.orphan_files_removed += 1; + } + } + } + } + + // Phase 2: walk filesystem, remove files not in the index + // Walk archives and installed roots + for phase_root in &[ + super::paths::archives_root(&cache_root), + super::paths::installed_root(&cache_root), + ] { + if !phase_root.exists() { + continue; + } + remove_partial_dirs(phase_root, &mut report); + } + + Ok(report) +} + +/// Remove any `.partial` directories (incomplete downloads/installs). +fn remove_partial_dirs(root: &Path, report: &mut GcReport) { + if let Ok(walker) = walkdir_sync(root) { + for entry in walker { + let path = entry; + if path.is_dir() + && path + .file_name() + .map(|n| n.to_string_lossy().ends_with(".partial")) + .unwrap_or(false) + { + let _ = std::fs::remove_dir_all(&path); + report.orphan_files_removed += 1; + } + } + } +} + +/// Simple recursive directory walker (no external dependency needed here). +fn walkdir_sync(root: &Path) -> std::io::Result> { + let mut result = Vec::new(); + if root.is_dir() { + for entry in std::fs::read_dir(root)? { + let entry = entry?; + let path = entry.path(); + result.push(path.clone()); + if path.is_dir() { + if let Ok(children) = walkdir_sync(&path) { + result.extend(children); + } + } + } + } + Ok(result) +} + +/// Remove a file or directory. +fn remove_path(path: &Path) -> std::io::Result<()> { + if path.is_dir() { + std::fs::remove_dir_all(path) + } else { + std::fs::remove_file(path) + } +} + +#[cfg(test)] +mod tests { + use super::super::index::CacheIndex; + use super::super::paths::Kind; + use super::*; + + fn make_budget(archive: u64, installed: u64, high: u64) -> CacheBudget { + CacheBudget { + archive_budget: archive, + installed_budget: installed, + high_watermark: high, + low_watermark: (high as f64 * 0.80) as u64, + } + } + + #[test] + fn test_gc_no_eviction_under_budget() { + let idx = CacheIndex::open_in_memory().unwrap(); + idx.record_install(Kind::Packages, "https://a.com/a", "1.0", "p", 100) + .unwrap(); + + let budget = make_budget(1_000_000, 1_000_000, 2_000_000); + let report = run_gc(&idx, &budget).unwrap(); + assert_eq!(report.installed_evicted, 0); + assert_eq!(report.archives_evicted, 0); + } + + #[test] + fn test_gc_evicts_installed_first() { + let tmp = tempfile::TempDir::new().unwrap(); + let idx = CacheIndex::open(tmp.path()).unwrap(); + + // installed: 3000 bytes over budget of 2000 + idx.record_install( + Kind::Packages, + "https://a.com/a", + "1.0", + "installed/a", + 1000, + ) + .unwrap(); + idx.record_install( + Kind::Packages, + "https://b.com/b", + "1.0", + "installed/b", + 1000, + ) + .unwrap(); + idx.record_install( + Kind::Packages, + "https://c.com/c", + "1.0", + "installed/c", + 1000, + ) + .unwrap(); + + // archive: 500 bytes within budget + idx.record_archive( + Kind::Packages, + "https://d.com/d", + "1.0", + "archives/d", + 500, + "sha", + ) + .unwrap(); + + let budget = make_budget(10000, 2000, 5000); + let report = run_gc(&idx, &budget).unwrap(); + + // Should evict installed to get under budget, archives untouched + assert!(report.installed_evicted > 0); + assert_eq!(report.archives_evicted, 0); + } + + #[test] + fn test_gc_evicts_archives_when_over_budget() { + let tmp = tempfile::TempDir::new().unwrap(); + let idx = CacheIndex::open(tmp.path()).unwrap(); + + // archives: 3000 bytes over budget of 1000 + idx.record_archive( + Kind::Packages, + "https://a.com/a", + "1.0", + "archives/a", + 1000, + "s1", + ) + .unwrap(); + idx.record_archive( + Kind::Packages, + "https://b.com/b", + "1.0", + "archives/b", + 1000, + "s2", + ) + .unwrap(); + idx.record_archive( + Kind::Packages, + "https://c.com/c", + "1.0", + "archives/c", + 1000, + "s3", + ) + .unwrap(); + + let budget = make_budget(1000, 10000, 20000); + let report = run_gc(&idx, &budget).unwrap(); + + assert!(report.archives_evicted > 0); + assert!(report.archive_bytes_freed > 0); + } + + #[test] + fn test_gc_lease_blocks_eviction() { + let tmp = tempfile::TempDir::new().unwrap(); + let idx = CacheIndex::open(tmp.path()).unwrap(); + + // One installed entry, pinned + let entry = idx + .record_install( + Kind::Packages, + "https://a.com/a", + "1.0", + "installed/a", + 5000, + ) + .unwrap(); + idx.pin(entry.id, std::process::id()).unwrap(); + + // Budget is tiny — would normally evict + let budget = make_budget(100, 100, 200); + let report = run_gc(&idx, &budget).unwrap(); + + // But it's pinned, so no eviction + assert_eq!(report.installed_evicted, 0); + } + + #[test] + fn test_gc_low_watermark_stops_eviction() { + let tmp = tempfile::TempDir::new().unwrap(); + let idx = CacheIndex::open(tmp.path()).unwrap(); + + // 10 entries, 100 bytes each = 1000 total + for i in 0..10 { + idx.record_install( + Kind::Packages, + &format!("https://x.com/{}", i), + "1.0", + &format!("installed/{}", i), + 100, + ) + .unwrap(); + } + + // installed_budget = 500, high_watermark = 1200, low_watermark = 960 + // Combined = 1000, not > high_watermark (1200), but installed (1000) > installed_budget (500) + // Should evict down to low_watermark min installed_budget = 500 + let budget = make_budget(10000, 500, 1200); + let report = run_gc(&idx, &budget).unwrap(); + + // Should have evicted some entries + assert!(report.installed_evicted > 0); + // But not all of them + let remaining = idx.total_installed_bytes().unwrap() as u64; + assert!(remaining <= 500, "remaining {} should be <= 500", remaining); + } + + #[test] + fn test_reconcile_removes_partial_dirs() { + let tmp = tempfile::TempDir::new().unwrap(); + let idx = CacheIndex::open(tmp.path()).unwrap(); + + // Create a .partial directory in installed/ + let partial = super::super::paths::installed_root(tmp.path()) + .join("packages") + .join("test") + .join("abc") + .join("1.0.partial"); + std::fs::create_dir_all(&partial).unwrap(); + assert!(partial.exists()); + + let report = reconcile(&idx).unwrap(); + assert!(report.orphan_files_removed > 0); + assert!(!partial.exists()); + } +} diff --git a/crates/fbuild-packages/src/disk_cache/index.rs b/crates/fbuild-packages/src/disk_cache/index.rs new file mode 100644 index 00000000..8a79bb75 --- /dev/null +++ b/crates/fbuild-packages/src/disk_cache/index.rs @@ -0,0 +1,797 @@ +//! SQLite-based crash-safe index for the two-phase disk cache. +//! +//! Opened in WAL mode with `synchronous=NORMAL` for multi-reader/single-writer +//! safety and crash recovery. The WAL is replayed on next open after a crash. + +use super::paths::{self, Kind}; +use rusqlite::{params, Connection, OptionalExtension}; +use std::path::{Path, PathBuf}; +use std::sync::Mutex; +use std::time::{SystemTime, UNIX_EPOCH}; + +const SCHEMA_VERSION: i64 = 1; + +/// A row in the `entries` table. +#[derive(Debug, Clone)] +pub struct CacheEntry { + pub id: i64, + pub kind: Kind, + pub url: String, + pub stem: String, + pub hash: String, + pub version: String, + pub archive_path: Option, + pub archive_bytes: Option, + pub archive_sha256: Option, + pub installed_path: Option, + pub installed_bytes: Option, + pub installed_at: Option, + pub archived_at: Option, + pub last_used_at: i64, + pub use_count: i64, + pub pinned: i64, +} + +/// The crash-safe SQLite index. +/// +/// Wraps the connection in a `Mutex` so the index is `Send + Sync` +/// and can be shared via `Arc` across threads. +pub struct CacheIndex { + conn: Mutex, + cache_root: PathBuf, +} + +impl CacheIndex { + /// Open (or create) the index at the standard location under `cache_root`. + /// Runs migrations if needed. + pub fn open(cache_root: &Path) -> rusqlite::Result { + let db_path = paths::index_path(cache_root); + std::fs::create_dir_all(cache_root).map_err(|e| { + rusqlite::Error::InvalidPath(PathBuf::from(format!( + "failed to create cache root: {}", + e + ))) + })?; + let conn = Connection::open(&db_path)?; + + // WAL mode for crash safety and concurrent access + conn.pragma_update(None, "journal_mode", "WAL")?; + conn.pragma_update(None, "synchronous", "NORMAL")?; + // Enable foreign keys + conn.pragma_update(None, "foreign_keys", "ON")?; + + let idx = Self { + conn: Mutex::new(conn), + cache_root: cache_root.to_path_buf(), + }; + idx.migrate()?; + Ok(idx) + } + + /// Open with an in-memory database (for testing). + #[cfg(test)] + pub fn open_in_memory() -> rusqlite::Result { + let conn = Connection::open_in_memory()?; + conn.pragma_update(None, "foreign_keys", "ON")?; + let idx = Self { + conn: Mutex::new(conn), + cache_root: PathBuf::from("/tmp/test_cache"), + }; + idx.migrate()?; + Ok(idx) + } + + fn migrate(&self) -> rusqlite::Result<()> { + let conn = self.conn.lock().unwrap(); + let version: i64 = conn + .query_row( + "SELECT value FROM cache_meta WHERE key = 'schema_version'", + [], + |row| { + let v: String = row.get(0)?; + Ok(v.parse::().unwrap_or(0)) + }, + ) + .unwrap_or(0); + + if version < 1 { + Self::migrate_v1(&conn)?; + } + Ok(()) + } + + fn migrate_v1(conn: &Connection) -> rusqlite::Result<()> { + let tx = conn.unchecked_transaction()?; + + tx.execute_batch( + "CREATE TABLE IF NOT EXISTS cache_meta ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL + ); + + CREATE TABLE IF NOT EXISTS entries ( + id INTEGER PRIMARY KEY, + kind TEXT NOT NULL, + url TEXT NOT NULL, + stem TEXT NOT NULL, + hash TEXT NOT NULL, + version TEXT NOT NULL, + archive_path TEXT, + archive_bytes INTEGER, + archive_sha256 TEXT, + installed_path TEXT, + installed_bytes INTEGER, + installed_at INTEGER, + archived_at INTEGER, + last_used_at INTEGER NOT NULL, + use_count INTEGER NOT NULL DEFAULT 0, + pinned INTEGER NOT NULL DEFAULT 0, + UNIQUE(kind, hash, version) + ); + + CREATE INDEX IF NOT EXISTS idx_lru_installed + ON entries(last_used_at) WHERE installed_path IS NOT NULL; + CREATE INDEX IF NOT EXISTS idx_lru_archive + ON entries(last_used_at) WHERE archive_path IS NOT NULL; + + CREATE TABLE IF NOT EXISTS leases ( + entry_id INTEGER NOT NULL REFERENCES entries(id) ON DELETE CASCADE, + holder_pid INTEGER NOT NULL, + acquired_at INTEGER NOT NULL, + PRIMARY KEY(entry_id, holder_pid) + );", + )?; + + tx.execute( + "INSERT OR REPLACE INTO cache_meta (key, value) VALUES ('schema_version', ?1)", + params![SCHEMA_VERSION.to_string()], + )?; + + tx.commit()?; + Ok(()) + } + + /// Get the current schema version. + pub fn schema_version(&self) -> rusqlite::Result { + let conn = self.conn.lock().unwrap(); + conn.query_row( + "SELECT value FROM cache_meta WHERE key = 'schema_version'", + [], + |row| { + let v: String = row.get(0)?; + Ok(v.parse::().unwrap_or(0)) + }, + ) + .optional() + .map(|opt| opt.unwrap_or(0)) + } + + fn now_epoch() -> i64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs() as i64 + } + + /// Look up an entry by kind, url, and version. + pub fn lookup( + &self, + kind: Kind, + url: &str, + version: &str, + ) -> rusqlite::Result> { + let (_, hash) = paths::stem_and_hash(url); + let conn = self.conn.lock().unwrap(); + conn.query_row( + "SELECT id, kind, url, stem, hash, version, + archive_path, archive_bytes, archive_sha256, + installed_path, installed_bytes, installed_at, + archived_at, last_used_at, use_count, pinned + FROM entries + WHERE kind = ?1 AND hash = ?2 AND version = ?3", + params![kind.as_str(), hash, version], + Self::row_to_entry, + ) + .optional() + } + + /// Record that an archive was downloaded. + pub fn record_archive( + &self, + kind: Kind, + url: &str, + version: &str, + archive_path: &str, + archive_bytes: i64, + archive_sha256: &str, + ) -> rusqlite::Result { + let (stem, hash) = paths::stem_and_hash(url); + let now = Self::now_epoch(); + let conn = self.conn.lock().unwrap(); + conn.execute( + "INSERT INTO entries (kind, url, stem, hash, version, + archive_path, archive_bytes, archive_sha256, + archived_at, last_used_at, use_count, pinned) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, 0, 0) + ON CONFLICT(kind, hash, version) DO UPDATE SET + archive_path = excluded.archive_path, + archive_bytes = excluded.archive_bytes, + archive_sha256 = excluded.archive_sha256, + archived_at = excluded.archived_at, + last_used_at = excluded.last_used_at", + params![ + kind.as_str(), + url, + stem, + hash, + version, + archive_path, + archive_bytes, + archive_sha256, + now, + now, + ], + )?; + drop(conn); + + self.lookup(kind, url, version) + .map(|opt| opt.expect("entry must exist after insert")) + } + + /// Record that an archive was extracted/installed. + pub fn record_install( + &self, + kind: Kind, + url: &str, + version: &str, + installed_path: &str, + installed_bytes: i64, + ) -> rusqlite::Result { + let (stem, hash) = paths::stem_and_hash(url); + let now = Self::now_epoch(); + let conn = self.conn.lock().unwrap(); + conn.execute( + "INSERT INTO entries (kind, url, stem, hash, version, + installed_path, installed_bytes, installed_at, + last_used_at, use_count, pinned) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, 0, 0) + ON CONFLICT(kind, hash, version) DO UPDATE SET + installed_path = excluded.installed_path, + installed_bytes = excluded.installed_bytes, + installed_at = excluded.installed_at, + last_used_at = excluded.last_used_at", + params![ + kind.as_str(), + url, + stem, + hash, + version, + installed_path, + installed_bytes, + now, + now, + ], + )?; + drop(conn); + + self.lookup(kind, url, version) + .map(|opt| opt.expect("entry must exist after insert")) + } + + /// Bump LRU timestamp and use count for an entry. + pub fn touch(&self, entry_id: i64) -> rusqlite::Result<()> { + let now = Self::now_epoch(); + let conn = self.conn.lock().unwrap(); + conn.execute( + "UPDATE entries SET last_used_at = ?1, use_count = use_count + 1 WHERE id = ?2", + params![now, entry_id], + )?; + Ok(()) + } + + /// Increment the pinned count for an entry (lease acquired). + pub fn pin(&self, entry_id: i64, holder_pid: u32) -> rusqlite::Result<()> { + let now = Self::now_epoch(); + let conn = self.conn.lock().unwrap(); + conn.execute( + "INSERT OR IGNORE INTO leases (entry_id, holder_pid, acquired_at) VALUES (?1, ?2, ?3)", + params![entry_id, holder_pid as i64, now], + )?; + conn.execute( + "UPDATE entries SET pinned = (SELECT COUNT(*) FROM leases WHERE entry_id = ?1) WHERE id = ?1", + params![entry_id], + )?; + Ok(()) + } + + /// Decrement the pinned count for an entry (lease released). + pub fn unpin(&self, entry_id: i64, holder_pid: u32) -> rusqlite::Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute( + "DELETE FROM leases WHERE entry_id = ?1 AND holder_pid = ?2", + params![entry_id, holder_pid as i64], + )?; + conn.execute( + "UPDATE entries SET pinned = (SELECT COUNT(*) FROM leases WHERE entry_id = ?1) WHERE id = ?1", + params![entry_id], + )?; + Ok(()) + } + + /// Get total bytes for all archives. + pub fn total_archive_bytes(&self) -> rusqlite::Result { + let conn = self.conn.lock().unwrap(); + conn.query_row( + "SELECT COALESCE(SUM(archive_bytes), 0) FROM entries WHERE archive_path IS NOT NULL", + [], + |row| row.get::<_, i64>(0), + ) + } + + /// Get total bytes for all installed entries. + pub fn total_installed_bytes(&self) -> rusqlite::Result { + let conn = self.conn.lock().unwrap(); + conn.query_row( + "SELECT COALESCE(SUM(installed_bytes), 0) FROM entries WHERE installed_path IS NOT NULL", + [], + |row| row.get::<_, i64>(0), + ) + } + + /// Get LRU installed entries (oldest first), skipping pinned. + pub fn lru_installed_entries(&self, limit: usize) -> rusqlite::Result> { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare( + "SELECT id, kind, url, stem, hash, version, + archive_path, archive_bytes, archive_sha256, + installed_path, installed_bytes, installed_at, + archived_at, last_used_at, use_count, pinned + FROM entries + WHERE installed_path IS NOT NULL AND pinned = 0 + ORDER BY last_used_at ASC + LIMIT ?1", + )?; + + let entries = stmt + .query_map(params![limit as i64], Self::row_to_entry)? + .collect::, _>>()?; + Ok(entries) + } + + /// Get LRU archive entries (oldest first), skipping pinned. + pub fn lru_archive_entries(&self, limit: usize) -> rusqlite::Result> { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare( + "SELECT id, kind, url, stem, hash, version, + archive_path, archive_bytes, archive_sha256, + installed_path, installed_bytes, installed_at, + archived_at, last_used_at, use_count, pinned + FROM entries + WHERE archive_path IS NOT NULL AND pinned = 0 + ORDER BY last_used_at ASC + LIMIT ?1", + )?; + + let entries = stmt + .query_map(params![limit as i64], Self::row_to_entry)? + .collect::, _>>()?; + Ok(entries) + } + + /// Null out the installed_path for an entry (after evicting its directory). + pub fn clear_installed(&self, entry_id: i64) -> rusqlite::Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute( + "UPDATE entries SET installed_path = NULL, installed_bytes = NULL, installed_at = NULL WHERE id = ?1", + params![entry_id], + )?; + Ok(()) + } + + /// Null out the archive_path for an entry (after evicting its archive). + pub fn clear_archive(&self, entry_id: i64) -> rusqlite::Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute( + "UPDATE entries SET archive_path = NULL, archive_bytes = NULL, archive_sha256 = NULL, archived_at = NULL WHERE id = ?1", + params![entry_id], + )?; + Ok(()) + } + + /// Delete an entry entirely (when both archive and installed are gone). + pub fn delete_entry(&self, entry_id: i64) -> rusqlite::Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute("DELETE FROM entries WHERE id = ?1", params![entry_id])?; + Ok(()) + } + + /// Reap leases for dead PIDs. + pub fn reap_dead_leases(&self) -> rusqlite::Result { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare("SELECT DISTINCT holder_pid FROM leases")?; + let pids: Vec = stmt + .query_map([], |row| row.get::<_, i64>(0))? + .collect::, _>>()?; + drop(stmt); + + let mut reaped = 0; + for pid in pids { + if !is_pid_alive(pid as u32) { + conn.execute("DELETE FROM leases WHERE holder_pid = ?1", params![pid])?; + reaped += 1; + } + } + + // Refresh pinned counts for all affected entries + if reaped > 0 { + conn.execute_batch( + "UPDATE entries SET pinned = (SELECT COUNT(*) FROM leases WHERE leases.entry_id = entries.id)", + )?; + } + + Ok(reaped) + } + + /// Get all entries (for reconciliation). + pub fn all_entries(&self) -> rusqlite::Result> { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare( + "SELECT id, kind, url, stem, hash, version, + archive_path, archive_bytes, archive_sha256, + installed_path, installed_bytes, installed_at, + archived_at, last_used_at, use_count, pinned + FROM entries", + )?; + + let entries = stmt + .query_map([], Self::row_to_entry)? + .collect::, _>>()?; + Ok(entries) + } + + /// Get total entry count. + pub fn entry_count(&self) -> rusqlite::Result { + let conn = self.conn.lock().unwrap(); + conn.query_row("SELECT COUNT(*) FROM entries", [], |row| { + row.get::<_, i64>(0) + }) + } + + /// Get a reference to the cache root path. + pub fn cache_root(&self) -> &Path { + &self.cache_root + } + + fn row_to_entry(row: &rusqlite::Row<'_>) -> rusqlite::Result { + let kind_str: String = row.get(1)?; + let kind: Kind = kind_str.parse().map_err(|e: String| { + rusqlite::Error::FromSqlConversionFailure( + 1, + rusqlite::types::Type::Text, + Box::new(std::io::Error::new(std::io::ErrorKind::InvalidData, e)), + ) + })?; + Ok(CacheEntry { + id: row.get(0)?, + kind, + url: row.get(2)?, + stem: row.get(3)?, + hash: row.get(4)?, + version: row.get(5)?, + archive_path: row.get(6)?, + archive_bytes: row.get(7)?, + archive_sha256: row.get(8)?, + installed_path: row.get(9)?, + installed_bytes: row.get(10)?, + installed_at: row.get(11)?, + archived_at: row.get(12)?, + last_used_at: row.get(13)?, + use_count: row.get(14)?, + pinned: row.get(15)?, + }) + } +} + +/// Check if a PID is alive. Platform-specific. +fn is_pid_alive(pid: u32) -> bool { + #[cfg(unix)] + { + // kill(pid, 0) checks if process exists without sending a signal + unsafe { libc::kill(pid as i32, 0) == 0 } + } + #[cfg(windows)] + { + use std::process::Command; + Command::new("tasklist") + .args(["/FI", &format!("PID eq {}", pid), "/NH"]) + .output() + .map(|o| { + let stdout = String::from_utf8_lossy(&o.stdout); + stdout.contains(&pid.to_string()) + }) + .unwrap_or(false) + } + #[cfg(not(any(unix, windows)))] + { + let _ = pid; + false + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_index_open_creates_schema() { + let tmp = tempfile::TempDir::new().unwrap(); + let idx = CacheIndex::open(tmp.path()).unwrap(); + assert_eq!(idx.schema_version().unwrap(), 1); + assert_eq!(idx.entry_count().unwrap(), 0); + } + + #[test] + fn test_index_reopen_preserves_data() { + let tmp = tempfile::TempDir::new().unwrap(); + { + let idx = CacheIndex::open(tmp.path()).unwrap(); + idx.record_archive( + Kind::Packages, + "https://example.com/pkg.tar.gz", + "1.0.0", + "archives/packages/example-pkg/abc123/1.0.0/pkg.tar.gz", + 1024, + "sha256abc", + ) + .unwrap(); + } + // Reopen + let idx = CacheIndex::open(tmp.path()).unwrap(); + let entry = idx + .lookup(Kind::Packages, "https://example.com/pkg.tar.gz", "1.0.0") + .unwrap(); + assert!(entry.is_some()); + let entry = entry.unwrap(); + assert_eq!(entry.archive_bytes, Some(1024)); + } + + #[test] + fn test_record_archive_then_install_roundtrip() { + let idx = CacheIndex::open_in_memory().unwrap(); + let url = "https://example.com/tool.tar.gz"; + + let entry = idx + .record_archive( + Kind::Toolchains, + url, + "7.3.0", + "archives/tool.tar.gz", + 5000, + "deadbeef", + ) + .unwrap(); + assert_eq!(entry.kind, Kind::Toolchains); + assert_eq!(entry.version, "7.3.0"); + assert_eq!(entry.archive_bytes, Some(5000)); + assert!(entry.installed_path.is_none()); + + let entry = idx + .record_install( + Kind::Toolchains, + url, + "7.3.0", + "installed/toolchains/tool/abc/7.3.0", + 20000, + ) + .unwrap(); + assert_eq!(entry.archive_bytes, Some(5000)); // archive still there + assert_eq!(entry.installed_bytes, Some(20000)); + assert!(entry.installed_path.is_some()); + } + + #[test] + fn test_lookup_returns_none_for_missing() { + let idx = CacheIndex::open_in_memory().unwrap(); + let result = idx + .lookup(Kind::Packages, "https://example.com/nope", "1.0.0") + .unwrap(); + assert!(result.is_none()); + } + + #[test] + fn test_touch_bumps_use_count() { + let idx = CacheIndex::open_in_memory().unwrap(); + let entry = idx + .record_archive( + Kind::Packages, + "https://example.com/a", + "1.0", + "path", + 100, + "sha", + ) + .unwrap(); + assert_eq!(entry.use_count, 0); + + idx.touch(entry.id).unwrap(); + let entry = idx + .lookup(Kind::Packages, "https://example.com/a", "1.0") + .unwrap() + .unwrap(); + assert_eq!(entry.use_count, 1); + + idx.touch(entry.id).unwrap(); + let entry = idx + .lookup(Kind::Packages, "https://example.com/a", "1.0") + .unwrap() + .unwrap(); + assert_eq!(entry.use_count, 2); + } + + #[test] + fn test_pin_and_unpin() { + let idx = CacheIndex::open_in_memory().unwrap(); + let entry = idx + .record_archive( + Kind::Packages, + "https://example.com/a", + "1.0", + "path", + 100, + "sha", + ) + .unwrap(); + assert_eq!(entry.pinned, 0); + + idx.pin(entry.id, 12345).unwrap(); + let entry = idx + .lookup(Kind::Packages, "https://example.com/a", "1.0") + .unwrap() + .unwrap(); + assert_eq!(entry.pinned, 1); + + // Pin with another PID + idx.pin(entry.id, 67890).unwrap(); + let entry = idx + .lookup(Kind::Packages, "https://example.com/a", "1.0") + .unwrap() + .unwrap(); + assert_eq!(entry.pinned, 2); + + // Unpin one + idx.unpin(entry.id, 12345).unwrap(); + let entry = idx + .lookup(Kind::Packages, "https://example.com/a", "1.0") + .unwrap() + .unwrap(); + assert_eq!(entry.pinned, 1); + } + + #[test] + fn test_lru_installed_entries_skip_pinned() { + let idx = CacheIndex::open_in_memory().unwrap(); + + // Create two installed entries + let e1 = idx + .record_install( + Kind::Packages, + "https://example.com/a", + "1.0", + "path_a", + 1000, + ) + .unwrap(); + let _e2 = idx + .record_install( + Kind::Packages, + "https://example.com/b", + "1.0", + "path_b", + 2000, + ) + .unwrap(); + + // Pin e1 + idx.pin(e1.id, 99999).unwrap(); + + // LRU should only return e2 (unpinned) + let lru = idx.lru_installed_entries(10).unwrap(); + assert_eq!(lru.len(), 1); + assert_eq!(lru[0].url, "https://example.com/b"); + } + + #[test] + fn test_clear_installed_nulls_fields() { + let idx = CacheIndex::open_in_memory().unwrap(); + let url = "https://example.com/pkg"; + idx.record_archive(Kind::Packages, url, "1.0", "archive_path", 100, "sha") + .unwrap(); + let entry = idx + .record_install(Kind::Packages, url, "1.0", "install_path", 500) + .unwrap(); + + idx.clear_installed(entry.id).unwrap(); + let entry = idx.lookup(Kind::Packages, url, "1.0").unwrap().unwrap(); + assert!(entry.installed_path.is_none()); + assert!(entry.installed_bytes.is_none()); + // Archive still present + assert!(entry.archive_path.is_some()); + } + + #[test] + fn test_clear_archive_nulls_fields() { + let idx = CacheIndex::open_in_memory().unwrap(); + let url = "https://example.com/pkg"; + let entry = idx + .record_archive(Kind::Packages, url, "1.0", "archive_path", 100, "sha") + .unwrap(); + + idx.clear_archive(entry.id).unwrap(); + let entry = idx.lookup(Kind::Packages, url, "1.0").unwrap().unwrap(); + assert!(entry.archive_path.is_none()); + assert!(entry.archive_bytes.is_none()); + assert!(entry.archive_sha256.is_none()); + } + + #[test] + fn test_delete_entry() { + let idx = CacheIndex::open_in_memory().unwrap(); + let entry = idx + .record_archive( + Kind::Packages, + "https://example.com/a", + "1.0", + "path", + 100, + "sha", + ) + .unwrap(); + assert_eq!(idx.entry_count().unwrap(), 1); + + idx.delete_entry(entry.id).unwrap(); + assert_eq!(idx.entry_count().unwrap(), 0); + } + + #[test] + fn test_total_bytes_accounting() { + let idx = CacheIndex::open_in_memory().unwrap(); + assert_eq!(idx.total_archive_bytes().unwrap(), 0); + assert_eq!(idx.total_installed_bytes().unwrap(), 0); + + idx.record_archive(Kind::Packages, "https://a.com/a", "1.0", "p1", 1000, "s1") + .unwrap(); + idx.record_archive(Kind::Packages, "https://b.com/b", "1.0", "p2", 2000, "s2") + .unwrap(); + assert_eq!(idx.total_archive_bytes().unwrap(), 3000); + + idx.record_install(Kind::Toolchains, "https://c.com/c", "1.0", "p3", 5000) + .unwrap(); + assert_eq!(idx.total_installed_bytes().unwrap(), 5000); + } + + #[test] + fn test_reconcile_orphan_row_nulled() { + let tmp = tempfile::TempDir::new().unwrap(); + let idx = CacheIndex::open(tmp.path()).unwrap(); + + let entry = idx + .record_archive( + Kind::Packages, + "https://example.com/orphan", + "1.0", + "archives/packages/orphan/hash/1.0/pkg.tar.gz", + 500, + "sha", + ) + .unwrap(); + + assert!(entry.archive_path.is_some()); + idx.clear_archive(entry.id).unwrap(); + let entry = idx + .lookup(Kind::Packages, "https://example.com/orphan", "1.0") + .unwrap() + .unwrap(); + assert!(entry.archive_path.is_none()); + assert_eq!(idx.entry_count().unwrap(), 1); + } +} diff --git a/crates/fbuild-packages/src/disk_cache/lease.rs b/crates/fbuild-packages/src/disk_cache/lease.rs new file mode 100644 index 00000000..e0a5d0d8 --- /dev/null +++ b/crates/fbuild-packages/src/disk_cache/lease.rs @@ -0,0 +1,117 @@ +//! RAII lease guard that pins cache entries during builds. +//! +//! Holding a `Lease` prevents the GC from evicting the associated entry. +//! The lease is released on drop. + +use super::index::CacheIndex; +use std::sync::Arc; + +/// RAII guard that pins a cache entry. Released on drop. +pub struct Lease { + index: Arc, + entry_id: i64, + holder_pid: u32, +} + +impl Lease { + /// Acquire a lease for the given entry. + pub fn acquire(index: Arc, entry_id: i64) -> rusqlite::Result { + let pid = std::process::id(); + index.pin(entry_id, pid)?; + Ok(Self { + index, + entry_id, + holder_pid: pid, + }) + } + + /// The entry ID this lease protects. + pub fn entry_id(&self) -> i64 { + self.entry_id + } +} + +impl Drop for Lease { + fn drop(&mut self) { + let _ = self.index.unpin(self.entry_id, self.holder_pid); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_lease_pins_and_unpins_on_drop() { + let idx = Arc::new(CacheIndex::open_in_memory().unwrap()); + let entry = idx + .record_archive( + super::super::paths::Kind::Packages, + "https://example.com/a", + "1.0", + "path", + 100, + "sha", + ) + .unwrap(); + + // Before lease + let e = idx + .lookup( + super::super::paths::Kind::Packages, + "https://example.com/a", + "1.0", + ) + .unwrap() + .unwrap(); + assert_eq!(e.pinned, 0); + + // Acquire lease + { + let _lease = Lease::acquire(Arc::clone(&idx), entry.id).unwrap(); + let e = idx + .lookup( + super::super::paths::Kind::Packages, + "https://example.com/a", + "1.0", + ) + .unwrap() + .unwrap(); + assert_eq!(e.pinned, 1); + } + + // After drop + let e = idx + .lookup( + super::super::paths::Kind::Packages, + "https://example.com/a", + "1.0", + ) + .unwrap() + .unwrap(); + assert_eq!(e.pinned, 0); + } + + #[test] + fn test_lease_blocks_lru_eviction() { + let idx = Arc::new(CacheIndex::open_in_memory().unwrap()); + + // Create installed entry + let entry = idx + .record_install( + super::super::paths::Kind::Toolchains, + "https://example.com/tool", + "1.0", + "installed/tool", + 5000, + ) + .unwrap(); + + // Acquire lease + let _lease = Lease::acquire(Arc::clone(&idx), entry.id).unwrap(); + + // LRU query should skip this pinned entry + let lru = idx.lru_installed_entries(10).unwrap(); + assert!(lru.is_empty(), "leased entry should not appear in LRU list"); + } +} diff --git a/crates/fbuild-packages/src/disk_cache/mod.rs b/crates/fbuild-packages/src/disk_cache/mod.rs new file mode 100644 index 00000000..7b7fe570 --- /dev/null +++ b/crates/fbuild-packages/src/disk_cache/mod.rs @@ -0,0 +1,343 @@ +//! Two-phase disk cache with LRU garbage collection and crash-safe SQLite index. +//! +//! Separates downloaded archives from installed (extracted) content. +//! GC evicts cheap-to-rehydrate installed directories before expensive archives. +//! +//! # Usage +//! +//! ```ignore +//! let cache = DiskCache::open()?; +//! if let Some(entry) = cache.lookup(Kind::Toolchains, url, version)? { +//! let _lease = cache.lease(&entry)?; // pin during build +//! cache.touch(&entry)?; +//! // use entry.installed_path ... +//! } +//! ``` + +pub mod budget; +pub mod gc; +pub mod index; +pub mod lease; +pub mod paths; + +pub use budget::CacheBudget; +pub use gc::GcReport; +pub use index::{CacheEntry, CacheIndex}; +pub use lease::Lease; +pub use paths::Kind; + +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +/// The public facade for the two-phase disk cache. +pub struct DiskCache { + index: Arc, + cache_root: PathBuf, + budget: CacheBudget, +} + +impl DiskCache { + /// Open the disk cache at the standard location. + pub fn open() -> rusqlite::Result { + let cache_root = fbuild_paths::get_cache_root(); + Self::open_at(&cache_root) + } + + /// Open the disk cache at a specific root (for testing). + pub fn open_at(cache_root: &Path) -> rusqlite::Result { + let index = CacheIndex::open(cache_root)?; + let budget = CacheBudget::compute(cache_root); + + // Ensure phase directories exist + std::fs::create_dir_all(paths::archives_root(cache_root)).ok(); + std::fs::create_dir_all(paths::installed_root(cache_root)).ok(); + + Ok(Self { + index: Arc::new(index), + cache_root: cache_root.to_path_buf(), + budget, + }) + } + + /// Look up a cache entry. + pub fn lookup( + &self, + kind: Kind, + url: &str, + version: &str, + ) -> rusqlite::Result> { + self.index.lookup(kind, url, version) + } + + /// Record that an archive was downloaded. + pub fn record_archive( + &self, + kind: Kind, + url: &str, + version: &str, + archive_path: &str, + archive_bytes: i64, + archive_sha256: &str, + ) -> rusqlite::Result { + self.index.record_archive( + kind, + url, + version, + archive_path, + archive_bytes, + archive_sha256, + ) + } + + /// Record that an entry was installed (extracted from archive). + pub fn record_install( + &self, + kind: Kind, + url: &str, + version: &str, + installed_path: &str, + installed_bytes: i64, + ) -> rusqlite::Result { + self.index + .record_install(kind, url, version, installed_path, installed_bytes) + } + + /// Acquire a lease for the given entry, preventing GC eviction. + pub fn lease(&self, entry: &CacheEntry) -> rusqlite::Result { + Lease::acquire(Arc::clone(&self.index), entry.id) + } + + /// Bump LRU timestamp for a cache hit. + pub fn touch(&self, entry: &CacheEntry) -> rusqlite::Result<()> { + self.index.touch(entry.id) + } + + /// Run a full GC pass. + pub fn run_gc(&self) -> rusqlite::Result { + gc::run_gc(&self.index, &self.budget) + } + + /// Reconcile index against filesystem (run on daemon startup). + pub fn reconcile(&self) -> rusqlite::Result { + gc::reconcile(&self.index) + } + + /// Get cache statistics. + pub fn stats(&self) -> rusqlite::Result { + Ok(CacheStats { + archive_bytes: self.index.total_archive_bytes()? as u64, + installed_bytes: self.index.total_installed_bytes()? as u64, + entry_count: self.index.entry_count()?, + budget: self.budget, + }) + } + + /// Get the cache root path. + pub fn cache_root(&self) -> &Path { + &self.cache_root + } + + /// Get the computed budget. + pub fn budget(&self) -> &CacheBudget { + &self.budget + } + + // --- Path helpers for callers --- + + /// Get the archive entry directory for a given kind/url/version. + pub fn archive_dir(&self, kind: Kind, url: &str, version: &str) -> PathBuf { + paths::archive_entry_dir(&self.cache_root, kind, url, version) + } + + /// Get the staging directory for an in-progress archive download. + pub fn archive_staging_dir(&self, kind: Kind, url: &str, version: &str) -> PathBuf { + paths::archive_staging_dir(&self.cache_root, kind, url, version) + } + + /// Get the installed entry directory. + pub fn installed_dir(&self, kind: Kind, url: &str, version: &str) -> PathBuf { + paths::installed_entry_dir(&self.cache_root, kind, url, version) + } + + /// Get the staging directory for an in-progress installation. + pub fn install_staging_dir(&self, kind: Kind, url: &str, version: &str) -> PathBuf { + paths::install_staging_dir(&self.cache_root, kind, url, version) + } +} + +/// Cache statistics for `fbuild status`. +#[derive(Debug, Clone)] +pub struct CacheStats { + pub archive_bytes: u64, + pub installed_bytes: u64, + pub entry_count: i64, + pub budget: CacheBudget, +} + +impl CacheStats { + pub fn total_bytes(&self) -> u64 { + self.archive_bytes + self.installed_bytes + } +} + +impl std::fmt::Display for CacheStats { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Cache: {} entries, {} installed + {} archives = {} total (budget: {} high, {} low)", + self.entry_count, + format_bytes(self.installed_bytes), + format_bytes(self.archive_bytes), + format_bytes(self.total_bytes()), + format_bytes(self.budget.high_watermark), + format_bytes(self.budget.low_watermark), + ) + } +} + +fn format_bytes(bytes: u64) -> String { + const GIB: u64 = 1024 * 1024 * 1024; + const MIB: u64 = 1024 * 1024; + const KIB: u64 = 1024; + + if bytes >= GIB { + format!("{:.1} GiB", bytes as f64 / GIB as f64) + } else if bytes >= MIB { + format!("{:.1} MiB", bytes as f64 / MIB as f64) + } else if bytes >= KIB { + format!("{:.1} KiB", bytes as f64 / KIB as f64) + } else { + format!("{} B", bytes) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_disk_cache_open_and_stats() { + let tmp = tempfile::TempDir::new().unwrap(); + let cache = DiskCache::open_at(tmp.path()).unwrap(); + let stats = cache.stats().unwrap(); + assert_eq!(stats.entry_count, 0); + assert_eq!(stats.archive_bytes, 0); + assert_eq!(stats.installed_bytes, 0); + } + + #[test] + fn test_disk_cache_full_workflow() { + let tmp = tempfile::TempDir::new().unwrap(); + let cache = DiskCache::open_at(tmp.path()).unwrap(); + let url = "https://example.com/tool.tar.gz"; + + // Record archive + let entry = cache + .record_archive( + Kind::Toolchains, + url, + "7.3.0", + "archives/tool.tar.gz", + 5000, + "abc123", + ) + .unwrap(); + assert!(entry.archive_path.is_some()); + + // Record install + let entry = cache + .record_install( + Kind::Toolchains, + url, + "7.3.0", + "installed/tool/7.3.0", + 20000, + ) + .unwrap(); + assert!(entry.installed_path.is_some()); + + // Lookup + let found = cache.lookup(Kind::Toolchains, url, "7.3.0").unwrap(); + assert!(found.is_some()); + + // Touch + cache.touch(&found.unwrap()).unwrap(); + + // Stats + let stats = cache.stats().unwrap(); + assert_eq!(stats.entry_count, 1); + assert_eq!(stats.archive_bytes, 5000); + assert_eq!(stats.installed_bytes, 20000); + } + + #[test] + fn test_disk_cache_lease_workflow() { + let tmp = tempfile::TempDir::new().unwrap(); + let cache = DiskCache::open_at(tmp.path()).unwrap(); + + let entry = cache + .record_install( + Kind::Packages, + "https://example.com/a", + "1.0", + "path_a", + 1000, + ) + .unwrap(); + + // Acquire lease + let _lease = cache.lease(&entry).unwrap(); + + // Entry should be pinned + let entry = cache + .lookup(Kind::Packages, "https://example.com/a", "1.0") + .unwrap() + .unwrap(); + assert_eq!(entry.pinned, 1); + } + + #[test] + fn test_disk_cache_path_helpers() { + let tmp = tempfile::TempDir::new().unwrap(); + let cache = DiskCache::open_at(tmp.path()).unwrap(); + let url = "https://example.com/pkg.tar.gz"; + + let archive_dir = cache.archive_dir(Kind::Packages, url, "1.0"); + assert!(archive_dir.to_string_lossy().contains("archives")); + assert!(archive_dir.to_string_lossy().contains("packages")); + + let installed_dir = cache.installed_dir(Kind::Packages, url, "1.0"); + assert!(installed_dir.to_string_lossy().contains("installed")); + } + + #[test] + fn test_format_bytes() { + assert_eq!(format_bytes(0), "0 B"); + assert_eq!(format_bytes(512), "512 B"); + assert_eq!(format_bytes(1024), "1.0 KiB"); + assert_eq!(format_bytes(1024 * 1024), "1.0 MiB"); + assert_eq!(format_bytes(1024 * 1024 * 1024), "1.0 GiB"); + assert_eq!(format_bytes(15 * 1024 * 1024 * 1024), "15.0 GiB"); + } + + #[test] + fn test_cache_stats_display() { + let stats = CacheStats { + archive_bytes: 1024 * 1024 * 100, + installed_bytes: 1024 * 1024 * 500, + entry_count: 42, + budget: CacheBudget::compute_with_disk_size(500 * 1024 * 1024 * 1024), + }; + let display = format!("{}", stats); + assert!(display.contains("42 entries")); + } + + #[test] + fn test_reconcile_on_open() { + let tmp = tempfile::TempDir::new().unwrap(); + let cache = DiskCache::open_at(tmp.path()).unwrap(); + // Reconcile should succeed on empty cache + let report = cache.reconcile().unwrap(); + assert_eq!(report.orphan_files_removed, 0); + } +} diff --git a/crates/fbuild-packages/src/disk_cache/paths.rs b/crates/fbuild-packages/src/disk_cache/paths.rs new file mode 100644 index 00000000..5b34ecff --- /dev/null +++ b/crates/fbuild-packages/src/disk_cache/paths.rs @@ -0,0 +1,223 @@ +//! Sole source of cache path construction for the two-phase disk cache. +//! +//! All cache paths flow through this module. No other code in the workspace +//! should construct cache paths directly. + +use crate::cache::{hash_url, url_stem}; +use std::path::{Path, PathBuf}; + +/// The kind of cached artifact. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Kind { + Packages, + Toolchains, + Platforms, + Libraries, + Frameworks, +} + +impl Kind { + pub fn as_str(&self) -> &'static str { + match self { + Kind::Packages => "packages", + Kind::Toolchains => "toolchains", + Kind::Platforms => "platforms", + Kind::Libraries => "libraries", + Kind::Frameworks => "frameworks", + } + } + + pub fn all() -> &'static [Kind] { + &[ + Kind::Packages, + Kind::Toolchains, + Kind::Platforms, + Kind::Libraries, + Kind::Frameworks, + ] + } +} + +impl std::fmt::Display for Kind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +impl std::str::FromStr for Kind { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "packages" => Ok(Kind::Packages), + "toolchains" => Ok(Kind::Toolchains), + "platforms" => Ok(Kind::Platforms), + "libraries" => Ok(Kind::Libraries), + "frameworks" => Ok(Kind::Frameworks), + other => Err(format!("unknown cache kind: {}", other)), + } + } +} + +/// Compute the stem and hash for a URL (delegates to existing helpers). +pub fn stem_and_hash(url: &str) -> (String, String) { + (url_stem(url), hash_url(url)) +} + +/// Root of the archives phase: `{cache_root}/archives/` +pub fn archives_root(cache_root: &Path) -> PathBuf { + cache_root.join("archives") +} + +/// Root of the installed phase: `{cache_root}/installed/` +pub fn installed_root(cache_root: &Path) -> PathBuf { + cache_root.join("installed") +} + +/// Path to the SQLite index: `{cache_root}/index.sqlite` +pub fn index_path(cache_root: &Path) -> PathBuf { + cache_root.join("index.sqlite") +} + +/// Path to the GC advisory lock: `{cache_root}/gc.lock` +pub fn gc_lock_path(cache_root: &Path) -> PathBuf { + cache_root.join("gc.lock") +} + +/// Archive entry path: `{cache_root}/archives/{kind}/{stem}/{hash}/{version}/` +pub fn archive_entry_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { + let (stem, hash) = stem_and_hash(url); + archives_root(cache_root) + .join(kind.as_str()) + .join(stem) + .join(hash) + .join(version) +} + +/// Staging path for an in-progress archive download. +/// Uses `.partial` suffix so reconciliation can identify incomplete downloads. +pub fn archive_staging_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { + let (stem, hash) = stem_and_hash(url); + archives_root(cache_root) + .join(kind.as_str()) + .join(stem) + .join(hash) + .join(format!("{}.partial", version)) +} + +/// Installed entry path: `{cache_root}/installed/{kind}/{stem}/{hash}/{version}/` +pub fn installed_entry_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { + let (stem, hash) = stem_and_hash(url); + installed_root(cache_root) + .join(kind.as_str()) + .join(stem) + .join(hash) + .join(version) +} + +/// Staging path for an in-progress extraction. +pub fn install_staging_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { + let (stem, hash) = stem_and_hash(url); + installed_root(cache_root) + .join(kind.as_str()) + .join(stem) + .join(hash) + .join(format!("{}.partial", version)) +} + +/// Sentinel file that marks a completed installation. +pub fn install_complete_sentinel(installed_dir: &Path) -> PathBuf { + installed_dir.join(".install_complete") +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + + #[test] + fn test_archives_root() { + let root = Path::new("/tmp/cache"); + assert_eq!(archives_root(root), Path::new("/tmp/cache/archives")); + } + + #[test] + fn test_installed_root() { + let root = Path::new("/tmp/cache"); + assert_eq!(installed_root(root), Path::new("/tmp/cache/installed")); + } + + #[test] + fn test_index_path() { + let root = Path::new("/tmp/cache"); + assert_eq!(index_path(root), Path::new("/tmp/cache/index.sqlite")); + } + + #[test] + fn test_archive_entry_dir_structure() { + let root = Path::new("/tmp/cache"); + let url = "https://github.com/FastLED/FastLED#master"; + let dir = archive_entry_dir(root, Kind::Libraries, url, "3.6.0"); + let dir_str = dir.to_string_lossy(); + assert!(dir_str.contains("archives")); + assert!(dir_str.contains("libraries")); + assert!(dir_str.contains("FastLED-FastLED")); // stem + assert!(dir_str.contains("3.6.0")); // version + } + + #[test] + fn test_installed_entry_dir_structure() { + let root = Path::new("/tmp/cache"); + let url = "https://downloads.arduino.cc/tools"; + let dir = installed_entry_dir(root, Kind::Toolchains, url, "7.3.0"); + let dir_str = dir.to_string_lossy(); + assert!(dir_str.contains("installed")); + assert!(dir_str.contains("toolchains")); + assert!(dir_str.contains("arduino-tools")); // stem + assert!(dir_str.contains("7.3.0")); + } + + #[test] + fn test_staging_dirs_have_partial_suffix() { + let root = Path::new("/tmp/cache"); + let url = "https://example.com/pkg.tar.gz"; + let archive_staging = archive_staging_dir(root, Kind::Packages, url, "1.0.0"); + assert!(archive_staging.to_string_lossy().ends_with("1.0.0.partial")); + + let install_staging = install_staging_dir(root, Kind::Packages, url, "1.0.0"); + assert!(install_staging.to_string_lossy().ends_with("1.0.0.partial")); + } + + #[test] + fn test_install_complete_sentinel() { + let dir = Path::new("/tmp/cache/installed/toolchains/gcc/abc123/7.3.0"); + let sentinel = install_complete_sentinel(dir); + assert_eq!( + sentinel, + Path::new("/tmp/cache/installed/toolchains/gcc/abc123/7.3.0/.install_complete") + ); + } + + #[test] + fn test_kind_roundtrip() { + for kind in Kind::all() { + let s = kind.as_str(); + let parsed: Kind = s.parse().unwrap(); + assert_eq!(*kind, parsed); + } + } + + #[test] + fn test_kind_display() { + assert_eq!(format!("{}", Kind::Toolchains), "toolchains"); + assert_eq!(format!("{}", Kind::Packages), "packages"); + } + + #[test] + fn test_stem_and_hash_delegates_correctly() { + let url = "https://example.com/pkg.tar.gz"; + let (stem, hash) = stem_and_hash(url); + assert_eq!(stem, url_stem(url)); + assert_eq!(hash, hash_url(url)); + } +} diff --git a/crates/fbuild-packages/src/lib.rs b/crates/fbuild-packages/src/lib.rs index 61afe608..b7a87d56 100644 --- a/crates/fbuild-packages/src/lib.rs +++ b/crates/fbuild-packages/src/lib.rs @@ -7,12 +7,14 @@ //! - Package, Toolchain, and Framework traits pub mod cache; +pub mod disk_cache; pub mod downloader; pub mod extractor; pub mod library; pub mod toolchain; pub use cache::Cache; +pub use disk_cache::DiskCache; use std::collections::HashMap; use std::future::Future; From de9a2cecc7230e992372c4e6ffd24faac3d01dac Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 22:54:06 -0700 Subject: [PATCH 02/12] fix(cache): address CodeRabbit review feedback on disk cache PR #29 - Sanitize version path components to prevent directory traversal - Write .install_complete sentinel after staged_install() rename - Use refcount-based lease pinning so multiple guards in same process don't unpin each other on drop - Add per-process nonce to leases to prevent PID-reuse collisions - Use POSIX-compatible df -P flags for macOS/BSD compatibility - Evict archives when combined total exceeds high watermark - Add orphan removal for non-indexed entries in reconcile phase 2 - Recompute cache budget before each GC pass (fresh disk stats) - Propagate phase-directory creation failures instead of swallowing - Add tests: version traversal sanitization, lease refcount independence --- .../fbuild-packages/src/disk_cache/budget.rs | 16 ++-- crates/fbuild-packages/src/disk_cache/gc.rs | 64 ++++++++++++-- .../fbuild-packages/src/disk_cache/index.rs | 53 ++++++++---- .../fbuild-packages/src/disk_cache/lease.rs | 86 ++++++++++++++++++- crates/fbuild-packages/src/disk_cache/mod.rs | 18 +++- .../fbuild-packages/src/disk_cache/paths.rs | 53 +++++++++++- crates/fbuild-packages/src/lib.rs | 85 +++++++++++++++++- 7 files changed, 332 insertions(+), 43 deletions(-) diff --git a/crates/fbuild-packages/src/disk_cache/budget.rs b/crates/fbuild-packages/src/disk_cache/budget.rs index 87e08116..c9a1dfcc 100644 --- a/crates/fbuild-packages/src/disk_cache/budget.rs +++ b/crates/fbuild-packages/src/disk_cache/budget.rs @@ -149,11 +149,9 @@ fn get_total_disk_space_windows(path: &Path) -> u64 { #[cfg(unix)] fn get_total_disk_space_unix(path: &Path) -> u64 { use std::process::Command; - // Use `df` to get total space - let output = Command::new("df") - .args(["-B1", "--output=size"]) - .arg(path) - .output(); + // Use POSIX-compatible `df -P` which works on both GNU and BSD/macOS. + // Output is in 512-byte blocks; multiply by 512 to get bytes. + let output = Command::new("df").arg("-P").arg(path).output(); output .ok() @@ -162,7 +160,13 @@ fn get_total_disk_space_unix(path: &Path) -> u64 { stdout .lines() .nth(1) // skip header - .and_then(|line| line.trim().parse::().ok()) + .and_then(|line| { + // POSIX df -P columns: Filesystem 512-blocks Used Available Capacity Mounted + line.split_whitespace() + .nth(1) // total 512-byte blocks + .and_then(|s| s.parse::().ok()) + .map(|blocks| blocks * 512) + }) }) .unwrap_or(500 * 1024 * 1024 * 1024) // fallback 500 GB } diff --git a/crates/fbuild-packages/src/disk_cache/gc.rs b/crates/fbuild-packages/src/disk_cache/gc.rs index 9ee0ff7c..fdd99574 100644 --- a/crates/fbuild-packages/src/disk_cache/gc.rs +++ b/crates/fbuild-packages/src/disk_cache/gc.rs @@ -8,7 +8,7 @@ use super::budget::CacheBudget; use super::index::CacheIndex; -use std::path::Path; +use std::path::{Path, PathBuf}; /// Report of a GC run. #[derive(Debug, Clone, Default)] @@ -85,14 +85,15 @@ pub fn run_gc(index: &CacheIndex, budget: &CacheBudget) -> rusqlite::Result budget.archive_budget { + if archive_bytes > budget.archive_budget || total_bytes > budget.high_watermark { let entries = index.lru_archive_entries(1000)?; for entry in entries { - if archive_bytes <= budget.archive_budget { + if archive_bytes <= budget.archive_budget && total_bytes <= budget.high_watermark { break; } let bytes = entry.archive_bytes.unwrap_or(0) as u64; @@ -110,6 +111,7 @@ pub fn run_gc(index: &CacheIndex, budget: &CacheBudget) -> rusqlite::Result rusqlite::Result { } } - // Phase 2: walk filesystem, remove files not in the index - // Walk archives and installed roots + // Phase 2: walk filesystem, remove orphan files not in the index + // Collect all known relative paths from the index for membership checks + let all_entries = index.all_entries()?; + let mut known_paths = std::collections::HashSet::new(); + for entry in &all_entries { + if let Some(ref p) = entry.archive_path { + known_paths.insert(cache_root.join(p)); + } + if let Some(ref p) = entry.installed_path { + known_paths.insert(cache_root.join(p)); + } + } + for phase_root in &[ super::paths::archives_root(&cache_root), super::paths::installed_root(&cache_root), @@ -169,11 +182,48 @@ pub fn reconcile(index: &CacheIndex) -> rusqlite::Result { continue; } remove_partial_dirs(phase_root, &mut report); + remove_orphan_entries(phase_root, &known_paths, &mut report); } Ok(report) } +/// Remove leaf directories (version-level) that are not tracked by the index. +fn remove_orphan_entries( + root: &Path, + known_paths: &std::collections::HashSet, + report: &mut GcReport, +) { + // Walk two levels: kind/stem/hash/version + if let Ok(entries) = walkdir_sync(root) { + for path in entries { + if !path.is_dir() { + continue; + } + // Skip .partial dirs (handled separately) + if path + .file_name() + .map(|n| n.to_string_lossy().ends_with(".partial")) + .unwrap_or(false) + { + continue; + } + // Only remove leaf dirs (those with no subdirectories) + let has_subdirs = std::fs::read_dir(&path) + .map(|rd| rd.filter_map(|e| e.ok()).any(|e| e.path().is_dir())) + .unwrap_or(true); + if has_subdirs { + continue; + } + // If this leaf dir isn't known to the index, it's an orphan + if !known_paths.contains(&path) { + let _ = std::fs::remove_dir_all(&path); + report.orphan_files_removed += 1; + } + } + } +} + /// Remove any `.partial` directories (incomplete downloads/installs). fn remove_partial_dirs(root: &Path, report: &mut GcReport) { if let Ok(walker) = walkdir_sync(root) { @@ -352,7 +402,7 @@ mod tests { 5000, ) .unwrap(); - idx.pin(entry.id, std::process::id()).unwrap(); + idx.pin(entry.id, std::process::id(), 1).unwrap(); // Budget is tiny — would normally evict let budget = make_budget(100, 100, 200); diff --git a/crates/fbuild-packages/src/disk_cache/index.rs b/crates/fbuild-packages/src/disk_cache/index.rs index 8a79bb75..ed626d15 100644 --- a/crates/fbuild-packages/src/disk_cache/index.rs +++ b/crates/fbuild-packages/src/disk_cache/index.rs @@ -135,10 +135,12 @@ impl CacheIndex { ON entries(last_used_at) WHERE archive_path IS NOT NULL; CREATE TABLE IF NOT EXISTS leases ( - entry_id INTEGER NOT NULL REFERENCES entries(id) ON DELETE CASCADE, - holder_pid INTEGER NOT NULL, - acquired_at INTEGER NOT NULL, - PRIMARY KEY(entry_id, holder_pid) + entry_id INTEGER NOT NULL REFERENCES entries(id) ON DELETE CASCADE, + holder_pid INTEGER NOT NULL, + holder_nonce INTEGER NOT NULL DEFAULT 0, + refcount INTEGER NOT NULL DEFAULT 1, + acquired_at INTEGER NOT NULL, + PRIMARY KEY(entry_id, holder_pid, holder_nonce) );", )?; @@ -290,29 +292,46 @@ impl CacheIndex { } /// Increment the pinned count for an entry (lease acquired). - pub fn pin(&self, entry_id: i64, holder_pid: u32) -> rusqlite::Result<()> { + /// Uses a per-(PID, nonce) refcount so multiple `Lease` guards in the same + /// process correctly track independent pins. + pub fn pin(&self, entry_id: i64, holder_pid: u32, holder_nonce: u64) -> rusqlite::Result<()> { let now = Self::now_epoch(); let conn = self.conn.lock().unwrap(); - conn.execute( - "INSERT OR IGNORE INTO leases (entry_id, holder_pid, acquired_at) VALUES (?1, ?2, ?3)", - params![entry_id, holder_pid as i64, now], + let updated = conn.execute( + "UPDATE leases SET refcount = refcount + 1 + WHERE entry_id = ?1 AND holder_pid = ?2 AND holder_nonce = ?3", + params![entry_id, holder_pid as i64, holder_nonce as i64], )?; + if updated == 0 { + conn.execute( + "INSERT INTO leases (entry_id, holder_pid, holder_nonce, refcount, acquired_at) + VALUES (?1, ?2, ?3, 1, ?4)", + params![entry_id, holder_pid as i64, holder_nonce as i64, now], + )?; + } conn.execute( - "UPDATE entries SET pinned = (SELECT COUNT(*) FROM leases WHERE entry_id = ?1) WHERE id = ?1", + "UPDATE entries SET pinned = (SELECT COALESCE(SUM(refcount), 0) FROM leases WHERE entry_id = ?1) WHERE id = ?1", params![entry_id], )?; Ok(()) } /// Decrement the pinned count for an entry (lease released). - pub fn unpin(&self, entry_id: i64, holder_pid: u32) -> rusqlite::Result<()> { + /// Decrements refcount; removes the row when it reaches zero. + pub fn unpin(&self, entry_id: i64, holder_pid: u32, holder_nonce: u64) -> rusqlite::Result<()> { let conn = self.conn.lock().unwrap(); conn.execute( - "DELETE FROM leases WHERE entry_id = ?1 AND holder_pid = ?2", - params![entry_id, holder_pid as i64], + "UPDATE leases SET refcount = refcount - 1 + WHERE entry_id = ?1 AND holder_pid = ?2 AND holder_nonce = ?3", + params![entry_id, holder_pid as i64, holder_nonce as i64], + )?; + conn.execute( + "DELETE FROM leases + WHERE entry_id = ?1 AND holder_pid = ?2 AND holder_nonce = ?3 AND refcount <= 0", + params![entry_id, holder_pid as i64, holder_nonce as i64], )?; conn.execute( - "UPDATE entries SET pinned = (SELECT COUNT(*) FROM leases WHERE entry_id = ?1) WHERE id = ?1", + "UPDATE entries SET pinned = (SELECT COALESCE(SUM(refcount), 0) FROM leases WHERE entry_id = ?1) WHERE id = ?1", params![entry_id], )?; Ok(()) @@ -643,7 +662,7 @@ mod tests { .unwrap(); assert_eq!(entry.pinned, 0); - idx.pin(entry.id, 12345).unwrap(); + idx.pin(entry.id, 12345, 1).unwrap(); let entry = idx .lookup(Kind::Packages, "https://example.com/a", "1.0") .unwrap() @@ -651,7 +670,7 @@ mod tests { assert_eq!(entry.pinned, 1); // Pin with another PID - idx.pin(entry.id, 67890).unwrap(); + idx.pin(entry.id, 67890, 2).unwrap(); let entry = idx .lookup(Kind::Packages, "https://example.com/a", "1.0") .unwrap() @@ -659,7 +678,7 @@ mod tests { assert_eq!(entry.pinned, 2); // Unpin one - idx.unpin(entry.id, 12345).unwrap(); + idx.unpin(entry.id, 12345, 1).unwrap(); let entry = idx .lookup(Kind::Packages, "https://example.com/a", "1.0") .unwrap() @@ -692,7 +711,7 @@ mod tests { .unwrap(); // Pin e1 - idx.pin(e1.id, 99999).unwrap(); + idx.pin(e1.id, 99999, 1).unwrap(); // LRU should only return e2 (unpinned) let lru = idx.lru_installed_entries(10).unwrap(); diff --git a/crates/fbuild-packages/src/disk_cache/lease.rs b/crates/fbuild-packages/src/disk_cache/lease.rs index e0a5d0d8..34dbe2ce 100644 --- a/crates/fbuild-packages/src/disk_cache/lease.rs +++ b/crates/fbuild-packages/src/disk_cache/lease.rs @@ -1,27 +1,53 @@ //! RAII lease guard that pins cache entries during builds. //! //! Holding a `Lease` prevents the GC from evicting the associated entry. -//! The lease is released on drop. +//! The lease is released on drop. Multiple leases from the same process +//! correctly maintain independent refcounts via a per-process nonce. use super::index::CacheIndex; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; +/// Per-process nonce, initialized once at startup to a value derived from +/// the process start time. Guards against PID reuse: a recycled PID will +/// have a different nonce and won't collide with stale lease rows. +fn process_nonce() -> u64 { + static NONCE: AtomicU64 = AtomicU64::new(0); + let val = NONCE.load(Ordering::Relaxed); + if val != 0 { + return val; + } + // Use current time as a nonce — unique enough to distinguish PID reuse. + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() as u64; + // Race is benign: worst case two threads compute different nonces, + // one wins, and both use the winner's value on subsequent calls. + let nonce = nonce.max(1); // ensure non-zero + NONCE.store(nonce, Ordering::Relaxed); + nonce +} + /// RAII guard that pins a cache entry. Released on drop. pub struct Lease { index: Arc, entry_id: i64, holder_pid: u32, + holder_nonce: u64, } impl Lease { /// Acquire a lease for the given entry. pub fn acquire(index: Arc, entry_id: i64) -> rusqlite::Result { let pid = std::process::id(); - index.pin(entry_id, pid)?; + let nonce = process_nonce(); + index.pin(entry_id, pid, nonce)?; Ok(Self { index, entry_id, holder_pid: pid, + holder_nonce: nonce, }) } @@ -33,7 +59,9 @@ impl Lease { impl Drop for Lease { fn drop(&mut self) { - let _ = self.index.unpin(self.entry_id, self.holder_pid); + let _ = self + .index + .unpin(self.entry_id, self.holder_pid, self.holder_nonce); } } @@ -114,4 +142,56 @@ mod tests { let lru = idx.lru_installed_entries(10).unwrap(); assert!(lru.is_empty(), "leased entry should not appear in LRU list"); } + + #[test] + fn test_multiple_leases_same_pid_independent_refcount() { + let idx = Arc::new(CacheIndex::open_in_memory().unwrap()); + let entry = idx + .record_install( + super::super::paths::Kind::Packages, + "https://example.com/multi", + "1.0", + "path_multi", + 1000, + ) + .unwrap(); + + // Acquire two leases for the same entry from the same process + let lease1 = Lease::acquire(Arc::clone(&idx), entry.id).unwrap(); + let lease2 = Lease::acquire(Arc::clone(&idx), entry.id).unwrap(); + + let e = idx + .lookup( + super::super::paths::Kind::Packages, + "https://example.com/multi", + "1.0", + ) + .unwrap() + .unwrap(); + assert_eq!(e.pinned, 2, "two leases should give pinned=2"); + + // Drop one lease — should still be pinned with count=1 + drop(lease1); + let e = idx + .lookup( + super::super::paths::Kind::Packages, + "https://example.com/multi", + "1.0", + ) + .unwrap() + .unwrap(); + assert_eq!(e.pinned, 1, "one lease dropped, pinned should be 1"); + + // Drop the second — fully unpinned + drop(lease2); + let e = idx + .lookup( + super::super::paths::Kind::Packages, + "https://example.com/multi", + "1.0", + ) + .unwrap() + .unwrap(); + assert_eq!(e.pinned, 0, "both leases dropped, pinned should be 0"); + } } diff --git a/crates/fbuild-packages/src/disk_cache/mod.rs b/crates/fbuild-packages/src/disk_cache/mod.rs index 7b7fe570..1c92fb5d 100644 --- a/crates/fbuild-packages/src/disk_cache/mod.rs +++ b/crates/fbuild-packages/src/disk_cache/mod.rs @@ -48,9 +48,16 @@ impl DiskCache { let index = CacheIndex::open(cache_root)?; let budget = CacheBudget::compute(cache_root); - // Ensure phase directories exist - std::fs::create_dir_all(paths::archives_root(cache_root)).ok(); - std::fs::create_dir_all(paths::installed_root(cache_root)).ok(); + // Ensure phase directories exist — propagate failures so callers + // don't silently operate on an unusable cache layout. + let map_io = |e: std::io::Error| { + rusqlite::Error::InvalidPath(PathBuf::from(format!( + "failed to create cache phase dir: {}", + e + ))) + }; + std::fs::create_dir_all(paths::archives_root(cache_root)).map_err(&map_io)?; + std::fs::create_dir_all(paths::installed_root(cache_root)).map_err(map_io)?; Ok(Self { index: Arc::new(index), @@ -113,8 +120,11 @@ impl DiskCache { } /// Run a full GC pass. + /// Recomputes budgets from current disk space so long-lived processes + /// don't enforce stale watermarks. pub fn run_gc(&self) -> rusqlite::Result { - gc::run_gc(&self.index, &self.budget) + let budget = CacheBudget::compute(&self.cache_root); + gc::run_gc(&self.index, &budget) } /// Reconcile index against filesystem (run on daemon startup). diff --git a/crates/fbuild-packages/src/disk_cache/paths.rs b/crates/fbuild-packages/src/disk_cache/paths.rs index 5b34ecff..a1182724 100644 --- a/crates/fbuild-packages/src/disk_cache/paths.rs +++ b/crates/fbuild-packages/src/disk_cache/paths.rs @@ -64,6 +64,12 @@ pub fn stem_and_hash(url: &str) -> (String, String) { (url_stem(url), hash_url(url)) } +/// Sanitize a path component to prevent directory traversal. +/// Strips path separators, `.` and `..` sequences, and null bytes. +fn sanitize_component(s: &str) -> String { + s.replace(['/', '\\', '\0'], "_").replace("..", "_") +} + /// Root of the archives phase: `{cache_root}/archives/` pub fn archives_root(cache_root: &Path) -> PathBuf { cache_root.join("archives") @@ -87,42 +93,46 @@ pub fn gc_lock_path(cache_root: &Path) -> PathBuf { /// Archive entry path: `{cache_root}/archives/{kind}/{stem}/{hash}/{version}/` pub fn archive_entry_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { let (stem, hash) = stem_and_hash(url); + let safe_version = sanitize_component(version); archives_root(cache_root) .join(kind.as_str()) .join(stem) .join(hash) - .join(version) + .join(safe_version) } /// Staging path for an in-progress archive download. /// Uses `.partial` suffix so reconciliation can identify incomplete downloads. pub fn archive_staging_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { let (stem, hash) = stem_and_hash(url); + let safe_version = sanitize_component(version); archives_root(cache_root) .join(kind.as_str()) .join(stem) .join(hash) - .join(format!("{}.partial", version)) + .join(format!("{}.partial", safe_version)) } /// Installed entry path: `{cache_root}/installed/{kind}/{stem}/{hash}/{version}/` pub fn installed_entry_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { let (stem, hash) = stem_and_hash(url); + let safe_version = sanitize_component(version); installed_root(cache_root) .join(kind.as_str()) .join(stem) .join(hash) - .join(version) + .join(safe_version) } /// Staging path for an in-progress extraction. pub fn install_staging_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { let (stem, hash) = stem_and_hash(url); + let safe_version = sanitize_component(version); installed_root(cache_root) .join(kind.as_str()) .join(stem) .join(hash) - .join(format!("{}.partial", version)) + .join(format!("{}.partial", safe_version)) } /// Sentinel file that marks a completed installation. @@ -220,4 +230,39 @@ mod tests { assert_eq!(stem, url_stem(url)); assert_eq!(hash, hash_url(url)); } + + #[test] + fn test_version_traversal_sanitized() { + let tmp = tempfile::TempDir::new().unwrap(); + let root = tmp.path(); + let url = "https://example.com/pkg"; + + // Path traversal attempt should be sanitized + let dir = archive_entry_dir(root, Kind::Packages, url, "../../etc/passwd"); + let dir_str = dir.to_string_lossy(); + assert!( + !dir_str.contains(".."), + "path traversal not sanitized: {}", + dir_str + ); + // Ensure the path stays under the cache root + assert!( + dir.starts_with(root), + "path escaped cache root: {}", + dir_str + ); + + // Backslash traversal + let dir = installed_entry_dir(root, Kind::Packages, url, r"1.0\..\..\etc"); + let dir_str = dir.to_string_lossy(); + assert!( + !dir_str.contains(".."), + "backslash traversal not sanitized: {}", + dir_str + ); + + // Normal versions are unchanged + let dir = archive_entry_dir(root, Kind::Packages, url, "1.2.3"); + assert!(dir.to_string_lossy().contains("1.2.3")); + } } diff --git a/crates/fbuild-packages/src/lib.rs b/crates/fbuild-packages/src/lib.rs index b7a87d56..ec49ea03 100644 --- a/crates/fbuild-packages/src/lib.rs +++ b/crates/fbuild-packages/src/lib.rs @@ -20,6 +20,26 @@ use std::collections::HashMap; use std::future::Future; use std::path::{Path, PathBuf}; +/// Recursively compute the total size of a directory in bytes. +fn dir_size(path: &Path) -> u64 { + std::fs::read_dir(path) + .into_iter() + .flatten() + .filter_map(|e| e.ok()) + .map(|e| { + let meta = e.metadata().unwrap_or_else(|_| { + // Fallback: zero-size if metadata fails + std::fs::metadata(e.path()).unwrap_or_else(|_| unreachable!()) + }); + if meta.is_dir() { + dir_size(&e.path()) + } else { + meta.len() + } + }) + .sum() +} + pub(crate) fn block_on_package_future(future: F) -> fbuild_core::Result where F: Future>, @@ -149,6 +169,8 @@ pub struct PackageBase { pub cache: Cache, /// Cache subdirectory: "toolchains" or "platforms". pub cache_subdir: CacheSubdir, + /// Optional DiskCache for LRU tracking. Best-effort: `None` if SQLite open fails. + disk_cache: Option, } /// Which cache subdirectory to use. @@ -158,6 +180,15 @@ pub enum CacheSubdir { Platforms, } +impl From for disk_cache::Kind { + fn from(subdir: CacheSubdir) -> Self { + match subdir { + CacheSubdir::Toolchains => disk_cache::Kind::Toolchains, + CacheSubdir::Platforms => disk_cache::Kind::Platforms, + } + } +} + impl PackageBase { pub fn new( name: &str, @@ -168,14 +199,17 @@ impl PackageBase { cache_subdir: CacheSubdir, project_dir: &Path, ) -> Self { + let cache = Cache::new(project_dir); + let disk_cache = DiskCache::open().ok(); Self { name: name.to_string(), version: version.to_string(), url: url.to_string(), cache_key: cache_key.to_string(), checksum: checksum.map(|s| s.to_string()), - cache: Cache::new(project_dir), + cache, cache_subdir, + disk_cache, } } @@ -191,6 +225,7 @@ impl PackageBase { project_dir: &Path, cache_root: &Path, ) -> Self { + let disk_cache = DiskCache::open_at(cache_root).ok(); Self { name: name.to_string(), version: version.to_string(), @@ -199,6 +234,7 @@ impl PackageBase { checksum: checksum.map(|s| s.to_string()), cache: Cache::with_cache_root(project_dir, cache_root), cache_subdir, + disk_cache, } } @@ -213,9 +249,42 @@ impl PackageBase { } /// Check if already installed in cache. + /// On a cache hit, bumps the LRU timestamp in the DiskCache index. pub fn is_cached(&self) -> bool { let path = self.install_path(); - path.exists() && path.is_dir() + let cached = path.exists() && path.is_dir(); + if cached { + self.touch_disk_cache(); + } + cached + } + + /// Best-effort LRU touch in the DiskCache index. + fn touch_disk_cache(&self) { + if let Some(ref dc) = self.disk_cache { + let kind = self.cache_subdir.into(); + if let Ok(Some(entry)) = dc.lookup(kind, &self.cache_key, &self.version) { + let _ = dc.touch(&entry); + } + } + } + + /// Best-effort: record a completed install in the DiskCache index. + fn record_install_in_disk_cache(&self, install_path: &Path) { + if let Some(ref dc) = self.disk_cache { + let kind = self.cache_subdir.into(); + let installed_bytes = dir_size(install_path) as i64; + let rel_path = install_path + .strip_prefix(dc.cache_root()) + .unwrap_or(install_path); + let _ = dc.record_install( + kind, + &self.cache_key, + &self.version, + &rel_path.to_string_lossy(), + installed_bytes, + ); + } } /// Download and install with staged directory pattern. @@ -284,6 +353,18 @@ impl PackageBase { fbuild_core::FbuildError::PackageError(format!("failed to commit installation: {}", e)) })?; + // Write sentinel so GC reconciliation recognizes this as a complete install + let sentinel = disk_cache::paths::install_complete_sentinel(&install_path); + std::fs::write(&sentinel, b"").map_err(|e| { + fbuild_core::FbuildError::PackageError(format!( + "failed to write install sentinel: {}", + e + )) + })?; + + // Best-effort: record in DiskCache index for LRU tracking + self.record_install_in_disk_cache(&install_path); + tracing::info!("installed {} v{}", self.name, self.version); Ok(install_path) } From b638357557c86669a8d28a2e7f93eb82fc43dd91 Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 23:05:00 -0700 Subject: [PATCH 03/12] feat(cache): add lookup_latest and toolchain_metadata_dir helpers Add DiskCache::lookup_latest() to find the most recently used installed entry for a kind+url pair regardless of version. Add Cache::toolchain_metadata_dir() for toolchain metadata storage. These helpers prepare for migrating callers from Cache to DiskCache. --- crates/fbuild-packages/src/cache.rs | 8 ++++++++ .../fbuild-packages/src/disk_cache/index.rs | 20 +++++++++++++++++++ crates/fbuild-packages/src/disk_cache/mod.rs | 6 ++++++ 3 files changed, 34 insertions(+) diff --git a/crates/fbuild-packages/src/cache.rs b/crates/fbuild-packages/src/cache.rs index 227e90df..34546414 100644 --- a/crates/fbuild-packages/src/cache.rs +++ b/crates/fbuild-packages/src/cache.rs @@ -142,6 +142,14 @@ impl Cache { Ok(()) } + /// Get a toolchain metadata cache directory by name. + /// + /// Returns `{cache_root}/toolchains/{toolchain_name}` — used by + /// `esp32_metadata::resolve_toolchain_url_sync` to store `tools.json`. + pub fn toolchain_metadata_dir(&self, toolchain_name: &str) -> PathBuf { + self.toolchains_dir().join(toolchain_name) + } + /// Clean build directory for an environment and profile. pub fn clean_build(&self, env_name: &str, profile: BuildProfile) -> std::io::Result<()> { let build_dir = self.get_build_dir(env_name, profile); diff --git a/crates/fbuild-packages/src/disk_cache/index.rs b/crates/fbuild-packages/src/disk_cache/index.rs index ed626d15..be8f3f76 100644 --- a/crates/fbuild-packages/src/disk_cache/index.rs +++ b/crates/fbuild-packages/src/disk_cache/index.rs @@ -468,6 +468,26 @@ impl CacheIndex { Ok(entries) } + /// Look up the most recently used entry for a kind+url pair (any version). + /// Returns the entry with the highest `last_used_at` that has an installed path. + pub fn lookup_latest(&self, kind: Kind, url: &str) -> rusqlite::Result> { + let (_, hash) = paths::stem_and_hash(url); + let conn = self.conn.lock().unwrap(); + conn.query_row( + "SELECT id, kind, url, stem, hash, version, + archive_path, archive_bytes, archive_sha256, + installed_path, installed_bytes, installed_at, + archived_at, last_used_at, use_count, pinned + FROM entries + WHERE kind = ?1 AND hash = ?2 AND installed_path IS NOT NULL + ORDER BY last_used_at DESC + LIMIT 1", + params![kind.as_str(), hash], + Self::row_to_entry, + ) + .optional() + } + /// Get total entry count. pub fn entry_count(&self) -> rusqlite::Result { let conn = self.conn.lock().unwrap(); diff --git a/crates/fbuild-packages/src/disk_cache/mod.rs b/crates/fbuild-packages/src/disk_cache/mod.rs index 1c92fb5d..3ef68807 100644 --- a/crates/fbuild-packages/src/disk_cache/mod.rs +++ b/crates/fbuild-packages/src/disk_cache/mod.rs @@ -173,6 +173,12 @@ impl DiskCache { pub fn install_staging_dir(&self, kind: Kind, url: &str, version: &str) -> PathBuf { paths::install_staging_dir(&self.cache_root, kind, url, version) } + + /// Look up the most recently used installed entry for a kind+url pair (any version). + /// Useful when you want to find *any* cached version of a package. + pub fn lookup_latest(&self, kind: Kind, url: &str) -> rusqlite::Result> { + self.index.lookup_latest(kind, url) + } } /// Cache statistics for `fbuild status`. From f5ebbc5f1882f28d96e48590349a13d5368a9d8a Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 23:10:26 -0700 Subject: [PATCH 04/12] fix(cache): address CodeRabbit round-2 review feedback on PR #29 - Make dir_size() symlink-safe and non-panicking (use symlink_metadata) - Reject empty/"." from sanitize_component to prevent path collisions - Recompute budget on stats()/budget() calls for long-lived processes - Remove stale budget field from DiskCache struct - Skip recording legacy Cache paths in DiskCache index - Make sentinel write best-effort after atomic rename - Log unpin failures in Lease::Drop instead of silently discarding --- .../fbuild-packages/src/disk_cache/lease.rs | 12 ++++- crates/fbuild-packages/src/disk_cache/mod.rs | 14 +++--- .../fbuild-packages/src/disk_cache/paths.rs | 8 +++- crates/fbuild-packages/src/lib.rs | 48 +++++++++++-------- 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/crates/fbuild-packages/src/disk_cache/lease.rs b/crates/fbuild-packages/src/disk_cache/lease.rs index 34dbe2ce..2caae6d9 100644 --- a/crates/fbuild-packages/src/disk_cache/lease.rs +++ b/crates/fbuild-packages/src/disk_cache/lease.rs @@ -59,9 +59,17 @@ impl Lease { impl Drop for Lease { fn drop(&mut self) { - let _ = self + if let Err(e) = self .index - .unpin(self.entry_id, self.holder_pid, self.holder_nonce); + .unpin(self.entry_id, self.holder_pid, self.holder_nonce) + { + tracing::warn!( + "failed to unpin lease entry_id={} holder_pid={}: {}", + self.entry_id, + self.holder_pid, + e + ); + } } } diff --git a/crates/fbuild-packages/src/disk_cache/mod.rs b/crates/fbuild-packages/src/disk_cache/mod.rs index 3ef68807..ca365a88 100644 --- a/crates/fbuild-packages/src/disk_cache/mod.rs +++ b/crates/fbuild-packages/src/disk_cache/mod.rs @@ -33,7 +33,6 @@ use std::sync::Arc; pub struct DiskCache { index: Arc, cache_root: PathBuf, - budget: CacheBudget, } impl DiskCache { @@ -46,7 +45,6 @@ impl DiskCache { /// Open the disk cache at a specific root (for testing). pub fn open_at(cache_root: &Path) -> rusqlite::Result { let index = CacheIndex::open(cache_root)?; - let budget = CacheBudget::compute(cache_root); // Ensure phase directories exist — propagate failures so callers // don't silently operate on an unusable cache layout. @@ -62,7 +60,6 @@ impl DiskCache { Ok(Self { index: Arc::new(index), cache_root: cache_root.to_path_buf(), - budget, }) } @@ -133,12 +130,15 @@ impl DiskCache { } /// Get cache statistics. + /// Recomputes the budget from current disk space so callers always + /// see fresh watermarks. pub fn stats(&self) -> rusqlite::Result { + let budget = CacheBudget::compute(&self.cache_root); Ok(CacheStats { archive_bytes: self.index.total_archive_bytes()? as u64, installed_bytes: self.index.total_installed_bytes()? as u64, entry_count: self.index.entry_count()?, - budget: self.budget, + budget, }) } @@ -147,9 +147,9 @@ impl DiskCache { &self.cache_root } - /// Get the computed budget. - pub fn budget(&self) -> &CacheBudget { - &self.budget + /// Get the current budget, recomputed from live disk space. + pub fn budget(&self) -> CacheBudget { + CacheBudget::compute(&self.cache_root) } // --- Path helpers for callers --- diff --git a/crates/fbuild-packages/src/disk_cache/paths.rs b/crates/fbuild-packages/src/disk_cache/paths.rs index a1182724..21a42ab6 100644 --- a/crates/fbuild-packages/src/disk_cache/paths.rs +++ b/crates/fbuild-packages/src/disk_cache/paths.rs @@ -66,8 +66,14 @@ pub fn stem_and_hash(url: &str) -> (String, String) { /// Sanitize a path component to prevent directory traversal. /// Strips path separators, `.` and `..` sequences, and null bytes. +/// Returns `"_"` if the result would be empty or `"."`. fn sanitize_component(s: &str) -> String { - s.replace(['/', '\\', '\0'], "_").replace("..", "_") + let sanitized = s.replace(['/', '\\', '\0'], "_").replace("..", "_"); + if sanitized.is_empty() || sanitized == "." { + "_".to_string() + } else { + sanitized + } } /// Root of the archives phase: `{cache_root}/archives/` diff --git a/crates/fbuild-packages/src/lib.rs b/crates/fbuild-packages/src/lib.rs index ec49ea03..2c33d406 100644 --- a/crates/fbuild-packages/src/lib.rs +++ b/crates/fbuild-packages/src/lib.rs @@ -21,20 +21,24 @@ use std::future::Future; use std::path::{Path, PathBuf}; /// Recursively compute the total size of a directory in bytes. +/// +/// Symlink-safe: uses `symlink_metadata` and skips symlinks to avoid +/// infinite recursion. Tolerates permission errors by treating +/// inaccessible entries as zero-size. fn dir_size(path: &Path) -> u64 { - std::fs::read_dir(path) - .into_iter() - .flatten() + let Ok(entries) = std::fs::read_dir(path) else { + return 0; + }; + entries .filter_map(|e| e.ok()) - .map(|e| { - let meta = e.metadata().unwrap_or_else(|_| { - // Fallback: zero-size if metadata fails - std::fs::metadata(e.path()).unwrap_or_else(|_| unreachable!()) - }); - if meta.is_dir() { - dir_size(&e.path()) + .filter_map(|e| { + let meta = std::fs::symlink_metadata(e.path()).ok()?; + if meta.is_symlink() { + None // skip symlinks to avoid cycles + } else if meta.is_dir() { + Some(dir_size(&e.path())) } else { - meta.len() + Some(meta.len()) } }) .sum() @@ -270,13 +274,16 @@ impl PackageBase { } /// Best-effort: record a completed install in the DiskCache index. + /// Only records if `install_path` is under the DiskCache root — legacy + /// Cache paths are skipped to avoid indexing absolute legacy paths. fn record_install_in_disk_cache(&self, install_path: &Path) { if let Some(ref dc) = self.disk_cache { + let rel_path = match install_path.strip_prefix(dc.cache_root()) { + Ok(rel) => rel, + Err(_) => return, // legacy path outside DiskCache root + }; let kind = self.cache_subdir.into(); let installed_bytes = dir_size(install_path) as i64; - let rel_path = install_path - .strip_prefix(dc.cache_root()) - .unwrap_or(install_path); let _ = dc.record_install( kind, &self.cache_key, @@ -353,14 +360,13 @@ impl PackageBase { fbuild_core::FbuildError::PackageError(format!("failed to commit installation: {}", e)) })?; - // Write sentinel so GC reconciliation recognizes this as a complete install + // Write sentinel so GC reconciliation recognizes this as a complete install. + // Best-effort: the install succeeded (rename was atomic), so a sentinel + // failure should not cause the caller to see an error. let sentinel = disk_cache::paths::install_complete_sentinel(&install_path); - std::fs::write(&sentinel, b"").map_err(|e| { - fbuild_core::FbuildError::PackageError(format!( - "failed to write install sentinel: {}", - e - )) - })?; + if let Err(e) = std::fs::write(&sentinel, b"") { + tracing::warn!("failed to write install sentinel: {}", e); + } // Best-effort: record in DiskCache index for LRU tracking self.record_install_in_disk_cache(&install_path); From 8d03f8b57f61fcae7535f45119c7da3ac84d55f4 Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 23:20:18 -0700 Subject: [PATCH 05/12] feat(cache): add background GC loop, cache stats and GC CLI commands - Daemon: background GC loop runs every 5 minutes (60s initial delay) - Daemon: GET /api/cache/stats and POST /api/cache/gc endpoints - CLI: `fbuild daemon cache-stats` shows disk cache statistics - CLI: `fbuild daemon gc` triggers garbage collection - Both commands fall back to local DiskCache when daemon is not running --- crates/fbuild-cli/src/daemon_client.rs | 54 +++++++++ crates/fbuild-cli/src/main.rs | 125 +++++++++++++++++++++ crates/fbuild-daemon/src/handlers/cache.rs | 63 +++++++++++ crates/fbuild-daemon/src/handlers/mod.rs | 1 + crates/fbuild-daemon/src/main.rs | 54 ++++++++- crates/fbuild-daemon/src/models.rs | 28 +++++ 6 files changed, 324 insertions(+), 1 deletion(-) create mode 100644 crates/fbuild-daemon/src/handlers/cache.rs diff --git a/crates/fbuild-cli/src/daemon_client.rs b/crates/fbuild-cli/src/daemon_client.rs index fb5b6a77..cbd19811 100644 --- a/crates/fbuild-cli/src/daemon_client.rs +++ b/crates/fbuild-cli/src/daemon_client.rs @@ -273,6 +273,30 @@ pub struct ClearLocksResponse { pub message: String, } +#[derive(Debug, Deserialize)] +pub struct CacheStatsResponse { + pub success: bool, + pub archive_bytes: u64, + pub installed_bytes: u64, + pub total_bytes: u64, + pub entry_count: i64, + pub high_watermark: u64, + pub low_watermark: u64, + pub archive_budget: u64, + pub message: Option, +} + +#[derive(Debug, Deserialize)] +pub struct GcResponse { + pub success: bool, + pub installed_evicted: u64, + pub installed_bytes_freed: u64, + pub archives_evicted: u64, + pub archive_bytes_freed: u64, + pub total_bytes_freed: u64, + pub message: Option, +} + #[derive(Debug, Deserialize)] pub struct HealthResponseFull { #[allow(dead_code)] @@ -660,6 +684,36 @@ impl DaemonClient { .map_err(|e| fbuild_core::FbuildError::DaemonError(format!("invalid response: {}", e))) } + /// Get cache statistics from the daemon. + pub async fn cache_stats(&self) -> fbuild_core::Result { + let resp = self + .client + .get(format!("{}/api/cache/stats", self.base_url)) + .timeout(std::time::Duration::from_secs(5)) + .send() + .await + .map_err(|e| fbuild_core::FbuildError::DaemonError(format!("request failed: {}", e)))?; + + resp.json::() + .await + .map_err(|e| fbuild_core::FbuildError::DaemonError(format!("invalid response: {}", e))) + } + + /// Trigger a GC run on the daemon. + pub async fn run_gc(&self) -> fbuild_core::Result { + let resp = self + .client + .post(format!("{}/api/cache/gc", self.base_url)) + .timeout(std::time::Duration::from_secs(30)) + .send() + .await + .map_err(|e| fbuild_core::FbuildError::DaemonError(format!("request failed: {}", e)))?; + + resp.json::() + .await + .map_err(|e| fbuild_core::FbuildError::DaemonError(format!("invalid response: {}", e))) + } + async fn post( &self, path: &str, diff --git a/crates/fbuild-cli/src/main.rs b/crates/fbuild-cli/src/main.rs index fa56d32a..98560200 100644 --- a/crates/fbuild-cli/src/main.rs +++ b/crates/fbuild-cli/src/main.rs @@ -344,6 +344,10 @@ enum DaemonAction { Locks, /// Clear stale locks ClearLocks, + /// Show disk cache statistics + CacheStats, + /// Run disk cache garbage collection + Gc, /// Tail daemon logs (alias for `fbuild show daemon`) Monitor { /// Don't follow the log file (just print last lines and exit) @@ -2329,6 +2333,12 @@ async fn run_daemon(action: DaemonAction) -> fbuild_core::Result<()> { DaemonAction::ClearLocks => { run_daemon_clear_locks(&client).await?; } + DaemonAction::CacheStats => { + run_daemon_cache_stats(&client).await?; + } + DaemonAction::Gc => { + run_daemon_gc(&client).await?; + } DaemonAction::Monitor { no_follow, lines } => { return run_show("daemon", !no_follow, lines); } @@ -2437,6 +2447,121 @@ async fn run_daemon_clear_locks(client: &DaemonClient) -> fbuild_core::Result<() Ok(()) } +async fn run_daemon_cache_stats(client: &DaemonClient) -> fbuild_core::Result<()> { + if !client.health().await { + // Fall back to local cache stats if daemon isn't running + match fbuild_packages::DiskCache::open() { + Ok(dc) => { + let stats = dc.stats().map_err(|e| { + fbuild_core::FbuildError::Other(format!("failed to read cache stats: {}", e)) + })?; + println!("{}", stats); + } + Err(e) => { + eprintln!("failed to open disk cache: {}", e); + } + } + return Ok(()); + } + + let stats = client.cache_stats().await?; + if !stats.success { + eprintln!( + "failed to get cache stats: {}", + stats.message.as_deref().unwrap_or("unknown error") + ); + return Ok(()); + } + println!("Disk Cache Statistics:"); + println!(" Entries: {}", stats.entry_count); + println!(" Installed: {}", format_size(stats.installed_bytes)); + println!(" Archives: {}", format_size(stats.archive_bytes)); + println!(" Total: {}", format_size(stats.total_bytes)); + println!( + " Watermarks: {} high / {} low", + format_size(stats.high_watermark), + format_size(stats.low_watermark) + ); + println!(" Archive budget: {}", format_size(stats.archive_budget)); + Ok(()) +} + +async fn run_daemon_gc(client: &DaemonClient) -> fbuild_core::Result<()> { + if !client.health().await { + // Fall back to local GC if daemon isn't running + match fbuild_packages::DiskCache::open() { + Ok(dc) => match dc.run_gc() { + Ok(report) => { + print_gc_report(&report); + } + Err(e) => { + eprintln!("GC failed: {}", e); + } + }, + Err(e) => { + eprintln!("failed to open disk cache: {}", e); + } + } + return Ok(()); + } + + let result = client.run_gc().await?; + if !result.success { + eprintln!( + "GC failed: {}", + result.message.as_deref().unwrap_or("unknown error") + ); + return Ok(()); + } + println!("GC complete:"); + println!( + " Installed evicted: {} ({})", + result.installed_evicted, + format_size(result.installed_bytes_freed) + ); + println!( + " Archives evicted: {} ({})", + result.archives_evicted, + format_size(result.archive_bytes_freed) + ); + println!( + " Total freed: {}", + format_size(result.total_bytes_freed) + ); + Ok(()) +} + +fn print_gc_report(report: &fbuild_packages::disk_cache::GcReport) { + if report.total_bytes_freed() == 0 + && report.orphan_files_removed == 0 + && report.orphan_rows_cleaned == 0 + { + println!("GC: nothing to clean up"); + return; + } + println!("GC complete:"); + println!( + " Installed evicted: {} ({})", + report.installed_evicted, + format_size(report.installed_bytes_freed) + ); + println!( + " Archives evicted: {} ({})", + report.archives_evicted, + format_size(report.archive_bytes_freed) + ); + println!( + " Total freed: {}", + format_size(report.total_bytes_freed()) + ); + if report.orphan_files_removed > 0 { + println!(" Orphan files removed: {}", report.orphan_files_removed); + } + if report.orphan_rows_cleaned > 0 { + println!(" Orphan rows cleaned: {}", report.orphan_rows_cleaned); + } +} + async fn run_daemon_kill( client: &DaemonClient, pid: Option, diff --git a/crates/fbuild-daemon/src/handlers/cache.rs b/crates/fbuild-daemon/src/handlers/cache.rs new file mode 100644 index 00000000..9ead7b2a --- /dev/null +++ b/crates/fbuild-daemon/src/handlers/cache.rs @@ -0,0 +1,63 @@ +//! Cache statistics and GC handlers. + +use crate::context::DaemonContext; +use crate::models::{CacheStatsResponse, GcResponse}; +use axum::extract::State; +use axum::Json; +use std::sync::Arc; + +/// GET /api/cache/stats +pub async fn cache_stats(State(_ctx): State>) -> Json { + match fbuild_packages::DiskCache::open() { + Ok(dc) => match dc.stats() { + Ok(stats) => Json(CacheStatsResponse { + success: true, + archive_bytes: stats.archive_bytes, + installed_bytes: stats.installed_bytes, + total_bytes: stats.total_bytes(), + entry_count: stats.entry_count, + high_watermark: stats.budget.high_watermark, + low_watermark: stats.budget.low_watermark, + archive_budget: stats.budget.archive_budget, + message: None, + }), + Err(e) => Json(CacheStatsResponse { + success: false, + message: Some(format!("failed to read cache stats: {}", e)), + ..Default::default() + }), + }, + Err(e) => Json(CacheStatsResponse { + success: false, + message: Some(format!("failed to open cache: {}", e)), + ..Default::default() + }), + } +} + +/// POST /api/cache/gc +pub async fn run_gc(State(_ctx): State>) -> Json { + match fbuild_packages::DiskCache::open() { + Ok(dc) => match dc.run_gc() { + Ok(report) => Json(GcResponse { + success: true, + installed_evicted: report.installed_evicted, + installed_bytes_freed: report.installed_bytes_freed, + archives_evicted: report.archives_evicted, + archive_bytes_freed: report.archive_bytes_freed, + total_bytes_freed: report.total_bytes_freed(), + message: None, + }), + Err(e) => Json(GcResponse { + success: false, + message: Some(format!("GC failed: {}", e)), + ..Default::default() + }), + }, + Err(e) => Json(GcResponse { + success: false, + message: Some(format!("failed to open cache: {}", e)), + ..Default::default() + }), + } +} diff --git a/crates/fbuild-daemon/src/handlers/mod.rs b/crates/fbuild-daemon/src/handlers/mod.rs index 429f8842..b8a9aeac 100644 --- a/crates/fbuild-daemon/src/handlers/mod.rs +++ b/crates/fbuild-daemon/src/handlers/mod.rs @@ -1,5 +1,6 @@ //! HTTP and WebSocket route handlers for the daemon. +pub mod cache; pub mod devices; pub mod emulator; pub mod health; diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index 6beb1268..abdaf23f 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -7,7 +7,7 @@ use clap::Parser; use fbuild_daemon::context::{ DaemonContext, IDLE_TIMEOUT, SELF_EVICTION_TIMEOUT, STALE_LOCK_CHECK_INTERVAL, }; -use fbuild_daemon::handlers::{devices, emulator, health, locks, operations, websockets}; +use fbuild_daemon::handlers::{cache, devices, emulator, health, locks, operations, websockets}; use std::sync::Arc; #[derive(Parser)] @@ -63,6 +63,8 @@ async fn main() { .route("/api/devices/:port/preempt", post(devices::device_preempt)) .route("/api/locks/status", get(locks::lock_status)) .route("/api/locks/clear", post(locks::clear_locks)) + .route("/api/cache/stats", get(cache::cache_stats)) + .route("/api/cache/gc", post(cache::run_gc)) .route("/api/install-deps", post(operations::install_deps)) .route("/api/reset", post(operations::reset)) .route("/api/test-emu", post(emulator::test_emu)) @@ -193,6 +195,40 @@ async fn main() { }); } + // Spawn background GC loop — runs every 5 minutes + { + tokio::spawn(async move { + // Wait 60s after startup before first GC to avoid slowing boot + tokio::time::sleep(std::time::Duration::from_secs(60)).await; + + let gc_interval = std::time::Duration::from_secs(300); + loop { + match fbuild_packages::DiskCache::open() { + Ok(dc) => match dc.run_gc() { + Ok(report) => { + if report.total_bytes_freed() > 0 { + tracing::info!( + "background GC: freed {} installed ({} entries) + {} archives ({} entries)", + format_bytes_compact(report.installed_bytes_freed), + report.installed_evicted, + format_bytes_compact(report.archive_bytes_freed), + report.archives_evicted, + ); + } + } + Err(e) => { + tracing::warn!("background GC failed: {}", e); + } + }, + Err(e) => { + tracing::debug!("background GC: could not open cache: {}", e); + } + } + tokio::time::sleep(gc_interval).await; + } + }); + } + // Clone refs so we can check operation state on Ctrl+C let shutdown_tx_signal = context.shutdown_tx.clone(); let op_in_progress = context.operation_in_progress.clone(); @@ -422,6 +458,22 @@ fn set_exclusive_address_windows(sock: &socket2::Socket) -> std::io::Result<()> } } +/// Compact byte formatter for log messages. +fn format_bytes_compact(bytes: u64) -> String { + const GIB: u64 = 1024 * 1024 * 1024; + const MIB: u64 = 1024 * 1024; + const KIB: u64 = 1024; + if bytes >= GIB { + format!("{:.1} GiB", bytes as f64 / GIB as f64) + } else if bytes >= MIB { + format!("{:.1} MiB", bytes as f64 / MIB as f64) + } else if bytes >= KIB { + format!("{:.1} KiB", bytes as f64 / KIB as f64) + } else { + format!("{} B", bytes) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/fbuild-daemon/src/models.rs b/crates/fbuild-daemon/src/models.rs index 549ff18e..ff8336b8 100644 --- a/crates/fbuild-daemon/src/models.rs +++ b/crates/fbuild-daemon/src/models.rs @@ -394,6 +394,34 @@ pub struct TestEmuRequest { pub pio_env: BTreeMap, } +/// GET /api/cache/stats response. +#[derive(Debug, Default, Serialize)] +pub struct CacheStatsResponse { + pub success: bool, + pub archive_bytes: u64, + pub installed_bytes: u64, + pub total_bytes: u64, + pub entry_count: i64, + pub high_watermark: u64, + pub low_watermark: u64, + pub archive_budget: u64, + #[serde(skip_serializing_if = "Option::is_none")] + pub message: Option, +} + +/// POST /api/cache/gc response. +#[derive(Debug, Default, Serialize)] +pub struct GcResponse { + pub success: bool, + pub installed_evicted: u64, + pub installed_bytes_freed: u64, + pub archives_evicted: u64, + pub archive_bytes_freed: u64, + pub total_bytes_freed: u64, + #[serde(skip_serializing_if = "Option::is_none")] + pub message: Option, +} + /// POST /api/reset request. #[derive(Debug, Deserialize)] pub struct ResetRequest { From 8b2ce4e5eeb23f066816cb42bfb443f82978097a Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 23:21:34 -0700 Subject: [PATCH 06/12] feat(cli): add `fbuild purge --gc` shortcut for LRU garbage collection --- crates/fbuild-cli/src/main.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/fbuild-cli/src/main.rs b/crates/fbuild-cli/src/main.rs index 98560200..e6c89976 100644 --- a/crates/fbuild-cli/src/main.rs +++ b/crates/fbuild-cli/src/main.rs @@ -194,6 +194,9 @@ enum Commands { dry_run: bool, #[arg(long)] project_dir: Option, + /// Run LRU garbage collection instead of full purge + #[arg(long)] + gc: bool, }, /// Manage the fbuild daemon Daemon { @@ -589,7 +592,14 @@ async fn main() { target, dry_run, project_dir, - }) => run_purge(target, dry_run, project_dir), + gc, + }) => { + if gc { + run_purge_gc() + } else { + run_purge(target, dry_run, project_dir) + } + } Some(Commands::Daemon { action }) => run_daemon(action).await, Some(Commands::Show { target, @@ -2021,6 +2031,22 @@ async fn run_clang_tool( } } +fn run_purge_gc() -> fbuild_core::Result<()> { + match fbuild_packages::DiskCache::open() { + Ok(dc) => match dc.run_gc() { + Ok(report) => { + print_gc_report(&report); + Ok(()) + } + Err(e) => Err(fbuild_core::FbuildError::Other(format!("GC failed: {}", e))), + }, + Err(e) => Err(fbuild_core::FbuildError::Other(format!( + "failed to open disk cache: {}", + e + ))), + } +} + fn run_purge( target: Option, dry_run: bool, From 00f087cc1fe7862dd5cb866b6b79a52b91ad8f1d Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 23:30:23 -0700 Subject: [PATCH 07/12] fix(cache): harden GC against TOCTOU races and symlink traversal - Remove .exists() checks before remove_dir_all/remove_path in GC, handling NotFound gracefully to eliminate TOCTOU race windows - Make walkdir_sync symlink-safe using symlink_metadata to prevent infinite recursion from symlink cycles and path escapes - Add startup reconciliation task in daemon to clean up partial installs and orphan files from crashed previous instances --- crates/fbuild-daemon/src/main.rs | 22 +++++++++++++++++ crates/fbuild-packages/src/disk_cache/gc.rs | 26 ++++++++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index abdaf23f..6e482da0 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -195,6 +195,28 @@ async fn main() { }); } + // Run startup reconciliation in a background task — cleans up partial + // installs and orphan files left by crashed previous instances. + { + tokio::spawn(async { + match fbuild_packages::DiskCache::open() { + Ok(dc) => match dc.reconcile() { + Ok(report) => { + if report.orphan_files_removed > 0 || report.orphan_rows_cleaned > 0 { + tracing::info!( + "startup reconciliation: cleaned {} orphan files, {} orphan rows", + report.orphan_files_removed, + report.orphan_rows_cleaned, + ); + } + } + Err(e) => tracing::warn!("startup reconciliation failed: {}", e), + }, + Err(e) => tracing::debug!("could not open cache for reconciliation: {}", e), + } + }); + } + // Spawn background GC loop — runs every 5 minutes { tokio::spawn(async move { diff --git a/crates/fbuild-packages/src/disk_cache/gc.rs b/crates/fbuild-packages/src/disk_cache/gc.rs index fdd99574..d4dd2aae 100644 --- a/crates/fbuild-packages/src/disk_cache/gc.rs +++ b/crates/fbuild-packages/src/disk_cache/gc.rs @@ -70,8 +70,10 @@ pub fn run_gc(index: &CacheIndex, budget: &CacheBudget) -> rusqlite::Result {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => { tracing::warn!("GC: failed to remove {}: {}", full_path.display(), e); continue; } @@ -101,8 +103,10 @@ pub fn run_gc(index: &CacheIndex, budget: &CacheBudget) -> rusqlite::Result {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => { tracing::warn!("GC: failed to remove {}: {}", full_path.display(), e); continue; } @@ -243,14 +247,24 @@ fn remove_partial_dirs(root: &Path, report: &mut GcReport) { } /// Simple recursive directory walker (no external dependency needed here). +/// Uses symlink_metadata to avoid following symlinks and prevent infinite recursion. fn walkdir_sync(root: &Path) -> std::io::Result> { let mut result = Vec::new(); if root.is_dir() { for entry in std::fs::read_dir(root)? { - let entry = entry?; + let entry = match entry { + Ok(e) => e, + Err(_) => continue, + }; let path = entry.path(); + // Use symlink_metadata to avoid following symlinks + let meta = match std::fs::symlink_metadata(&path) { + Ok(m) => m, + Err(_) => continue, + }; result.push(path.clone()); - if path.is_dir() { + // Only recurse into real directories, not symlinks + if meta.is_dir() { if let Ok(children) = walkdir_sync(&path) { result.extend(children); } From 208c64016d9fb7ce7a19cec01a9ab2b8c4dc6fc5 Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 23:41:54 -0700 Subject: [PATCH 08/12] fix(cache): address CodeRabbit round-4 review feedback on PR #29 - paths.rs: make version-to-path mapping collision-free by appending short hash when sanitization is lossy or name ends with .partial - mod.rs: clamp negative byte counts to 0 at API boundary - lease.rs: use OnceLock for process_nonce() single-assignment safety - cache.rs: validate toolchain_name to prevent directory traversal - models.rs: expose orphan_files_removed/orphan_rows_cleaned in GcResponse - handlers/cache.rs + main.rs: serialize background and manual GC with daemon-scoped mutex to prevent interleaved deletes - CLI: return non-zero exit code when cache-stats/gc commands fail --- crates/fbuild-cli/src/main.rs | 35 ++++++------ crates/fbuild-daemon/src/context.rs | 3 ++ crates/fbuild-daemon/src/handlers/cache.rs | 6 ++- crates/fbuild-daemon/src/main.rs | 3 ++ crates/fbuild-daemon/src/models.rs | 2 + crates/fbuild-packages/src/cache.rs | 15 +++++- .../fbuild-packages/src/disk_cache/lease.rs | 29 ++++------ crates/fbuild-packages/src/disk_cache/mod.rs | 10 ++-- .../fbuild-packages/src/disk_cache/paths.rs | 54 +++++++++++++++++-- 9 files changed, 110 insertions(+), 47 deletions(-) diff --git a/crates/fbuild-cli/src/main.rs b/crates/fbuild-cli/src/main.rs index e6c89976..de98dad8 100644 --- a/crates/fbuild-cli/src/main.rs +++ b/crates/fbuild-cli/src/main.rs @@ -2484,7 +2484,10 @@ async fn run_daemon_cache_stats(client: &DaemonClient) -> fbuild_core::Result<() println!("{}", stats); } Err(e) => { - eprintln!("failed to open disk cache: {}", e); + return Err(fbuild_core::FbuildError::Other(format!( + "failed to open disk cache: {}", + e + ))); } } return Ok(()); @@ -2492,11 +2495,10 @@ async fn run_daemon_cache_stats(client: &DaemonClient) -> fbuild_core::Result<() let stats = client.cache_stats().await?; if !stats.success { - eprintln!( + return Err(fbuild_core::FbuildError::Other(format!( "failed to get cache stats: {}", stats.message.as_deref().unwrap_or("unknown error") - ); - return Ok(()); + ))); } println!("Disk Cache Statistics:"); println!(" Entries: {}", stats.entry_count); @@ -2515,29 +2517,22 @@ async fn run_daemon_cache_stats(client: &DaemonClient) -> fbuild_core::Result<() async fn run_daemon_gc(client: &DaemonClient) -> fbuild_core::Result<()> { if !client.health().await { // Fall back to local GC if daemon isn't running - match fbuild_packages::DiskCache::open() { - Ok(dc) => match dc.run_gc() { - Ok(report) => { - print_gc_report(&report); - } - Err(e) => { - eprintln!("GC failed: {}", e); - } - }, - Err(e) => { - eprintln!("failed to open disk cache: {}", e); - } - } + let dc = fbuild_packages::DiskCache::open().map_err(|e| { + fbuild_core::FbuildError::Other(format!("failed to open disk cache: {}", e)) + })?; + let report = dc + .run_gc() + .map_err(|e| fbuild_core::FbuildError::Other(format!("GC failed: {}", e)))?; + print_gc_report(&report); return Ok(()); } let result = client.run_gc().await?; if !result.success { - eprintln!( + return Err(fbuild_core::FbuildError::Other(format!( "GC failed: {}", result.message.as_deref().unwrap_or("unknown error") - ); - return Ok(()); + ))); } println!("GC complete:"); println!( diff --git a/crates/fbuild-daemon/src/context.rs b/crates/fbuild-daemon/src/context.rs index ae9bf100..662f0c0d 100644 --- a/crates/fbuild-daemon/src/context.rs +++ b/crates/fbuild-daemon/src/context.rs @@ -118,6 +118,8 @@ pub struct DaemonContext { pub broadcast_hub: BroadcastHub, /// Active AVR8js sessions keyed by session ID. pub avr8js_sessions: DashMap, + /// Serializes GC runs so background and manual `/api/cache/gc` don't interleave. + pub gc_mutex: Arc>, } impl DaemonContext { @@ -155,6 +157,7 @@ impl DaemonContext { spawner_cwd, broadcast_hub: BroadcastHub::new(), avr8js_sessions: DashMap::new(), + gc_mutex: Arc::new(tokio::sync::Mutex::new(())), } } diff --git a/crates/fbuild-daemon/src/handlers/cache.rs b/crates/fbuild-daemon/src/handlers/cache.rs index 9ead7b2a..c39aa6dc 100644 --- a/crates/fbuild-daemon/src/handlers/cache.rs +++ b/crates/fbuild-daemon/src/handlers/cache.rs @@ -36,7 +36,9 @@ pub async fn cache_stats(State(_ctx): State>) -> Json>) -> Json { +pub async fn run_gc(State(ctx): State>) -> Json { + // Serialize with background GC loop to prevent interleaved deletes. + let _guard = ctx.gc_mutex.lock().await; match fbuild_packages::DiskCache::open() { Ok(dc) => match dc.run_gc() { Ok(report) => Json(GcResponse { @@ -46,6 +48,8 @@ pub async fn run_gc(State(_ctx): State>) -> Json archives_evicted: report.archives_evicted, archive_bytes_freed: report.archive_bytes_freed, total_bytes_freed: report.total_bytes_freed(), + orphan_files_removed: report.orphan_files_removed, + orphan_rows_cleaned: report.orphan_rows_cleaned, message: None, }), Err(e) => Json(GcResponse { diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index 6e482da0..e758de6b 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -219,12 +219,15 @@ async fn main() { // Spawn background GC loop — runs every 5 minutes { + let gc_mutex = context.gc_mutex.clone(); tokio::spawn(async move { // Wait 60s after startup before first GC to avoid slowing boot tokio::time::sleep(std::time::Duration::from_secs(60)).await; let gc_interval = std::time::Duration::from_secs(300); loop { + // Serialize with manual /api/cache/gc endpoint. + let _guard = gc_mutex.lock().await; match fbuild_packages::DiskCache::open() { Ok(dc) => match dc.run_gc() { Ok(report) => { diff --git a/crates/fbuild-daemon/src/models.rs b/crates/fbuild-daemon/src/models.rs index ff8336b8..44aaa71f 100644 --- a/crates/fbuild-daemon/src/models.rs +++ b/crates/fbuild-daemon/src/models.rs @@ -418,6 +418,8 @@ pub struct GcResponse { pub archives_evicted: u64, pub archive_bytes_freed: u64, pub total_bytes_freed: u64, + pub orphan_files_removed: usize, + pub orphan_rows_cleaned: usize, #[serde(skip_serializing_if = "Option::is_none")] pub message: Option, } diff --git a/crates/fbuild-packages/src/cache.rs b/crates/fbuild-packages/src/cache.rs index 34546414..8c2bf8b9 100644 --- a/crates/fbuild-packages/src/cache.rs +++ b/crates/fbuild-packages/src/cache.rs @@ -146,8 +146,21 @@ impl Cache { /// /// Returns `{cache_root}/toolchains/{toolchain_name}` — used by /// `esp32_metadata::resolve_toolchain_url_sync` to store `tools.json`. + /// + /// `toolchain_name` is validated to be a single normal path component + /// (no `/`, `\`, `..`, or absolute paths) to prevent directory traversal. pub fn toolchain_metadata_dir(&self, toolchain_name: &str) -> PathBuf { - self.toolchains_dir().join(toolchain_name) + use std::path::Component; + let path = Path::new(toolchain_name); + let mut comps = path.components(); + let is_safe = matches!(comps.next(), Some(Component::Normal(_))) && comps.next().is_none(); + if is_safe { + self.toolchains_dir().join(toolchain_name) + } else { + // Unsafe name — hash it to a safe single component + let safe_name = format!("_unsafe_{}", hash_url(toolchain_name)); + self.toolchains_dir().join(safe_name) + } } /// Clean build directory for an environment and profile. diff --git a/crates/fbuild-packages/src/disk_cache/lease.rs b/crates/fbuild-packages/src/disk_cache/lease.rs index 2caae6d9..92a0f021 100644 --- a/crates/fbuild-packages/src/disk_cache/lease.rs +++ b/crates/fbuild-packages/src/disk_cache/lease.rs @@ -5,28 +5,21 @@ //! correctly maintain independent refcounts via a per-process nonce. use super::index::CacheIndex; -use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::Arc; +use std::sync::{Arc, OnceLock}; -/// Per-process nonce, initialized once at startup to a value derived from +/// Per-process nonce, initialized exactly once to a value derived from /// the process start time. Guards against PID reuse: a recycled PID will /// have a different nonce and won't collide with stale lease rows. +/// Uses `OnceLock` to guarantee single-assignment even under concurrent access. fn process_nonce() -> u64 { - static NONCE: AtomicU64 = AtomicU64::new(0); - let val = NONCE.load(Ordering::Relaxed); - if val != 0 { - return val; - } - // Use current time as a nonce — unique enough to distinguish PID reuse. - let nonce = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_nanos() as u64; - // Race is benign: worst case two threads compute different nonces, - // one wins, and both use the winner's value on subsequent calls. - let nonce = nonce.max(1); // ensure non-zero - NONCE.store(nonce, Ordering::Relaxed); - nonce + static NONCE: OnceLock = OnceLock::new(); + *NONCE.get_or_init(|| { + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() as u64; + nonce.max(1) // ensure non-zero + }) } /// RAII guard that pins a cache entry. Released on drop. diff --git a/crates/fbuild-packages/src/disk_cache/mod.rs b/crates/fbuild-packages/src/disk_cache/mod.rs index ca365a88..b72ab53c 100644 --- a/crates/fbuild-packages/src/disk_cache/mod.rs +++ b/crates/fbuild-packages/src/disk_cache/mod.rs @@ -74,6 +74,7 @@ impl DiskCache { } /// Record that an archive was downloaded. + /// Negative `archive_bytes` are clamped to 0. pub fn record_archive( &self, kind: Kind, @@ -88,12 +89,13 @@ impl DiskCache { url, version, archive_path, - archive_bytes, + archive_bytes.max(0), archive_sha256, ) } /// Record that an entry was installed (extracted from archive). + /// Negative `installed_bytes` are clamped to 0. pub fn record_install( &self, kind: Kind, @@ -103,7 +105,7 @@ impl DiskCache { installed_bytes: i64, ) -> rusqlite::Result { self.index - .record_install(kind, url, version, installed_path, installed_bytes) + .record_install(kind, url, version, installed_path, installed_bytes.max(0)) } /// Acquire a lease for the given entry, preventing GC eviction. @@ -135,8 +137,8 @@ impl DiskCache { pub fn stats(&self) -> rusqlite::Result { let budget = CacheBudget::compute(&self.cache_root); Ok(CacheStats { - archive_bytes: self.index.total_archive_bytes()? as u64, - installed_bytes: self.index.total_installed_bytes()? as u64, + archive_bytes: self.index.total_archive_bytes()?.max(0) as u64, + installed_bytes: self.index.total_installed_bytes()?.max(0) as u64, entry_count: self.index.entry_count()?, budget, }) diff --git a/crates/fbuild-packages/src/disk_cache/paths.rs b/crates/fbuild-packages/src/disk_cache/paths.rs index 21a42ab6..e5c6f537 100644 --- a/crates/fbuild-packages/src/disk_cache/paths.rs +++ b/crates/fbuild-packages/src/disk_cache/paths.rs @@ -67,15 +67,40 @@ pub fn stem_and_hash(url: &str) -> (String, String) { /// Sanitize a path component to prevent directory traversal. /// Strips path separators, `.` and `..` sequences, and null bytes. /// Returns `"_"` if the result would be empty or `"."`. +/// +/// When the sanitized form differs from the input (lossy transformation) +/// or would collide with staging `.partial` directories, a short hash of +/// the original string is appended to keep the mapping collision-free. fn sanitize_component(s: &str) -> String { let sanitized = s.replace(['/', '\\', '\0'], "_").replace("..", "_"); - if sanitized.is_empty() || sanitized == "." { + let sanitized = if sanitized.is_empty() || sanitized == "." { "_".to_string() } else { sanitized + }; + // Append a short hash when the mapping is lossy or the name would collide + // with the `.partial` staging convention. + if sanitized != s || sanitized.ends_with(".partial") { + let hash = short_hash(s); + format!("{}_{}", sanitized, hash) + } else { + sanitized } } +/// 8-char hex hash of a string, for disambiguation suffixes. +fn short_hash(s: &str) -> String { + use sha2::{Digest, Sha256}; + let mut hasher = Sha256::new(); + hasher.update(s.as_bytes()); + let result = hasher.finalize(); + // First 4 bytes → 8 hex chars + result[..4] + .iter() + .map(|b| format!("{:02x}", b)) + .collect::() +} + /// Root of the archives phase: `{cache_root}/archives/` pub fn archives_root(cache_root: &Path) -> PathBuf { cache_root.join("archives") @@ -267,8 +292,31 @@ mod tests { dir_str ); - // Normal versions are unchanged + // Normal versions are unchanged (no hash suffix) let dir = archive_entry_dir(root, Kind::Packages, url, "1.2.3"); - assert!(dir.to_string_lossy().contains("1.2.3")); + assert!(dir.to_string_lossy().ends_with("1.2.3")); + } + + #[test] + fn test_sanitize_collision_free() { + // Different inputs that would collide without hash suffix + let a = sanitize_component("1/2"); + let b = sanitize_component("1_2"); + assert_ne!(a, b, "lossy sanitization must be disambiguated"); + + // Normal version: no suffix + let c = sanitize_component("1.2.3"); + assert_eq!(c, "1.2.3"); + } + + #[test] + fn test_sanitize_partial_collision() { + // "1.partial" must not collide with staging dir for version "1" + let a = sanitize_component("1.partial"); + assert!( + !a.ends_with(".partial"), + "must not end with .partial: {}", + a + ); } } From ef10516517e0971e5aaf6d08951abba98c831c4a Mon Sep 17 00:00:00 2001 From: zackees Date: Fri, 10 Apr 2026 23:50:29 -0700 Subject: [PATCH 09/12] fix(cache): fix GC mutex held during sleep and reap_dead_leases COUNT bug - Scope gc_mutex guard in background GC loop so it's dropped before the 5-minute sleep, preventing /api/cache/gc from blocking for minutes - Fix reap_dead_leases to use SUM(refcount) instead of COUNT(*) when refreshing pinned counts, matching pin()/unpin() behavior - Use .max(0) before as u64 casts in gc.rs for defensive consistency with stats() in mod.rs --- crates/fbuild-daemon/src/main.rs | 39 ++++++++++--------- crates/fbuild-packages/src/disk_cache/gc.rs | 6 +-- .../fbuild-packages/src/disk_cache/index.rs | 5 ++- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index e758de6b..85fdabd3 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -226,27 +226,30 @@ async fn main() { let gc_interval = std::time::Duration::from_secs(300); loop { - // Serialize with manual /api/cache/gc endpoint. - let _guard = gc_mutex.lock().await; - match fbuild_packages::DiskCache::open() { - Ok(dc) => match dc.run_gc() { - Ok(report) => { - if report.total_bytes_freed() > 0 { - tracing::info!( - "background GC: freed {} installed ({} entries) + {} archives ({} entries)", - format_bytes_compact(report.installed_bytes_freed), - report.installed_evicted, - format_bytes_compact(report.archive_bytes_freed), - report.archives_evicted, - ); + { + // Serialize with manual /api/cache/gc endpoint. + // Scope the guard so it's dropped before the sleep. + let _guard = gc_mutex.lock().await; + match fbuild_packages::DiskCache::open() { + Ok(dc) => match dc.run_gc() { + Ok(report) => { + if report.total_bytes_freed() > 0 { + tracing::info!( + "background GC: freed {} installed ({} entries) + {} archives ({} entries)", + format_bytes_compact(report.installed_bytes_freed), + report.installed_evicted, + format_bytes_compact(report.archive_bytes_freed), + report.archives_evicted, + ); + } } - } + Err(e) => { + tracing::warn!("background GC failed: {}", e); + } + }, Err(e) => { - tracing::warn!("background GC failed: {}", e); + tracing::debug!("background GC: could not open cache: {}", e); } - }, - Err(e) => { - tracing::debug!("background GC: could not open cache: {}", e); } } tokio::time::sleep(gc_interval).await; diff --git a/crates/fbuild-packages/src/disk_cache/gc.rs b/crates/fbuild-packages/src/disk_cache/gc.rs index d4dd2aae..b2107878 100644 --- a/crates/fbuild-packages/src/disk_cache/gc.rs +++ b/crates/fbuild-packages/src/disk_cache/gc.rs @@ -54,8 +54,8 @@ pub fn run_gc(index: &CacheIndex, budget: &CacheBudget) -> rusqlite::Result budget.installed_budget || total_bytes > budget.high_watermark { let target = budget.low_watermark.min(budget.installed_budget); @@ -88,7 +88,7 @@ pub fn run_gc(index: &CacheIndex, budget: &CacheBudget) -> rusqlite::Result budget.archive_budget || total_bytes > budget.high_watermark { diff --git a/crates/fbuild-packages/src/disk_cache/index.rs b/crates/fbuild-packages/src/disk_cache/index.rs index be8f3f76..4080c89b 100644 --- a/crates/fbuild-packages/src/disk_cache/index.rs +++ b/crates/fbuild-packages/src/disk_cache/index.rs @@ -441,10 +441,11 @@ impl CacheIndex { } } - // Refresh pinned counts for all affected entries + // Refresh pinned counts for all affected entries — use SUM(refcount) + // to match pin()/unpin() which also use SUM(refcount), not COUNT(*). if reaped > 0 { conn.execute_batch( - "UPDATE entries SET pinned = (SELECT COUNT(*) FROM leases WHERE leases.entry_id = entries.id)", + "UPDATE entries SET pinned = (SELECT COALESCE(SUM(refcount), 0) FROM leases WHERE leases.entry_id = entries.id)", )?; } From ad6f3f57259ed661a2aab78e74069e9db56ee86d Mon Sep 17 00:00:00 2001 From: zackees Date: Sat, 11 Apr 2026 00:00:25 -0700 Subject: [PATCH 10/12] fix(cache): fix df block size, zombie rows, Windows PID check, and API gaps - Fix `df -P` to use `-Pk` for correct 1024-byte blocks on Linux (previously used 512-byte multiplier, halving budget on Linux) - Delete zombie rows in GC Step 1 when archive_path is already None after clearing installed_path - Replace Windows `tasklist` PID check with `OpenProcess` FFI (fixes false-positive substring matching and is ~100x faster) - Add `installed_budget` field to CacheStatsResponse API - Reject NaN/Inf/negative values in parse_human_size - Remove dead code `gc_lock_path` (GC uses daemon mutex) --- crates/fbuild-cli/src/daemon_client.rs | 3 +++ crates/fbuild-daemon/src/handlers/cache.rs | 1 + crates/fbuild-daemon/src/models.rs | 1 + .../fbuild-packages/src/disk_cache/budget.rs | 23 ++++++++++++++----- crates/fbuild-packages/src/disk_cache/gc.rs | 5 ++++ .../fbuild-packages/src/disk_cache/index.rs | 23 +++++++++++-------- .../fbuild-packages/src/disk_cache/paths.rs | 5 ---- 7 files changed, 41 insertions(+), 20 deletions(-) diff --git a/crates/fbuild-cli/src/daemon_client.rs b/crates/fbuild-cli/src/daemon_client.rs index cbd19811..0b3ac649 100644 --- a/crates/fbuild-cli/src/daemon_client.rs +++ b/crates/fbuild-cli/src/daemon_client.rs @@ -283,6 +283,9 @@ pub struct CacheStatsResponse { pub high_watermark: u64, pub low_watermark: u64, pub archive_budget: u64, + #[serde(default)] + #[allow(dead_code)] + pub installed_budget: u64, pub message: Option, } diff --git a/crates/fbuild-daemon/src/handlers/cache.rs b/crates/fbuild-daemon/src/handlers/cache.rs index c39aa6dc..2e914de3 100644 --- a/crates/fbuild-daemon/src/handlers/cache.rs +++ b/crates/fbuild-daemon/src/handlers/cache.rs @@ -19,6 +19,7 @@ pub async fn cache_stats(State(_ctx): State>) -> Json Json(CacheStatsResponse { diff --git a/crates/fbuild-daemon/src/models.rs b/crates/fbuild-daemon/src/models.rs index 44aaa71f..4759e61a 100644 --- a/crates/fbuild-daemon/src/models.rs +++ b/crates/fbuild-daemon/src/models.rs @@ -405,6 +405,7 @@ pub struct CacheStatsResponse { pub high_watermark: u64, pub low_watermark: u64, pub archive_budget: u64, + pub installed_budget: u64, #[serde(skip_serializing_if = "Option::is_none")] pub message: Option, } diff --git a/crates/fbuild-packages/src/disk_cache/budget.rs b/crates/fbuild-packages/src/disk_cache/budget.rs index c9a1dfcc..2d9d9651 100644 --- a/crates/fbuild-packages/src/disk_cache/budget.rs +++ b/crates/fbuild-packages/src/disk_cache/budget.rs @@ -83,6 +83,9 @@ pub fn parse_human_size(s: &str) -> Option { }; let num: f64 = num_part.parse().ok()?; + if num.is_nan() || num.is_infinite() || num < 0.0 { + return None; + } let multiplier: u64 = match suffix.as_str() { "" | "B" => 1, @@ -149,9 +152,9 @@ fn get_total_disk_space_windows(path: &Path) -> u64 { #[cfg(unix)] fn get_total_disk_space_unix(path: &Path) -> u64 { use std::process::Command; - // Use POSIX-compatible `df -P` which works on both GNU and BSD/macOS. - // Output is in 512-byte blocks; multiply by 512 to get bytes. - let output = Command::new("df").arg("-P").arg(path).output(); + // Use `df -Pk` for portable 1024-byte block output. + // `-P` gives POSIX format, `-k` forces 1024-byte blocks on all platforms. + let output = Command::new("df").args(["-P", "-k"]).arg(path).output(); output .ok() @@ -161,11 +164,11 @@ fn get_total_disk_space_unix(path: &Path) -> u64 { .lines() .nth(1) // skip header .and_then(|line| { - // POSIX df -P columns: Filesystem 512-blocks Used Available Capacity Mounted + // POSIX df -Pk columns: Filesystem 1024-blocks Used Available Capacity Mounted line.split_whitespace() - .nth(1) // total 512-byte blocks + .nth(1) // total 1024-byte blocks .and_then(|s| s.parse::().ok()) - .map(|blocks| blocks * 512) + .map(|blocks| blocks * 1024) }) }) .unwrap_or(500 * 1024 * 1024 * 1024) // fallback 500 GB @@ -217,6 +220,14 @@ mod tests { fn test_parse_human_size_invalid() { assert_eq!(parse_human_size("abc"), None); assert_eq!(parse_human_size("10X"), None); + assert_eq!(parse_human_size("-5G"), None); + } + + #[test] + fn test_parse_human_size_nan_inf() { + assert_eq!(parse_human_size("NaN"), None); + assert_eq!(parse_human_size("inf"), None); + assert_eq!(parse_human_size("InfG"), None); } #[test] diff --git a/crates/fbuild-packages/src/disk_cache/gc.rs b/crates/fbuild-packages/src/disk_cache/gc.rs index b2107878..6edafddc 100644 --- a/crates/fbuild-packages/src/disk_cache/gc.rs +++ b/crates/fbuild-packages/src/disk_cache/gc.rs @@ -84,6 +84,11 @@ pub fn run_gc(index: &CacheIndex, budget: &CacheBudget) -> rusqlite::Result bool { } #[cfg(windows)] { - use std::process::Command; - Command::new("tasklist") - .args(["/FI", &format!("PID eq {}", pid), "/NH"]) - .output() - .map(|o| { - let stdout = String::from_utf8_lossy(&o.stdout); - stdout.contains(&pid.to_string()) - }) - .unwrap_or(false) + // Use OpenProcess to check if PID is alive (fast, no subprocess). + // PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 + extern "system" { + fn OpenProcess(access: u32, inherit: i32, pid: u32) -> *mut std::ffi::c_void; + fn CloseHandle(handle: *mut std::ffi::c_void) -> i32; + } + const PROCESS_QUERY_LIMITED_INFORMATION: u32 = 0x1000; + let handle = unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid) }; + if handle.is_null() { + false + } else { + unsafe { CloseHandle(handle) }; + true + } } #[cfg(not(any(unix, windows)))] { diff --git a/crates/fbuild-packages/src/disk_cache/paths.rs b/crates/fbuild-packages/src/disk_cache/paths.rs index e5c6f537..8cec3b3c 100644 --- a/crates/fbuild-packages/src/disk_cache/paths.rs +++ b/crates/fbuild-packages/src/disk_cache/paths.rs @@ -116,11 +116,6 @@ pub fn index_path(cache_root: &Path) -> PathBuf { cache_root.join("index.sqlite") } -/// Path to the GC advisory lock: `{cache_root}/gc.lock` -pub fn gc_lock_path(cache_root: &Path) -> PathBuf { - cache_root.join("gc.lock") -} - /// Archive entry path: `{cache_root}/archives/{kind}/{stem}/{hash}/{version}/` pub fn archive_entry_dir(cache_root: &Path, kind: Kind, url: &str, version: &str) -> PathBuf { let (stem, hash) = stem_and_hash(url); From d0b7384f158de5a7ff651732d5e5a7b8db18d65e Mon Sep 17 00:00:00 2001 From: zackees Date: Sat, 11 Apr 2026 00:05:05 -0700 Subject: [PATCH 11/12] fix(cache): spawn_blocking for async handlers, daemon-first purge GC, Windows filename sanitization - Wrap DiskCache::open/stats/run_gc in tokio::task::spawn_blocking to avoid blocking Tokio worker threads in async HTTP handlers - Route `fbuild purge --gc` through daemon /api/cache/gc when a daemon is running, respecting gc_mutex; fall back locally only when no daemon - Sanitize Windows-illegal filename characters (: * ? " < > |) and trailing dots/spaces in cache path components --- crates/fbuild-cli/src/main.rs | 33 ++++++- crates/fbuild-daemon/src/handlers/cache.rs | 93 +++++++++++-------- .../fbuild-packages/src/disk_cache/paths.rs | 55 ++++++++++- 3 files changed, 137 insertions(+), 44 deletions(-) diff --git a/crates/fbuild-cli/src/main.rs b/crates/fbuild-cli/src/main.rs index de98dad8..7ffb3e7f 100644 --- a/crates/fbuild-cli/src/main.rs +++ b/crates/fbuild-cli/src/main.rs @@ -595,7 +595,7 @@ async fn main() { gc, }) => { if gc { - run_purge_gc() + run_purge_gc().await } else { run_purge(target, dry_run, project_dir) } @@ -2031,7 +2031,36 @@ async fn run_clang_tool( } } -fn run_purge_gc() -> fbuild_core::Result<()> { +async fn run_purge_gc() -> fbuild_core::Result<()> { + // Try to route GC through the daemon to respect its gc_mutex. + let client = DaemonClient::new(); + if client.health().await { + let result = client.run_gc().await?; + if !result.success { + return Err(fbuild_core::FbuildError::Other(format!( + "GC failed: {}", + result.message.as_deref().unwrap_or("unknown error") + ))); + } + println!("GC complete (via daemon):"); + println!( + " Installed evicted: {} ({})", + result.installed_evicted, + format_size(result.installed_bytes_freed) + ); + println!( + " Archives evicted: {} ({})", + result.archives_evicted, + format_size(result.archive_bytes_freed) + ); + println!( + " Total freed: {}", + format_size(result.total_bytes_freed) + ); + return Ok(()); + } + + // No daemon running — safe to run GC locally. match fbuild_packages::DiskCache::open() { Ok(dc) => match dc.run_gc() { Ok(report) => { diff --git a/crates/fbuild-daemon/src/handlers/cache.rs b/crates/fbuild-daemon/src/handlers/cache.rs index 2e914de3..82289db9 100644 --- a/crates/fbuild-daemon/src/handlers/cache.rs +++ b/crates/fbuild-daemon/src/handlers/cache.rs @@ -8,29 +8,35 @@ use std::sync::Arc; /// GET /api/cache/stats pub async fn cache_stats(State(_ctx): State>) -> Json { - match fbuild_packages::DiskCache::open() { - Ok(dc) => match dc.stats() { - Ok(stats) => Json(CacheStatsResponse { - success: true, - archive_bytes: stats.archive_bytes, - installed_bytes: stats.installed_bytes, - total_bytes: stats.total_bytes(), - entry_count: stats.entry_count, - high_watermark: stats.budget.high_watermark, - low_watermark: stats.budget.low_watermark, - archive_budget: stats.budget.archive_budget, - installed_budget: stats.budget.installed_budget, - message: None, - }), - Err(e) => Json(CacheStatsResponse { - success: false, - message: Some(format!("failed to read cache stats: {}", e)), - ..Default::default() - }), - }, + let result = tokio::task::spawn_blocking(|| { + let dc = fbuild_packages::DiskCache::open() + .map_err(|e| format!("failed to open cache: {}", e))?; + dc.stats() + .map_err(|e| format!("failed to read cache stats: {}", e)) + }) + .await; + + match result { + Ok(Ok(stats)) => Json(CacheStatsResponse { + success: true, + archive_bytes: stats.archive_bytes, + installed_bytes: stats.installed_bytes, + total_bytes: stats.total_bytes(), + entry_count: stats.entry_count, + high_watermark: stats.budget.high_watermark, + low_watermark: stats.budget.low_watermark, + archive_budget: stats.budget.archive_budget, + installed_budget: stats.budget.installed_budget, + message: None, + }), + Ok(Err(msg)) => Json(CacheStatsResponse { + success: false, + message: Some(msg), + ..Default::default() + }), Err(e) => Json(CacheStatsResponse { success: false, - message: Some(format!("failed to open cache: {}", e)), + message: Some(format!("blocking task failed: {}", e)), ..Default::default() }), } @@ -40,28 +46,33 @@ pub async fn cache_stats(State(_ctx): State>) -> Json>) -> Json { // Serialize with background GC loop to prevent interleaved deletes. let _guard = ctx.gc_mutex.lock().await; - match fbuild_packages::DiskCache::open() { - Ok(dc) => match dc.run_gc() { - Ok(report) => Json(GcResponse { - success: true, - installed_evicted: report.installed_evicted, - installed_bytes_freed: report.installed_bytes_freed, - archives_evicted: report.archives_evicted, - archive_bytes_freed: report.archive_bytes_freed, - total_bytes_freed: report.total_bytes_freed(), - orphan_files_removed: report.orphan_files_removed, - orphan_rows_cleaned: report.orphan_rows_cleaned, - message: None, - }), - Err(e) => Json(GcResponse { - success: false, - message: Some(format!("GC failed: {}", e)), - ..Default::default() - }), - }, + let result = tokio::task::spawn_blocking(|| { + let dc = fbuild_packages::DiskCache::open() + .map_err(|e| format!("failed to open cache: {}", e))?; + dc.run_gc().map_err(|e| format!("GC failed: {}", e)) + }) + .await; + + match result { + Ok(Ok(report)) => Json(GcResponse { + success: true, + installed_evicted: report.installed_evicted, + installed_bytes_freed: report.installed_bytes_freed, + archives_evicted: report.archives_evicted, + archive_bytes_freed: report.archive_bytes_freed, + total_bytes_freed: report.total_bytes_freed(), + orphan_files_removed: report.orphan_files_removed, + orphan_rows_cleaned: report.orphan_rows_cleaned, + message: None, + }), + Ok(Err(msg)) => Json(GcResponse { + success: false, + message: Some(msg), + ..Default::default() + }), Err(e) => Json(GcResponse { success: false, - message: Some(format!("failed to open cache: {}", e)), + message: Some(format!("blocking task failed: {}", e)), ..Default::default() }), } diff --git a/crates/fbuild-packages/src/disk_cache/paths.rs b/crates/fbuild-packages/src/disk_cache/paths.rs index 8cec3b3c..f57e7e61 100644 --- a/crates/fbuild-packages/src/disk_cache/paths.rs +++ b/crates/fbuild-packages/src/disk_cache/paths.rs @@ -72,7 +72,16 @@ pub fn stem_and_hash(url: &str) -> (String, String) { /// or would collide with staging `.partial` directories, a short hash of /// the original string is appended to keep the mapping collision-free. fn sanitize_component(s: &str) -> String { - let sanitized = s.replace(['/', '\\', '\0'], "_").replace("..", "_"); + let sanitized: String = s + .chars() + .map(|c| match c { + '/' | '\\' | '\0' | ':' | '*' | '?' | '"' | '<' | '>' | '|' => '_', + _ => c, + }) + .collect(); + let sanitized = sanitized.replace("..", "_"); + // Trim trailing spaces and dots (invalid on Windows) + let sanitized = sanitized.trim_end_matches([' ', '.']).to_string(); let sanitized = if sanitized.is_empty() || sanitized == "." { "_".to_string() } else { @@ -314,4 +323,48 @@ mod tests { a ); } + + #[test] + fn test_sanitize_windows_illegal_chars() { + // Windows-illegal characters must be replaced + let a = sanitize_component("1:2"); + assert!(!a.contains(':'), "colon not sanitized: {}", a); + + let b = sanitize_component("v1*beta"); + assert!(!b.contains('*'), "asterisk not sanitized: {}", b); + + let c = sanitize_component("test?ver"); + assert!(!c.contains('?'), "question mark not sanitized: {}", c); + + let d = sanitize_component("ac"); + assert!( + !d.contains('<') && !d.contains('>'), + "angle brackets not sanitized: {}", + d + ); + + let e = sanitize_component("a|b"); + assert!(!e.contains('|'), "pipe not sanitized: {}", e); + + let f = sanitize_component(r#"a"b"#); + assert!(!f.contains('"'), "quote not sanitized: {}", f); + + // Trailing dots and spaces stripped (Windows restriction) + let g = sanitize_component("v1."); + assert!(!g.ends_with('.'), "trailing dot not stripped: {}", g); + + let h = sanitize_component("v1 "); + assert!(!h.ends_with(' '), "trailing space not stripped: {}", h); + } + + #[test] + fn test_sanitize_windows_collision_free() { + // "1:2" and "1_2" must not collide + let a = sanitize_component("1:2"); + let b = sanitize_component("1_2"); + assert_ne!( + a, b, + "Windows-illegal char sanitization must be disambiguated" + ); + } } From f08fac49738a3972db7ec978d79bf1874506ef27 Mon Sep 17 00:00:00 2001 From: zackees Date: Sat, 11 Apr 2026 02:02:00 -0700 Subject: [PATCH 12/12] fix(cli): expose orphan counters in daemon GC response The CLI's GcResponse struct was missing orphan_files_removed and orphan_rows_cleaned fields from the daemon's response. This caused `fbuild daemon gc` and `fbuild purge --gc` (via daemon) to silently drop reconciliation stats. Add the fields and print them. --- crates/fbuild-cli/src/daemon_client.rs | 4 ++++ crates/fbuild-cli/src/main.rs | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/crates/fbuild-cli/src/daemon_client.rs b/crates/fbuild-cli/src/daemon_client.rs index 0b3ac649..fb89cb90 100644 --- a/crates/fbuild-cli/src/daemon_client.rs +++ b/crates/fbuild-cli/src/daemon_client.rs @@ -297,6 +297,10 @@ pub struct GcResponse { pub archives_evicted: u64, pub archive_bytes_freed: u64, pub total_bytes_freed: u64, + #[serde(default)] + pub orphan_files_removed: usize, + #[serde(default)] + pub orphan_rows_cleaned: usize, pub message: Option, } diff --git a/crates/fbuild-cli/src/main.rs b/crates/fbuild-cli/src/main.rs index 7ffb3e7f..8694838f 100644 --- a/crates/fbuild-cli/src/main.rs +++ b/crates/fbuild-cli/src/main.rs @@ -2057,6 +2057,12 @@ async fn run_purge_gc() -> fbuild_core::Result<()> { " Total freed: {}", format_size(result.total_bytes_freed) ); + if result.orphan_files_removed > 0 { + println!(" Orphan files removed: {}", result.orphan_files_removed); + } + if result.orphan_rows_cleaned > 0 { + println!(" Orphan rows cleaned: {}", result.orphan_rows_cleaned); + } return Ok(()); } @@ -2578,6 +2584,12 @@ async fn run_daemon_gc(client: &DaemonClient) -> fbuild_core::Result<()> { " Total freed: {}", format_size(result.total_bytes_freed) ); + if result.orphan_files_removed > 0 { + println!(" Orphan files removed: {}", result.orphan_files_removed); + } + if result.orphan_rows_cleaned > 0 { + println!(" Orphan rows cleaned: {}", result.orphan_rows_cleaned); + } Ok(()) }