diff --git a/src/context/local.rs b/src/context/local.rs index 3ed5c56..4688c48 100644 --- a/src/context/local.rs +++ b/src/context/local.rs @@ -24,33 +24,31 @@ impl ContextDriver for LocalDriver { } fn create_temp_dir(&self) -> io::Result { - // Generate a unique temporary directory name with random string + // Sub-second precision and an atomic create: two concurrent + // contexts racing on the same name must never share a directory, + // so the loser of a create falls through to the next attempt + // instead of probing for existence first (a probe-then-create + // window loses exactly when two callers arrive together). let base_timestamp = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) .unwrap() - .as_secs(); + .as_millis(); let mut attempt = 0; loop { let work_dir_name = if attempt == 0 { - format!("pkh-{}", base_timestamp) + format!("pkh-{base_timestamp}") } else { - format!("pkh-{}-{}", base_timestamp, attempt) + format!("pkh-{base_timestamp}-{attempt}") }; let temp_dir_path = std::env::temp_dir().join(&work_dir_name); - // Check if directory already exists - if temp_dir_path.exists() { - attempt += 1; - continue; + match std::fs::create_dir(&temp_dir_path) { + Ok(()) => return Ok(temp_dir_path.to_string_lossy().to_string()), + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => attempt += 1, + Err(e) => return Err(e), } - - // Create the directory - std::fs::create_dir_all(&temp_dir_path)?; - - // Return the path as a string - return Ok(temp_dir_path.to_string_lossy().to_string()); } } @@ -190,3 +188,39 @@ fn copy_dir_recursive(src: &Path, dest: &Path) -> io::Result<()> { } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// Concurrent callers must never share a temporary directory: the + /// create is atomic, so a lost race falls through to the next name + /// instead of both callers probing the same free name and unpacking + /// into the same directory. + #[test] + fn create_temp_dir_is_unique_under_concurrency() { + const CALLERS: usize = 8; + let (tx, rx) = std::sync::mpsc::channel(); + let handles: Vec<_> = (0..CALLERS) + .map(|_| { + let tx = tx.clone(); + std::thread::spawn(move || { + let dir = LocalDriver.create_temp_dir().unwrap(); + tx.send(dir).unwrap(); + }) + }) + .collect(); + for handle in handles { + handle.join().unwrap(); + } + drop(tx); + + let mut names: Vec = rx.iter().collect(); + names.sort(); + let unique: std::collections::BTreeSet<&String> = names.iter().collect(); + assert_eq!(names.len(), unique.len(), "duplicate temp dirs: {names:?}"); + for name in &unique { + std::fs::remove_dir(name).unwrap(); + } + } +} diff --git a/src/context/unshare.rs b/src/context/unshare.rs index a7be3a1..afb8678 100644 --- a/src/context/unshare.rs +++ b/src/context/unshare.rs @@ -296,37 +296,41 @@ impl ContextDriver for UnshareDriver { fn create_temp_dir(&self) -> io::Result { // Create a temporary directory inside the chroot with unique naming + // Sub-second precision and an atomic create, like the local + // driver: concurrent callers racing on the same name must not + // share a directory, so an existing target falls through to the + // next attempt instead of a probe-then-create window. let base_timestamp = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap() - .as_secs(); + .as_millis(); let mut attempt = 0; loop { let work_dir_name = if attempt == 0 { - format!("pkh-build-{}", base_timestamp) + format!("pkh-build-{base_timestamp}") } else { - format!("pkh-build-{}-{}", base_timestamp, attempt) + format!("pkh-build-{base_timestamp}-{attempt}") }; - let work_dir_inside_chroot = format!("/tmp/{}", work_dir_name); + let work_dir_inside_chroot = format!("/tmp/{work_dir_name}"); let host_path = Path::new(&self.path).join("tmp").join(&work_dir_name); - // Check if directory already exists - if host_path.exists() { - attempt += 1; - continue; + match std::fs::create_dir(&host_path) { + Ok(()) => { + debug!( + "Created work directory: {} (host: {})", + work_dir_inside_chroot, + host_path.display() + ); + } + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => { + attempt += 1; + continue; + } + Err(e) => return Err(e), } - // Create the directory on the host filesystem - std::fs::create_dir_all(&host_path)?; - - debug!( - "Created work directory: {} (host: {})", - work_dir_inside_chroot, - host_path.display() - ); - // Return the path as it appears inside the chroot return Ok(work_dir_inside_chroot); }