From 38ff9faff639385c10ccb6412470e1355c73327c Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 31 May 2024 23:25:08 -0400 Subject: [PATCH] fix: retry writing lockfile on failure (#24052) Ran into this running the deno_graph ecosystem tests where many processes writing to the same path at the same time would cause an error. --- cli/args/lockfile.rs | 4 ++-- cli/util/fs.rs | 29 +++++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs index 30e91eb92a..7e59853b0f 100644 --- a/cli/args/lockfile.rs +++ b/cli/args/lockfile.rs @@ -8,7 +8,7 @@ use deno_runtime::deno_node::PackageJson; use crate::args::ConfigFile; use crate::cache; -use crate::util::fs::atomic_write_file; +use crate::util::fs::atomic_write_file_with_retries; use crate::Flags; use super::DenoSubcommand; @@ -84,7 +84,7 @@ pub fn write_lockfile_if_has_changes( }; // do an atomic write to reduce the chance of multiple deno // processes corrupting the file - atomic_write_file(&lockfile.filename, bytes, cache::CACHE_PERM) + atomic_write_file_with_retries(&lockfile.filename, bytes, cache::CACHE_PERM) .context("Failed writing lockfile.")?; lockfile.has_content_changed = false; Ok(()) diff --git a/cli/util/fs.rs b/cli/util/fs.rs index 9bdb1d014e..5adb777b03 100644 --- a/cli/util/fs.rs +++ b/cli/util/fs.rs @@ -32,6 +32,28 @@ use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; use crate::util::progress_bar::ProgressMessagePrompt; +pub fn atomic_write_file_with_retries>( + file_path: &Path, + data: T, + mode: u32, +) -> std::io::Result<()> { + let mut count = 0; + loop { + match atomic_write_file(file_path, data.as_ref(), mode) { + Ok(()) => return Ok(()), + Err(err) => { + if count >= 5 { + // too many retries, return the error + return Err(err); + } + count += 1; + let sleep_ms = std::cmp::min(50, 10 * count); + std::thread::sleep(std::time::Duration::from_millis(sleep_ms)); + } + } + } +} + /// Writes the file to the file system at a temporary path, then /// renames it to the destination in a single sys call in order /// to never leave the file system in a corrupted state. @@ -50,8 +72,11 @@ pub fn atomic_write_file>( mode: u32, ) -> std::io::Result<()> { write_file(temp_file_path, data, mode)?; - std::fs::rename(temp_file_path, file_path)?; - Ok(()) + std::fs::rename(temp_file_path, file_path).map_err(|err| { + // clean up the created temp file on error + let _ = std::fs::remove_file(temp_file_path); + err + }) } fn inner(file_path: &Path, data: &[u8], mode: u32) -> std::io::Result<()> {