diff --git a/crates/fbuild-core/src/file_lock.rs b/crates/fbuild-core/src/file_lock.rs index 60800344..8b243d74 100644 --- a/crates/fbuild-core/src/file_lock.rs +++ b/crates/fbuild-core/src/file_lock.rs @@ -20,6 +20,16 @@ pub struct FileLockGuard { _file: File, } +impl Drop for FileLockGuard { + fn drop(&mut self) { + // Unlock explicitly instead of relying on close: macOS can transiently + // report contention for a re-acquire racing the close-release + // (FastLED/fbuild#1340). Explicit LOCK_UN is the deterministic release; + // soldr's lifecycle guard unlocks the same way. + let _ = self._file.unlock(); + } +} + /// Try to acquire an OS-released lock on `path`. /// /// Returns `Ok(None)` when another process holds a conflicting lock. The lock @@ -122,6 +132,26 @@ pub async fn acquire( mod tests { use super::*; + /// Post-release re-acquires poll through a short deadline instead of + /// asserting single-shot availability: macOS can transiently report + /// contention just after close-release (FastLED/fbuild#1340). The + /// production contract is poll-and-retry, so the property under test is + /// eventual availability. + fn reacquire_within(path: &Path, mode: FileLockMode) -> FileLockGuard { + let deadline = Instant::now() + Duration::from_secs(1); + loop { + if let Some(guard) = try_acquire(path, mode).unwrap() { + return guard; + } + assert!( + Instant::now() < deadline, + "lock at {} did not become available within 1s of release", + path.display() + ); + std::thread::sleep(Duration::from_millis(10)); + } + } + #[test] fn shared_holders_block_exclusive_until_all_release() { let temp = tempfile::tempdir().unwrap(); @@ -145,11 +175,7 @@ mod tests { .is_none() ); drop(second); - assert!( - try_acquire(&path, FileLockMode::Exclusive) - .unwrap() - .is_some() - ); + let _third = reacquire_within(&path, FileLockMode::Exclusive); } #[test] @@ -162,7 +188,7 @@ mod tests { assert!(try_acquire(&path, FileLockMode::Shared).unwrap().is_none()); drop(exclusive); - assert!(try_acquire(&path, FileLockMode::Shared).unwrap().is_some()); + let _shared = reacquire_within(&path, FileLockMode::Shared); } #[tokio::test] diff --git a/crates/fbuild-paths/src/daemon_ownership.rs b/crates/fbuild-paths/src/daemon_ownership.rs index 07e0a586..ea015504 100644 --- a/crates/fbuild-paths/src/daemon_ownership.rs +++ b/crates/fbuild-paths/src/daemon_ownership.rs @@ -186,6 +186,28 @@ mod tests { use super::*; use tempfile::TempDir; + /// Post-release re-acquires poll through a short deadline instead of + /// asserting single-shot availability: the kernel can transiently report + /// contention just after close-release on macOS (FastLED/fbuild#1340), and + /// the production contract is poll-and-retry, so the property under test is + /// eventual availability. Hard I/O errors still fail immediately. + fn reacquire_within( + io_context: &str, + mut acquire: impl FnMut() -> std::io::Result>, + ) -> T { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(1); + loop { + match acquire().unwrap_or_else(|error| panic!("{io_context}: {error}")) { + Some(guard) => return guard, + None => assert!( + std::time::Instant::now() < deadline, + "{io_context}: lock did not become available within 1s of release" + ), + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + #[test] fn root_ownership_is_exclusive_within_process() { let temp = TempDir::new().expect("tempdir"); @@ -200,9 +222,9 @@ mod tests { "a second exclusive acquire while the first is held must return None" ); drop(first); - let third = RootOwnershipGuard::try_acquire_at(&path) - .expect("third acquire io") - .expect("lock must be available again after the holder drops"); + let third = reacquire_within("third acquire io", || { + RootOwnershipGuard::try_acquire_at(&path) + }); drop(third); } @@ -221,9 +243,9 @@ mod tests { let second = try_acquire_spawn_lock_result_at(&path).expect("second acquire io"); assert!(second.is_none(), "second acquire while held must be None"); drop(first); - let third = try_acquire_spawn_lock_result_at(&path) - .expect("third acquire io") - .expect("lock must be available after release"); + let third = reacquire_within("third acquire io", || { + try_acquire_spawn_lock_result_at(&path) + }); drop(third); } @@ -249,9 +271,7 @@ mod tests { "second acquire while held must be None" ); drop(first); - try_acquire_spawn_lock_result_at(&path) - .expect("re-acquire io") - .expect("lock must be available after release"); + reacquire_within("re-acquire io", || try_acquire_spawn_lock_result_at(&path)); } /// Soldr pattern (`spawn_lock_serializes_concurrent_threads`): fire a diff --git a/dylints/ban_std_pathbuf/src/allowlist.txt b/dylints/ban_std_pathbuf/src/allowlist.txt index 30f04010..e197b1cf 100644 --- a/dylints/ban_std_pathbuf/src/allowlist.txt +++ b/dylints/ban_std_pathbuf/src/allowlist.txt @@ -168,6 +168,9 @@ crates/fbuild-library/src/library/arduino_mbed_core.rs crates/fbuild-library/src/library/attiny_core.rs crates/fbuild-library/src/library/avr_framework.rs crates/fbuild-library/src/library/ch32v_core.rs +# New-file entry per allowlist policy: clearcore_core.rs (#1333) implements the +# shared Package/core traits typed in PathBuf; migration is repo-wide (FastLED/fbuild#1339). +crates/fbuild-library/src/library/clearcore_core.rs crates/fbuild-library/src/library/cmsis_atmel.rs crates/fbuild-library/src/library/cmsis_framework.rs crates/fbuild-library/src/library/esp32_framework/fs_utils.rs