From 9d422a18b1474969c9ca70f2d6c3a990c19ea721 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sun, 11 Oct 2026 15:23:27 +0000 Subject: [PATCH] Retry Windows sharing and lock violations, with a capped backoff retry_rename also retries ERROR_SHARING_VIOLATION (32) and ERROR_LOCK_VIOLATION (33), which a rename can report while a scanner holds the file, and caps each pause at 200 ms. --- crates/atomic/src/lib.rs | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/atomic/src/lib.rs b/crates/atomic/src/lib.rs index c1e93750..5ea15165 100644 --- a/crates/atomic/src/lib.rs +++ b/crates/atomic/src/lib.rs @@ -114,12 +114,13 @@ fn full_flush_unsupported(e: &io::Error) -> bool { cfg!(target_vendor = "apple") && matches!(e.raw_os_error(), Some(ENOTSUP | ENOTTY)) } -/// Retry configuration for renames that encounter transient file locks (`PermissionDenied`). +/// Retry configuration for renames that encounter transient file locks (see [`retry_rename`]). #[derive(Debug, Clone, Copy)] pub struct RenameRetry { /// Number of retries after the initial attempt (0 = no retry). pub attempts: u32, - /// Delay before the first retry in milliseconds; doubles on each subsequent attempt. + /// Delay before the first retry in milliseconds; doubles on each subsequent attempt, up to + /// 200 ms per attempt. pub first_delay_ms: u64, } @@ -286,22 +287,32 @@ fn copy_new(fs: &dyn AtomicFs, tmp: &Path, target: &Path) -> io::Result<()> { copied } -/// Retries an operation with exponential backoff while it fails with `PermissionDenied`. +/// The longest pause between two rename attempts. +const MAX_RETRY_DELAY_MS: u64 = 200; + +/// Retries an operation with exponential backoff while it fails because another process holds +/// the file: `PermissionDenied`, or on Windows a sharing or lock violation. pub fn retry_rename(retry: RenameRetry, mut rename: impl FnMut() -> io::Result<()>) -> io::Result<()> { - let mut delay = retry.first_delay_ms; + let mut delay = retry.first_delay_ms.min(MAX_RETRY_DELAY_MS); let mut left = retry.attempts; loop { match rename() { - Err(e) if e.kind() == io::ErrorKind::PermissionDenied && left > 0 => { + Err(e) if is_transient_lock(&e) && left > 0 => { left -= 1; sleep_ms(delay); - delay = delay.saturating_mul(2); + delay = delay.saturating_mul(2).min(MAX_RETRY_DELAY_MS); } r => return r, } } } +fn is_transient_lock(e: &io::Error) -> bool { + const ERROR_SHARING_VIOLATION: i32 = 32; + const ERROR_LOCK_VIOLATION: i32 = 33; + e.kind() == io::ErrorKind::PermissionDenied || (cfg!(windows) && matches!(e.raw_os_error(), Some(ERROR_SHARING_VIOLATION | ERROR_LOCK_VIOLATION))) +} + #[cfg(not(target_arch = "wasm32"))] fn sleep_ms(ms: u64) { if ms > 0 { @@ -639,4 +650,15 @@ mod tests { assert_eq!(fs.renames.get(), 0); assert_no_temp(&d.0); } + + #[test] + fn retry_delays_are_capped() { + let d = TempDir::new("cap"); + let p = d.0.join("doc.craft"); + let fs = Faulty { locked_renames: Cell::new(2), ..Default::default() }; + let started = std::time::Instant::now(); + assert!(atomic_write_custom(&fs, RenameRetry { attempts: 2, first_delay_ms: 60_000 }, &p, b"new").is_ok()); + assert!(started.elapsed() < std::time::Duration::from_secs(5)); + assert_eq!(fs.renames.get(), 3); + } }