fix: close build coalescing race

docs/how-it-works/architecture.mdx

@@ -26,6 +26,8 @@ check remote (via daemon) ── hit → restore via reflink → done
     ↓ miss
 acquire per-key build lock
     ↓
+re-check local store ── peer committed → restore via reflink → done
+    ↓ still absent
 run rustc
     ↓
 store output files in content-addressed blobs
@@ -35,7 +37,7 @@ send upload job to daemon (async)
 release lock
 ```
 
-The lock prevents duplicate compilation when cargo spawns multiple parallel rustc processes for the same crate (which can happen in certain workspace configurations). A process that loses the lock waits for the winner's result and restores from the cache instead of compiling.
+The lock prevents duplicate compilation when cargo spawns multiple parallel rustc processes for the same crate (which can happen in certain workspace configurations). A process that loses the lock waits for the winner's result and restores from the cache instead of compiling. A process that acquires the lock re-checks the store before compiling, covering a peer that committed between the initial lookup and lock acquisition.
 
 Build outcomes are separated into three cacheable cases:
 

src/store.rs

@@ -616,6 +616,16 @@ pub struct KeyLock {
     path: PathBuf,
 }
 
+/// Result of claiming responsibility for a cache miss.
+pub enum BuildClaim {
+    /// This process owns the key and may compile it.
+    Acquired(KeyLock),
+    /// A peer committed the key after the caller's cache lookup.
+    Committed(Box<EntryMeta>),
+    /// Another process currently owns the key.
+    Contended,
+}
+
 /// A fully-written key lock waiting to be published at its canonical path.
 /// Keeping preparation separate from publication prevents contenders from
 /// observing an empty lock file and mistaking an in-progress owner for stale.
@@ -1316,6 +1326,25 @@ impl Store {
         self.try_acquire_lock(self.entry_dir(cache_key).with_extension("lock"))
     }
 
+    /// Claim a cache miss, re-checking the store after acquiring the key lock.
+    ///
+    /// The re-check closes the window where a peer can commit and release its
+    /// lock between this process's cache lookup and lock acquisition.
+    pub fn claim_build(&self, cache_key: &str) -> Result<BuildClaim> {
+        let Some(lock) = self.try_lock(cache_key)? else {
+            return Ok(BuildClaim::Contended);
+        };
+        match self.get(cache_key)? {
+            Some(meta) if meta.files.is_empty() => {
+                tracing::warn!("cache entry {cache_key} has no files, evicting before build");
+                self.remove_entry(cache_key)?;
+                Ok(BuildClaim::Acquired(lock))
+            }
+            Some(meta) => Ok(BuildClaim::Committed(Box::new(meta))),
+            None => Ok(BuildClaim::Acquired(lock)),
+        }
+    }
+
     /// Acquire the cross-process GC lock so concurrent GC drivers — a manual
     /// `kache gc`, the daemon's periodic sweep, `maybe_evict_after_upload`, or a
     /// second daemon — don't double-scan and contend. Returns `None` if another
@@ -4583,19 +4612,114 @@ mod tests {
         let config = test_config(dir.path());
         let store = Store::open(&config).unwrap();
 
-        let lock1 = store.try_lock("testkey").unwrap();
-        assert!(lock1.is_some());
+        let lock1 = match store.claim_build("testkey").unwrap() {
+            BuildClaim::Acquired(lock) => lock,
+            BuildClaim::Committed(_) | BuildClaim::Contended => {
+                panic!("first build claim should acquire the key")
+            }
+        };
 
-        // Second lock attempt should fail
-        let lock2 = store.try_lock("testkey").unwrap();
-        assert!(lock2.is_none());
+        assert!(matches!(
+            store.claim_build("testkey").unwrap(),
+            BuildClaim::Contended
+        ));
 
-        // Drop first lock
         drop(lock1);
 
-        // Now should succeed
-        let lock3 = store.try_lock("testkey").unwrap();
-        assert!(lock3.is_some());
+        assert!(matches!(
+            store.claim_build("testkey").unwrap(),
+            BuildClaim::Acquired(_)
+        ));
+    }
+
+    #[test]
+    fn claim_build_rechecks_entry_after_acquiring_lock() {
+        let dir = tempfile::tempdir().unwrap();
+        let config = test_config(dir.path());
+        let peer = Store::open(&config).unwrap();
+        let waiter = Store::open(&config).unwrap();
+        let cache_key = "committed_during_claim_race";
+
+        let peer_lock = match peer.claim_build(cache_key).unwrap() {
+            BuildClaim::Acquired(lock) => lock,
+            BuildClaim::Committed(_) | BuildClaim::Contended => {
+                panic!("peer should acquire the initial build claim")
+            }
+        };
+        assert!(matches!(
+            waiter.claim_build(cache_key).unwrap(),
+            BuildClaim::Contended
+        ));
+
+        let output = dir.path().join("lib.rlib");
+        fs::write(&output, b"peer output").unwrap();
+        peer.put(
+            cache_key,
+            "peer",
+            &["rlib".to_string()],
+            &[],
+            "host",
+            "dev",
+            &[(output, "lib.rlib".to_string())],
+            "",
+            "",
+        )
+        .unwrap();
+        drop(peer_lock);
+
+        match waiter.claim_build(cache_key).unwrap() {
+            BuildClaim::Committed(meta) => assert_eq!(meta.cache_key, cache_key),
+            BuildClaim::Acquired(_) => panic!("committed entry must prevent a duplicate compile"),
+            BuildClaim::Contended => panic!("peer already released the build lock"),
+        }
+        assert!(
+            waiter.try_lock(cache_key).unwrap().is_some(),
+            "serving the committed entry must release the claim"
+        );
+    }
+
+    #[test]
+    fn claim_build_evicts_empty_committed_entry() {
+        let dir = tempfile::tempdir().unwrap();
+        let config = test_config(dir.path());
+        let store = Store::open(&config).unwrap();
+        let cache_key = "empty_committed_entry";
+        let entry_dir = store.entry_dir(cache_key);
+        fs::create_dir_all(&entry_dir).unwrap();
+        let meta = EntryMeta {
+            cache_key: cache_key.to_string(),
+            crate_name: "empty".to_string(),
+            crate_types: vec!["rlib".to_string()],
+            files: vec![],
+            stdout: String::new(),
+            stderr: String::new(),
+            features: vec![],
+            target: "host".to_string(),
+            profile: "dev".to_string(),
+            compile_time_ms: 0,
+            emit_kinds: Vec::new(),
+        };
+        fs::write(
+            entry_dir.join("meta.json"),
+            serde_json::to_string_pretty(&meta).unwrap(),
+        )
+        .unwrap();
+        store
+            .db
+            .execute(
+                "INSERT INTO entries (cache_key, crate_name, size, committed) VALUES (?1, ?2, 0, 1)",
+                params![cache_key, "empty"],
+            )
+            .unwrap();
+
+        match store.claim_build(cache_key).unwrap() {
+            BuildClaim::Acquired(_) => {}
+            BuildClaim::Committed(_) => panic!("empty entry must not be served"),
+            BuildClaim::Contended => panic!("no peer owns the build lock"),
+        }
+        assert!(store.get(cache_key).unwrap().is_none());
+        assert!(!entry_dir.exists());
+        assert!(store.try_lock(cache_key).unwrap().is_some());
     }
 
     #[test]

src/wrapper.rs

@@ -14,7 +14,7 @@ use crate::compiler::{
 use crate::config::Config;
 use crate::events::{self, BuildEvent, EventResult};
 use crate::link;
-use crate::store::{Store, StorePutResult};
+use crate::store::{BuildClaim, Store, StorePutResult};
 
 /// Check whether progress lines should be printed to stderr.
 ///
@@ -1321,12 +1321,13 @@ pub fn run(config: &Config, wrapper_args: &[String]) -> Result<i32> {
         }
     }
 
-    // 3. Cache miss — try to acquire build lock
-    let lock = match store.try_lock(&cache_key) {
-        Ok(Some(lock)) => lock,
+    // 3. Cache miss — claim the key, then re-check under the build lock.
+    let (lock, committed) = match store.claim_build(&cache_key) {
+        Ok(BuildClaim::Acquired(lock)) => (Some(lock), None),
+        Ok(BuildClaim::Committed(meta)) => (None, Some(*meta)),
         Err(e) => {
             tracing::warn!(
-                "acquiring build lock for {} failed: {} — recompiling",
+                "claiming build for {} failed: {} — recompiling",
                 crate_name,
                 e
             );
@@ -1336,81 +1337,80 @@ pub fn run(config: &Config, wrapper_args: &[String]) -> Result<i32> {
                 crate_name,
                 &event_root,
                 start,
-                format!("build lock unavailable: {e}"),
+                format!("build claim failed: {e}"),
             );
         }
-        Ok(None) => {
+        Ok(BuildClaim::Contended) => {
             // Another process is building this key — wait for it
             tracing::debug!("waiting for {} to be built by another process", crate_name);
-            if store.wait_for_committed(&cache_key).unwrap_or(false) {
-                // It's now available
-                if let Ok(Some(meta)) = store.get(&cache_key) {
-                    let restore_start = std::time::Instant::now();
-                    if let Err(e) = restore_from_cache(
-                        config,
-                        &compiler,
-                        &BlobSource::Store(&store),
-                        &args,
-                        &meta,
-                    ) {
-                        tracing::warn!(
-                            "restoring cache hit for {} failed: {} — recompiling",
-                            crate_name,
-                            e
-                        );
-                        return passthrough_with_event(
-                            config,
-                            &args,
-                            crate_name,
-                            &event_root,
-                            start,
-                            format!("restore failed: {e}"),
-                        );
-                    }
-                    let restore_ms = restore_start.elapsed().as_millis() as u64;
-                    let elapsed = start.elapsed().as_millis() as u64;
-                    let size: u64 = meta.files.iter().map(|f| f.size).sum();
-                    log_event_with_hash_stats(
-                        config,
-                        &event_root,
-                        crate_name,
-                        EventResult::LocalHit,
-                        elapsed,
-                        meta.compile_time_ms,
-                        size,
-                        &cache_key,
-                        key_ms,
-                        key_hash_stats,
-                        lookup_ms,
-                        restore_ms,
-                        0,
-                    );
-                    // Replay the original compiler diagnostics, exactly as
-                    // the other hit sites do — otherwise the process that
-                    // loses the build-lock race silently swallows the
-                    // cached warnings/notes.
-                    if !meta.stdout.is_empty() {
-                        print!("{}", meta.stdout);
-                    }
-                    if !meta.stderr.is_empty() {
-                        eprint!("{}", meta.stderr);
-                    }
-                    clean_incremental_dir(config, &args);
-                    return Ok(0);
-                }
-            }
-            // If waiting failed, fall through to compile
-            tracing::warn!("wait for {} failed, compiling ourselves", crate_name);
-            // Compile without caching
+            let committed = store
+                .wait_for_committed(&cache_key)
+                .unwrap_or(false)
+                .then(|| store.get(&cache_key).ok().flatten())
+                .flatten();
+            (None, committed)
+        }
+    };
+
+    if let Some(meta) = committed {
+        let restore_start = std::time::Instant::now();
+        if let Err(e) =
+            restore_from_cache(config, &compiler, &BlobSource::Store(&store), &args, &meta)
+        {
+            tracing::warn!(
+                "restoring cache hit for {} failed: {} — recompiling",
+                crate_name,
+                e
+            );
             return passthrough_with_event(
                 config,
                 &args,
                 crate_name,
                 &event_root,
                 start,
-                "build lock wait failed",
+                format!("restore failed: {e}"),
             );
         }
+        let restore_ms = restore_start.elapsed().as_millis() as u64;
+        let elapsed = start.elapsed().as_millis() as u64;
+        let size: u64 = meta.files.iter().map(|f| f.size).sum();
+        log_event_with_hash_stats(
+            config,
+            &event_root,
+            crate_name,
+            EventResult::LocalHit,
+            elapsed,
+            meta.compile_time_ms,
+            size,
+            &cache_key,
+            key_ms,
+            key_hash_stats,
+            lookup_ms,
+            restore_ms,
+            0,
+        );
+        // Replay the original compiler diagnostics, exactly as the other hit
+        // sites do, so a coalesced compile does not swallow warnings or notes.
+        if !meta.stdout.is_empty() {
+            print!("{}", meta.stdout);
+        }
+        if !meta.stderr.is_empty() {
+            eprint!("{}", meta.stderr);
+        }
+        clean_incremental_dir(config, &args);
+        return Ok(0);
+    }
+
+    let Some(lock) = lock else {
+        tracing::warn!("wait for {} failed, compiling ourselves", crate_name);
+        return passthrough_with_event(
+            config,
+            &args,
+            crate_name,
+            &event_root,
+            start,
+            "build lock wait failed",
+        );
     };
 
     // 4. Compile