Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions crates/fbuild-core/src/file_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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]
Expand All @@ -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]
Expand Down
38 changes: 29 additions & 9 deletions crates/fbuild-paths/src/daemon_ownership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(
io_context: &str,
mut acquire: impl FnMut() -> std::io::Result<Option<T>>,
) -> 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");
Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions dylints/ban_std_pathbuf/src/allowlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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
Expand Down
Loading