diff --git a/Cargo.toml b/Cargo.toml index 5a678bb..2f7f1f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ anyhow = "1.0.20" backtrace = "0.3.55" cargo-platform = "0.3" cargo_metadata = "0.23.0" +fslock = "0.2.1" sysinfo = { version = "0.38", default-features = false, features = ["system"] } whoami = "2" diff --git a/src/lib.rs b/src/lib.rs index 06d3f86..f0e08a6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -107,6 +107,8 @@ where let mut testdir = builder.create().expect("Failed to create testdir"); let mut count = 0; while private::create_cargo_pid_file(testdir.path()).is_err() { + // The directory was claimed by another process that was racing us and it was + // part of a separate testrun. Try to create a new one. count += 1; if count > 20 { break; diff --git a/src/private.rs b/src/private.rs index a679af4..f2353fc 100644 --- a/src/private.rs +++ b/src/private.rs @@ -9,9 +9,9 @@ use std::ffi::OsStr; use std::fs::{self, File}; use std::path::{Path, PathBuf}; use std::sync::LazyLock; -use std::time::{Duration, Instant}; -use anyhow::Result; +use anyhow::{Context, Result}; +use fslock::LockFile; use sysinfo::Pid; use crate::{NumberedDir, NumberedDirBuilder}; @@ -19,6 +19,9 @@ use crate::{NumberedDir, NumberedDirBuilder}; /// The filename in which we store the Cargo PID: `cargo-pid`. const CARGO_PID_FILE_NAME: &str = "cargo-pid"; +/// The lockfile for creating the cargo PID file. +const CARGO_PID_LOCKFILE: &str = "cargo-pid.lock"; + /// Whether we are a cargo sub-process. static CARGO_PID: LazyLock> = LazyLock::new(cargo_pid); @@ -82,46 +85,48 @@ fn cargo_target_dir() -> PathBuf { /// [`NumberedDir`]: crate::NumberedDir pub(crate) fn reuse_cargo(dir: &Path) -> bool { let file_name = dir.join(CARGO_PID_FILE_NAME); - let start = Instant::now(); - while start.elapsed() <= Duration::from_millis(500) { - if let Some(read_cargo_pid) = fs::read_to_string(&file_name) - .ok() - .and_then(|content| content.parse::().ok()) - { - return Some(read_cargo_pid) == *CARGO_PID; - } else { - // Wen we encounter a directory that has no pidfile we assume some other process - // just created the directory and is about to write the pdifile. So we wait a - // little in the hope the pidfile appears. - std::thread::sleep(Duration::from_millis(5)); - } + + // Fast-path, just read the pidfile + if let Some(read_cargo_pid) = fs::read_to_string(&file_name) + .ok() + .and_then(|content| content.parse::().ok()) + { + return Some(read_cargo_pid) == *CARGO_PID; } - // Give up, we'll create a new directory ourselves. - false + + // Slow path, try and claim this directory for us. We are probably racing several + // processes creating the next directory. Creating the pidfile uses a lockfile to make + // sure only one process creates the pidfile. + create_cargo_pid_file(dir).is_ok() } /// Creates a file storing the Cargo PID if not yet present. /// +/// Uses a lockfile to make sure only one process is writing the file at once. If the +/// pidfile was being written by another process at the same time and the PID matches it is +/// treated as a successful write. +/// /// # Returns /// /// An error return indicates that the pid file was created by another process that was not /// part of our testrun. So this numbered dir should not be used. -/// -/// # Panics -/// -/// If the PID file could not be created, written or read. pub(crate) fn create_cargo_pid_file(dir: &Path) -> Result<()> { let cargo_pid = CARGO_PID .map(|pid| pid.to_string()) .unwrap_or("failed to get cargo PID".to_string()); + + // Lock the lockfile, unlocks when handle is dropped. + let mut lockfile = LockFile::open(&dir.join(CARGO_PID_LOCKFILE))?; + lockfile.lock()?; + let file_name = dir.join(CARGO_PID_FILE_NAME); match File::create_new(&file_name) { Ok(_) => { - fs::write(&file_name, cargo_pid).expect("Failed to write cargo PID"); + fs::write(&file_name, cargo_pid).context("failed to write cargo-pid")?; Ok(()) } Err(_) => { - let contents = fs::read_to_string(&file_name).expect("Failed to read cargo-pid"); + let contents = fs::read_to_string(&file_name).context("failed to read cargo-pid")?; if cargo_pid == contents { Ok(()) } else {