context: name temp dirs atomically, not probe-then-create

create_temp_dir probed for a free pkh-<seconds> name and then created
the directory, so two contexts arriving together could both observe a
free name and unpack into the same directory — observed as two e2e
tests started within the same second failing on 'File exists when
hard linking' during the chroot tarball unpack.

Name with sub-second precision and create atomically: a losing race
gets AlreadyExists and falls through to the next attempt, which
removes the probe window instead of narrowing it. The schroot and
ssh drivers already use mktemp -d and need no change.

src/context/local.rs

@@ -24,33 +24,31 @@ impl ContextDriver for LocalDriver {
     }
 
     fn create_temp_dir(&self) -> io::Result<String> {
-        // 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<String> = 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();
+        }
+    }
+}

src/context/unshare.rs

@@ -296,37 +296,41 @@ impl ContextDriver for UnshareDriver {
 
     fn create_temp_dir(&self) -> io::Result<String> {
         // 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);
         }