Handle Windows AV/EDR file locks during script installations #9543

crates/uv-fs/src/lib.rs

@@ -216,6 +216,13 @@ pub fn copy_atomic_sync(from: impl AsRef<Path>, to: impl AsRef<Path>) -> std::io
     Ok(())
 }
 
+fn file_move_backoff() -> backoff::ExponentialBackoff {
+    backoff::ExponentialBackoffBuilder::default()
+        .with_initial_interval(std::time::Duration::from_millis(10))
+        .with_max_elapsed_time(Some(std::time::Duration::from_secs(10)))
+        .build()
+}
+
 /// Rename a file, retrying (on Windows) if it fails due to transient operating system errors.
 #[cfg(feature = "tokio")]
 pub async fn rename_with_retry(
@@ -227,15 +234,11 @@ pub async fn rename_with_retry(
         // This is most common for DLLs, and the common suggestion is to retry the operation with
         // some backoff.
         //
-        // See: <https://github.com/astral-sh/uv/issues/1491>
+        // See: <https://github.com/astral-sh/uv/issues/1491> & <https://github.com/astral-sh/uv/issues/9531>
         let from = from.as_ref();
         let to = to.as_ref();
 
-        let backoff = backoff::ExponentialBackoffBuilder::default()
-            .with_initial_interval(std::time::Duration::from_millis(10))
-            .with_max_elapsed_time(Some(std::time::Duration::from_secs(10)))
-            .build();
-
+        let backoff = file_move_backoff();
         backoff::future::retry(backoff, || async move {
             match fs_err::rename(from, to) {
                 Ok(()) => Ok(()),
@@ -257,6 +260,109 @@ pub async fn rename_with_retry(
     }
 }
 
+/// Rename a file, retrying (on Windows) if it fails due to transient operating system errors, in a synchronous context.
+pub fn rename_with_retry_sync(
+    from: impl AsRef<Path>,
+    to: impl AsRef<Path>,
+) -> Result<(), std::io::Error> {
+    if cfg!(windows) {
+        // On Windows, antivirus software can lock files temporarily, making them inaccessible.
+        // This is most common for DLLs, and the common suggestion is to retry the operation with
+        // some backoff.
+        //
+        // See: <https://github.com/astral-sh/uv/issues/1491> & <https://github.com/astral-sh/uv/issues/9531>
+        let from = from.as_ref();
+        let to = to.as_ref();
+
+        let backoff = file_move_backoff();
+        backoff::retry(backoff, || match fs_err::rename(from, to) {
+            Ok(()) => Ok(()),
+            Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => {
+                warn!(
+                    "Retrying rename from {} to {} due to transient error: {}",
+                    from.display(),
+                    to.display(),
+                    err
+                );
+                Err(backoff::Error::transient(err))
+            }
+            Err(err) => Err(backoff::Error::permanent(err)),
+        })
+        .map_err(|err| {
+            std::io::Error::new(
+                std::io::ErrorKind::Other,
+                format!(
+                    "Failed to rename {} to {}: {}",
+                    from.display(),
+                    to.display(),
+                    err
+                ),
+            )
+        })
+    } else {
+        fs_err::rename(from, to)
+    }
+}
+
+pub fn persist_with_retry_sync(
+    from: NamedTempFile,
+    to: impl AsRef<Path>,
+) -> Result<(), std::io::Error> {
+    if cfg!(windows) {
+        let to = to.as_ref();
+
+        // the NamedTempFile::persist method consumes `self`, and returns it back inside the Error in case of `PersistError`
+        // https://docs.rs/tempfile/latest/tempfile/struct.NamedTempFile.html#method.persist
+        // So we will update the `from` optional value in safe and borrow-checker friendly way every retry
+        // Allows us to use the NamedTempFile inside a FnMut closure used for backoff::retry
+        let mut from = Some(from);
+
+        let backoff = file_move_backoff();
+        let persisted = backoff::retry(backoff, move || {
+            if let Some(file) = from.take() {
+                file.persist(to).map_err(|err| {
+                    let error_message = err.to_string();
+                    warn!(
+                        "Retrying to persist temporary file to {}: {}",
+                        to.display(),
+                        error_message
+                    );
+
+                    // Set back the NamedTempFile returned back by the Error
+                    from = Some(err.file); 
+
+                    backoff::Error::transient(std::io::Error::new(
+                        std::io::ErrorKind::Other,
+                        format!(
+                            "Failed to persist temporary file to {}: {}",
+                            to.display(),
+                            error_message
+                        ),
+                    ))
+                })
+            } else {
+                Err(backoff::Error::permanent(std::io::Error::new(
+                    std::io::ErrorKind::Other,
+                    format!(
+                        "Failed to retrieve temporary file while trying to persist to {}",
+                        to.display()
+                    ),
+                )))
+            }
+        });
+
+        match persisted {
+            Ok(_) => Ok(()),
+            Err(err) => Err(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                format!("{:?}", err),
+            )),
+        }
+    } else {
+        fs_err::rename(from, to)
+    }
+}
+
 /// Iterate over the subdirectories of a directory.
 ///
 /// If the directory does not exist, returns an empty iterator.

crates/uv-install-wheel/src/wheel.rs

@@ -13,7 +13,7 @@ use tracing::{instrument, warn};
 use walkdir::WalkDir;
 
 use uv_cache_info::CacheInfo;
-use uv_fs::{relative_to, Simplified};
+use uv_fs::{persist_with_retry_sync, relative_to, rename_with_retry_sync, Simplified};
 use uv_normalize::PackageName;
 use uv_pypi_types::DirectUrl;
 use uv_shell::escape_posix_for_single_quotes;
@@ -424,16 +424,8 @@ fn install_script(
 
         let mut target = uv_fs::tempfile_in(&layout.scheme.scripts)?;
         let size_and_encoded_hash = copy_and_hash(&mut start.chain(script), &mut target)?;
-        target.persist(&script_absolute).map_err(|err| {
-            io::Error::new(
-                io::ErrorKind::Other,
-                format!(
-                    "Failed to persist temporary file to {}: {}",
-                    path.user_display(),
-                    err.error
-                ),
-            )
-        })?;
+
+        persist_with_retry_sync(target, &script_absolute)?;
         fs::remove_file(&path)?;
 
         // Make the script executable. We just created the file, so we can set permissions directly.
@@ -467,7 +459,7 @@ fn install_script(
 
             if permissions.mode() & 0o111 == 0o111 {
                 // If the permissions are already executable, we don't need to change them.
-                fs::rename(&path, &script_absolute)?;
+                rename_with_retry_sync(&path, &script_absolute)?;
             } else {
                 // If we have to modify the permissions, copy the file, since we might not own it.
                 warn!(
@@ -488,7 +480,7 @@ fn install_script(
 
         #[cfg(not(unix))]
         {
-            fs::rename(&path, &script_absolute)?;
+            rename_with_retry_sync(&path, &script_absolute)?;
         }
 
         None