Skip to content

Commit

Permalink
fix: retry writing lockfile on failure (denoland#24052)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dsherret committed Jun 1, 2024
1 parent f5c239f commit 38ff9fa
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
4 changes: 2 additions & 2 deletions cli/args/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
Expand Down
29 changes: 27 additions & 2 deletions cli/util/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: AsRef<[u8]>>(
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.
Expand All @@ -50,8 +72,11 @@ pub fn atomic_write_file<T: AsRef<[u8]>>(
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<()> {
Expand Down

0 comments on commit 38ff9fa

Please sign in to comment.