diff --git a/crates/ecrou/src/download.rs b/crates/ecrou/src/download.rs index 0d8578b..482250a 100644 --- a/crates/ecrou/src/download.rs +++ b/crates/ecrou/src/download.rs @@ -302,12 +302,15 @@ async fn download_file_async( (total_size > 0).then_some(total_size), ); - let temp_dest = dest.with_extension("partial"); + // Unique per call: concurrent runs writing the same entry each + // complete their own file, and the last rename wins with whole + // bytes either way. + let temp_dest = crate::utils::partial_path(dest); // Download into the temp file, then atomically rename to the final path. // Verification runs before the rename so a corrupted download never // reaches the cache; on any failure after the temp file is created we - // remove it so stale .partial files don't accumulate. + // remove it so stale partial files don't accumulate. let mut hasher = DownloadHasher::new(); let result: anyhow::Result<()> = async { let mut file = tokio::fs::File::create(&temp_dest) @@ -579,7 +582,7 @@ async fn try_download_oci_image( // Bundle layers.manifest + all layer blobs into a single .tar.gz cache file // using the tar + flate2 crates — no external `tar` binary required. - let temp_dest = dest.with_extension("tar.partial"); + let temp_dest = crate::utils::partial_path(dest); let bundle_result: anyhow::Result<()> = (|| { use flate2::{write::GzEncoder, Compression}; diff --git a/crates/ecrou/src/rootfs.rs b/crates/ecrou/src/rootfs.rs index 0e4897b..6092848 100644 --- a/crates/ecrou/src/rootfs.rs +++ b/crates/ecrou/src/rootfs.rs @@ -136,8 +136,14 @@ impl RootfsCache { let dir = self.dir.join("layers"); std::fs::create_dir_all(&dir) .with_context(|| format!("Failed to create the layer directory {}", dir.display()))?; - std::fs::write(dir.join(name), cache_filename) + // Atomic write: a concurrent run reading the pointer must + // never see a truncated filename mid-write. + let pointer = dir.join(name); + let partial = crate::utils::partial_path(&pointer); + std::fs::write(&partial, cache_filename) .with_context(|| format!("Failed to write the layer pointer for {name}"))?; + std::fs::rename(&partial, &pointer) + .with_context(|| format!("Failed to move the layer pointer into place for {name}"))?; Ok(()) } @@ -522,7 +528,7 @@ fn drop_loopback_nameservers(config: &str) -> String { /// Write a tarball of `rootfs` at `dest`, compressed according to dest's /// extension (.tar.gz, .tar.xz, .tar.zst, plain .tar), atomically via a -/// `.partial` file. +/// unique partial file ([`crate::utils::partial_path`]). fn pack_rootfs(rootfs: &Path, dest: &Path) -> anyhow::Result<()> { // Packing the tree into a tarball placed inside it would recurse // forever: the walker keeps seeing the archive grow. @@ -541,7 +547,7 @@ fn pack_rootfs(rootfs: &Path, dest: &Path) -> anyhow::Result<()> { } let filename = dest.file_name().and_then(|n| n.to_str()).unwrap_or(""); - let partial = dest.with_extension("partial"); + let partial = crate::utils::partial_path(dest); let file = std::fs::File::create(&partial) .with_context(|| format!("Failed to create {}", partial.display()))?; @@ -848,12 +854,21 @@ mod tests { #[test] fn pack_failure_leaves_no_partial_file() { - // A nonexistent rootfs must fail cleanly and clean up the .partial + // A nonexistent rootfs must fail cleanly and clean up its + // partial file, whatever unique name it carried. let cache = tempfile::tempdir().unwrap(); let dest = cache.path().join("x.tar.gz"); assert!(pack_rootfs(Path::new("/nonexistent-rootfs"), &dest).is_err()); assert!(!dest.exists()); - assert!(!dest.with_extension("partial").exists()); + let leftovers: Vec = std::fs::read_dir(cache.path()) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .filter(|name| name.contains("partial")) + .collect(); + assert!( + leftovers.is_empty(), + "partial files left behind: {leftovers:?}" + ); } #[test] diff --git a/crates/ecrou/src/utils.rs b/crates/ecrou/src/utils.rs index d9da1e2..5711e22 100644 --- a/crates/ecrou/src/utils.rs +++ b/crates/ecrou/src/utils.rs @@ -1,7 +1,7 @@ use anyhow::anyhow; use crate::Result; -use std::path::Path; +use std::path::{Path, PathBuf}; // ============================================================================ // Constants @@ -35,6 +35,23 @@ pub fn random_suffix(len: usize) -> String { .collect() } +// ============================================================================ +// Unique scratch paths for atomic writes +// ============================================================================ + +/// Sequence counter for [`partial_path`], unique within the process. +static PARTIAL_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + +/// A sibling scratch path for writing `dest` before an atomic rename: +/// the pid separates concurrent processes, the sequence separates +/// threads. Two writers racing on the same cache entry each complete +/// their own file, so whichever rename lands last, the bytes at `dest` +/// are whole. +pub fn partial_path(dest: &Path) -> PathBuf { + let seq = PARTIAL_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + dest.with_extension(format!("partial.{}-{seq}", std::process::id())) +} + // ============================================================================ // Architecture handling // ============================================================================