Repository: vercel/next.js
Pull request: Turbopack: Various cleanup for turbo-tasks-fs, mostly retry logic and string formatting

- https://github.com/vercel/next.js/pull/87661 removed the `tokio::task::spawn_blocking` call from `retry_blocking`, so we can simplify things a lot by removing the `Send + 'static` bounds, and eliminate a bunch of cloning `Path`s into `PathBuf`s.
- `retry_blocking` no longer takes a path argument. After removing the path cloning requirements, it's now easier for callers to capture paths via a closure, and it's more flexible for IO operations that need to pass in more than a single path.
- Adds `retry_blocking_custom`, which I use in #88669 that allows customizing what error kinds we retry on.
- Adds documentation to `retry_blocking` explaining why we need this. It's often unclear to new people on the team why we have to retry filesystem operations (Windows, basically). This was unclear to me when I was new to this code, and other people have asked similar questions.
- We shouldn't use `Path::display` for error messages or tracing spans. It's easier to use `Debug`, and `Debug` ensures that if the error is related to unicode, that information isn't lost in the error message.
- More consistently apply tracing spans using `.instrument`.
- Use the modern inline syntax for some format strings.
- `let foo = RcStr::from(bar)` is preferred over `let foo: RcStr = bar.into()`

Changed files:
filename: turbopack/crates/turbo-tasks-fs/src/lib.rs
patch: @@ -39,7 +39,7 @@ use std::{
     fmt::{self, Debug, Display, Formatter},
     fs::FileType,
     future::Future,
-    io::{self, BufRead, BufReader, ErrorKind, Read},
+    io::{self, BufRead, BufReader, ErrorKind, Read, Write as _},
     mem::take,
     path::{MAIN_SEPARATOR, Path, PathBuf},
     sync::{Arc, LazyLock, Weak},
@@ -127,61 +127,56 @@ pub const MAX_SAFE_FILE_NAME_LENGTH: usize = 200;
 pub fn validate_path_length(path: &Path) -> Result<Cow<'_, Path>> {
     /// Here we check if the path is too long for windows, and if so, attempt to canonicalize it
     /// to a UNC path.
-    #[cfg(windows)]
     fn validate_path_length_inner(path: &Path) -> Result<Cow<'_, Path>> {
-        const MAX_PATH_LENGTH_WINDOWS: usize = 260;
-        const UNC_PREFIX: &str = "\\\\?\\";
+        if cfg!(windows) {
+            const MAX_PATH_LENGTH_WINDOWS: usize = 260;
+            const UNC_PREFIX: &str = "\\\\?\\";
 
-        if path.starts_with(UNC_PREFIX) {
-            return Ok(path.into());
-        }
+            if path.starts_with(UNC_PREFIX) {
+                return Ok(path.into());
+            }
 
-        if path.as_os_str().len() > MAX_PATH_LENGTH_WINDOWS {
-            let new_path = std::fs::canonicalize(path)
-                .map_err(|_| anyhow!("file is too long, and could not be normalized"))?;
-            return Ok(new_path.into());
-        }
+            if path.as_os_str().len() > MAX_PATH_LENGTH_WINDOWS {
+                let new_path = std::fs::canonicalize(path).map_err(|err| {
+                    anyhow!(err).context("file is too long, and could not be normalized")
+                })?;
+                return Ok(new_path.into());
+            }
 
-        Ok(path.into())
-    }
+            Ok(path.into())
+        } else {
+            /// here we are only going to check if the total length exceeds, or the last segment
+            /// exceeds. This heuristic is primarily to avoid long file names, and it makes the
+            /// operation much cheaper.
+            const MAX_FILE_NAME_LENGTH_UNIX: usize = 255;
+            // macOS reports a limit of 1024, but I (@arlyon) have had issues with paths above 1016
+            // so we subtract a bit to be safe. on most linux distros this is likely a lot larger
+            // than 1024, but macOS is *special*
+            const MAX_PATH_LENGTH: usize = 1024 - 8;
+
+            // check the last segment (file name)
+            if path
+                .file_name()
+                .map(|n| n.as_encoded_bytes().len())
+                .unwrap_or(0)
+                > MAX_FILE_NAME_LENGTH_UNIX
+            {
+                anyhow::bail!(
+                    "file name is too long (exceeds {} bytes)",
+                    MAX_FILE_NAME_LENGTH_UNIX,
+                );
+            }
 
-    /// Here we are only going to check if the total length exceeds, or the last segment exceeds.
-    /// This heuristic is primarily to avoid long file names, and it makes the operation much
-    /// cheaper.
-    #[cfg(not(windows))]
-    fn validate_path_length_inner(path: &Path) -> Result<Cow<'_, Path>> {
-        const MAX_FILE_NAME_LENGTH_UNIX: usize = 255;
-        // macOS reports a limit of 1024, but I (@arlyon) have had issues with paths above 1016
-        // so we subtract a bit to be safe. on most linux distros this is likely a lot larger than
-        // 1024, but macOS is *special*
-        const MAX_PATH_LENGTH: usize = 1024 - 8;
-
-        // check the last segment (file name)
-        if path
-            .file_name()
-            .map(|n| n.as_encoded_bytes().len())
-            .unwrap_or(0)
-            > MAX_FILE_NAME_LENGTH_UNIX
-        {
-            anyhow::bail!(
-                "file name is too long (exceeds {} bytes)",
-                MAX_FILE_NAME_LENGTH_UNIX
-            );
-        }
+            if path.as_os_str().len() > MAX_PATH_LENGTH {
+                anyhow::bail!("path is too long (exceeds {MAX_PATH_LENGTH} bytes)");
+            }
 
-        if path.as_os_str().len() > MAX_PATH_LENGTH {
-            anyhow::bail!("path is too long (exceeds {} bytes)", MAX_PATH_LENGTH);
+            Ok(path.into())
         }
-
-        Ok(path.into())
     }
 
-    validate_path_length_inner(path).with_context(|| {
-        format!(
-            "path length for file {} exceeds max length of filesystem",
-            path.to_string_lossy()
-        )
-    })
+    validate_path_length_inner(path)
+        .with_context(|| format!("path length for file {path:?} exceeds max length of filesystem"))
 }
 
 trait ConcurrencyLimitedExt {
@@ -469,15 +464,10 @@ impl DiskFileSystemInner {
         let root_path = self.root_path().to_path_buf();
 
         // create the directory for the filesystem on disk, if it doesn't exist
-        retry_blocking(root_path.clone(), move |path| {
-            let _tracing =
-                tracing::info_span!("create root directory", name = display(path.display()))
-                    .entered();
-
-            std::fs::create_dir_all(path)
-        })
-        .concurrency_limited(&self.write_semaphore)
-        .await?;
+        retry_blocking(|| std::fs::create_dir_all(&root_path))
+            .instrument(tracing::info_span!("create root directory", name = ?root_path))
+            .concurrency_limited(&self.write_semaphore)
+            .await?;
 
         self.watcher
             .start_watching(self.clone(), report_invalidation_reason, poll_interval)?;
@@ -491,12 +481,8 @@ impl DiskFileSystemInner {
             |fs_context| fs_context.created_directories.contains(directory),
         );
         if !already_created {
-            let func = |p: &Path| std::fs::create_dir_all(p);
-            retry_blocking(directory.to_path_buf(), func)
-                .instrument(tracing::info_span!(
-                    "create directory",
-                    name = display(directory.display())
-                ))
+            retry_blocking(|| std::fs::create_dir_all(directory))
+                .instrument(tracing::info_span!("create directory", name = ?directory))
                 .concurrency_limited(&self.write_semaphore)
                 .await?;
             ApplyEffectsContext::with(|fs_context: &mut DiskFileSystemApplyContext| {
@@ -707,21 +693,16 @@ impl FileSystem for DiskFileSystem {
         self.inner.register_read_invalidator(&full_path)?;
 
         let _lock = self.inner.lock_path(&full_path).await;
-        let content = match retry_blocking(full_path.clone(), |path: &Path| File::from_path(path))
-            .instrument(tracing::info_span!(
-                "read file",
-                name = display(full_path.display())
-            ))
+        let content = match retry_blocking(|| File::from_path(&full_path))
+            .instrument(tracing::info_span!("read file", name = ?full_path))
             .concurrency_limited(&self.inner.read_semaphore)
             .await
         {
             Ok(file) => FileContent::new(file),
             Err(e) if e.kind() == ErrorKind::NotFound || e.kind() == ErrorKind::InvalidFilename => {
                 FileContent::NotFound
             }
-            Err(e) => {
-                bail!(anyhow!(e).context(format!("reading file {}", full_path.display())))
-            }
+            Err(e) => return Err(anyhow!(e).context(format!("reading file {full_path:?}"))),
         };
         Ok(content.cell())
     }
@@ -738,15 +719,11 @@ impl FileSystem for DiskFileSystem {
 
         self.inner.register_dir_invalidator(&full_path)?;
 
-        // we use the sync std function here as it's a lot faster (600%) in
-        // node-file-trace
-        let read_dir = match retry_blocking(full_path.clone(), |path| {
-            let _span =
-                tracing::info_span!("read directory", name = display(path.display())).entered();
-            std::fs::read_dir(path)
-        })
-        .concurrency_limited(&self.inner.read_semaphore)
-        .await
+        // we use the sync std function here as it's a lot faster (600%) in node-file-trace
+        let read_dir = match retry_blocking(|| std::fs::read_dir(&full_path))
+            .instrument(tracing::info_span!("read directory", name = ?full_path))
+            .concurrency_limited(&self.inner.read_semaphore)
+            .await
         {
             Ok(dir) => dir,
             Err(e)
@@ -757,7 +734,7 @@ impl FileSystem for DiskFileSystem {
                 return Ok(RawDirectoryContent::not_found());
             }
             Err(e) => {
-                bail!(anyhow!(e).context(format!("reading dir {}", full_path.display())))
+                return Err(anyhow!(e).context(format!("reading dir {full_path:?}")));
             }
         };
         let dir_path = fs_path.path.as_str();
@@ -797,7 +774,7 @@ impl FileSystem for DiskFileSystem {
                 };
 
                 // we filter out any non unicode names
-                let file_name: RcStr = e.file_name().to_str()?.into();
+                let file_name = RcStr::from(e.file_name().to_str()?);
                 // Filter out denied entries
                 if denied_entries.contains(file_name.as_str()) {
                     return None;
@@ -814,7 +791,7 @@ impl FileSystem for DiskFileSystem {
                 Some(anyhow::Ok((file_name, entry)))
             })
             .collect::<Result<_>>()
-            .with_context(|| format!("reading directory item in {}", full_path.display()))?;
+            .with_context(|| format!("reading directory item in {full_path:?}"))?;
 
         Ok(RawDirectoryContent::new(entries))
     }
@@ -832,18 +809,14 @@ impl FileSystem for DiskFileSystem {
         self.inner.register_read_invalidator(&full_path)?;
 
         let _lock = self.inner.lock_path(&full_path).await;
-        let link_path =
-            match retry_blocking(full_path.clone(), |path: &Path| std::fs::read_link(path))
-                .instrument(tracing::info_span!(
-                    "read symlink",
-                    name = display(full_path.display())
-                ))
-                .concurrency_limited(&self.inner.read_semaphore)
-                .await
-            {
-                Ok(res) => res,
-                Err(_) => return Ok(LinkContent::NotFound.cell()),
-            };
+        let link_path = match retry_blocking(|| std::fs::read_link(&full_path))
+            .instrument(tracing::info_span!("read symlink", name = ?full_path))
+            .concurrency_limited(&self.inner.read_semaphore)
+            .await
+        {
+            Ok(res) => res,
+            Err(_) => return Ok(LinkContent::NotFound.cell()),
+        };
         let is_link_absolute = link_path.is_absolute();
 
         let mut file = link_path.clone();
@@ -880,7 +853,7 @@ impl FileSystem for DiskFileSystem {
         };
 
         let (target, file_type) = if is_link_absolute {
-            let target_string: RcStr = relative_to_root_path.to_string_lossy().into();
+            let target_string = RcStr::from(relative_to_root_path.to_string_lossy());
             (
                 target_string.clone(),
                 FileSystemPath::new_normalized(fs_path.fs().to_resolved().await?, target_string)
@@ -889,7 +862,7 @@ impl FileSystem for DiskFileSystem {
             )
         } else {
             let link_path_string_cow = link_path.to_string_lossy();
-            let link_path_unix: RcStr = sys_to_unix(&link_path_string_cow).into();
+            let link_path_unix = RcStr::from(sys_to_unix(&link_path_string_cow));
             (
                 link_path_unix.clone(),
                 fs_path.parent().join(&link_path_unix)?.get_type().await?,
@@ -956,10 +929,7 @@ impl FileSystem for DiskFileSystem {
             // not wasting cycles.
             let compare = content
                 .streaming_compare(&full_path)
-                .instrument(tracing::info_span!(
-                    "read file before write",
-                    name = display(full_path.display())
-                ))
+                .instrument(tracing::info_span!("read file before write", name = ?full_path))
                 .concurrency_limited(&inner.read_semaphore)
                 .await?;
             if compare == FileComparison::Equal {
@@ -981,19 +951,14 @@ impl FileSystem for DiskFileSystem {
                     if create_directory && let Some(parent) = full_path.parent() {
                         inner.create_directory(parent).await.with_context(|| {
                             format!(
-                                "failed to create directory {} for write to {}",
-                                parent.display(),
-                                full_path.display()
+                                "failed to create directory {parent:?} for write to {full_path:?}",
                             )
                         })?;
                     }
 
-                    let full_path_to_write = full_path.clone();
                     let content = content.clone();
-                    retry_blocking(full_path_to_write.into_owned(), move |full_path| {
-                        use std::io::Write;
-
-                        let mut f = std::fs::File::create(full_path)?;
+                    retry_blocking(|| {
+                        let mut f = std::fs::File::create(&full_path)?;
                         let FileContent::Content(file) = &*content else {
                             unreachable!()
                         };
@@ -1007,7 +972,7 @@ impl FileSystem for DiskFileSystem {
                                 .is_some_and(|v| v == "1" || v == "true")
                         });
                         if *WRITE_VERSION {
-                            let mut full_path = full_path.to_owned();
+                            let mut full_path = full_path.clone().into_owned();
                             let hash = hash_xxh3_hash64(file);
                             let ext = full_path.extension();
                             let ext = if let Some(ext) = ext {
@@ -1024,32 +989,24 @@ impl FileSystem for DiskFileSystem {
                         }
                         Ok::<(), io::Error>(())
                     })
-                    .instrument(tracing::info_span!(
-                        "write file",
-                        name = display(full_path.display())
-                    ))
+                    .instrument(tracing::info_span!("write file", name = ?full_path))
                     .concurrency_limited(&inner.write_semaphore)
                     .await
-                    .with_context(|| format!("failed to write to {}", full_path.display()))?;
+                    .with_context(|| format!("failed to write to {full_path:?}"))?;
                 }
                 FileContent::NotFound => {
-                    retry_blocking(full_path.clone().into_owned(), |path| {
-                        std::fs::remove_file(path)
-                    })
-                    .instrument(tracing::info_span!(
-                        "remove file",
-                        name = display(full_path.display())
-                    ))
-                    .concurrency_limited(&inner.write_semaphore)
-                    .await
-                    .or_else(|err| {
-                        if err.kind() == ErrorKind::NotFound {
-                            Ok(())
-                        } else {
-                            Err(err)
-                        }
-                    })
-                    .with_context(|| format!("removing {} failed", full_path.display()))?;
+                    retry_blocking(|| std::fs::remove_file(&full_path))
+                        .instrument(tracing::info_span!("remove file", name = ?full_path))
+                        .concurrency_limited(&inner.write_semaphore)
+                        .await
+                        .or_else(|err| {
+                            if err.kind() == ErrorKind::NotFound {
+                                Ok(())
+                            } else {
+                                Err(err)
+                            }
+                        })
+                        .with_context(|| format!("removing {full_path:?} failed"))?;
                 }
             }
 
@@ -1134,17 +1091,10 @@ impl FileSystem for DiskFileSystem {
                 LinkContent::NotFound => OsSpecificLinkContent::NotFound,
             };
 
-            // TODO(sokra) perform a untracked read here, register an invalidator and get
-            // all existing invalidators
-            let old_content = match retry_blocking(full_path.clone().into_owned(), |path| {
-                std::fs::read_link(path)
-            })
-            .instrument(tracing::info_span!(
-                "read symlink before write",
-                name = display(full_path.display())
-            ))
-            .concurrency_limited(&inner.read_semaphore)
-            .await
+            let old_content = match retry_blocking(|| std::fs::read_link(&full_path))
+                .instrument(tracing::info_span!("read symlink before write", name = ?full_path))
+                .concurrency_limited(&inner.read_semaphore)
+                .await
             {
                 Ok(res) => Some((res.is_absolute(), res)),
                 Err(_) => None,
@@ -1183,9 +1133,8 @@ impl FileSystem for DiskFileSystem {
                     if create_directory && let Some(parent) = full_path.parent() {
                         inner.create_directory(parent).await.with_context(|| {
                             format!(
-                                "failed to create directory {} for write link to {}",
-                                parent.display(),
-                                full_path.display()
+                                "failed to create directory {parent:?} for write link to \
+                                 {full_path:?}",
                             )
                         })?;
                     }
@@ -1197,67 +1146,62 @@ impl FileSystem for DiskFileSystem {
                         remove_symbolic_link_dir_helper(&full_path)
                             .instrument(tracing::info_span!(
                                 "remove existing symlink before write",
-                                name = display(full_path.display())
+                                name = ?full_path,
                             ))
                             .concurrency_limited(&inner.write_semaphore)
                             .await
                             .with_context(|| {
-                                format!("removing existing symlink {} failed", full_path.display())
+                                format!("removing existing symlink {full_path:?} failed")
                             })?;
                     }
 
-                    let span =
-                        tracing::info_span!("create symlink", name = display(full_path.display()));
-                    retry_blocking(target.clone(), move |target_path| {
-                        let _span = tracing::info_span!(
-                            "write symlink",
-                            name = display(target_path.display())
-                        )
-                        .entered();
+                    retry_blocking(|| {
                         #[cfg(not(windows))]
                         {
-                            std::os::unix::fs::symlink(target_path, &full_path)
+                            std::os::unix::fs::symlink(&target, &full_path)
                         }
                         #[cfg(windows)]
                         {
                             if is_directory {
-                                std::os::windows::fs::junction_point(target_path, &full_path)
+                                std::os::windows::fs::junction_point(&target, &full_path)
                             } else {
-                                std::os::windows::fs::symlink_file(target_path, &full_path)
+                                std::os::windows::fs::symlink_file(&target, &full_path)
                             }
                         }
                     })
-                    .instrument(span)
+                    .instrument(tracing::info_span!(
+                        "write symlink",
+                        name = ?full_path,
+                        target = ?target,
+                    ))
                     .concurrency_limited(&inner.write_semaphore)
                     .await
                     .with_context(|| {
                         #[cfg(not(windows))]
-                        let message = format!("failed to create symlink to {}", target.display());
+                        let message = format!("failed to create symlink to {target:?}");
                         #[cfg(windows)]
                         let message = if is_directory {
-                            format!("failed to create junction point to {}", target.display())
+                            format!("failed to create junction point to {target:?}")
                         } else {
                             format!(
-                                "failed to create symlink to {}\n\
-                                (Note: creating file symlinks on Windows require developer mode or admin permissions: https://learn.microsoft.com/en-us/windows/advanced-settings/developer-mode)",
-                                target.display()
+                                "failed to create symlink to {target:?}\n\
+                                (Note: creating file symlinks on Windows require developer mode or \
+                                admin permissions: \
+                                https://learn.microsoft.com/en-us/windows/advanced-settings/developer-mode)",
                             )
                         };
                         message
                     })?;
                 }
                 OsSpecificLinkContent::Invalid => {
-                    bail!("invalid symlink target: {}", full_path.display())
+                    bail!("invalid symlink target: {full_path:?}")
                 }
                 OsSpecificLinkContent::NotFound => {
                     remove_symbolic_link_dir_helper(&full_path)
-                        .instrument(tracing::info_span!(
-                            "remove symlink",
-                            name = display(full_path.display())
-                        ))
+                        .instrument(tracing::info_span!("remove symlink", name = ?full_path))
                         .concurrency_limited(&inner.write_semaphore)
                         .await
-                        .with_context(|| format!("removing {} failed", full_path.display()))?;
+                        .with_context(|| format!("removing {full_path:?} failed"))?;
                 }
             }
 
@@ -1282,22 +1226,18 @@ impl FileSystem for DiskFileSystem {
         self.inner.register_read_invalidator(&full_path)?;
 
         let _lock = self.inner.lock_path(&full_path).await;
-        let meta = retry_blocking(full_path.clone(), |path| std::fs::metadata(path))
-            .instrument(tracing::info_span!(
-                "read metadata",
-                name = display(full_path.display())
-            ))
+        let meta = retry_blocking(|| std::fs::metadata(&full_path))
+            .instrument(tracing::info_span!("read metadata", name = ?full_path))
             .concurrency_limited(&self.inner.read_semaphore)
             .await
-            .with_context(|| format!("reading metadata for {}", full_path.display()))?;
+            .with_context(|| format!("reading metadata for {:?}", full_path))?;
 
         Ok(FileMeta::cell(meta.into()))
     }
 }
 
-async fn remove_symbolic_link_dir_helper(path: impl AsRef<Path>) -> Result<()> {
-    let path = path.as_ref();
-    retry_blocking(path.to_owned(), move |path| {
+async fn remove_symbolic_link_dir_helper(path: &Path) -> Result<()> {
+    retry_blocking(|| {
         if cfg!(windows) {
             // Junction points on Windows are treated as directories, and therefore need
             // `remove_dir`:
@@ -1531,7 +1471,7 @@ impl FileSystemPath {
             bail!(
                 "FileSystemPath(\"{}\").join(\"{}\") leaves the filesystem root",
                 self.path,
-                path
+                path,
             );
         }
     }
@@ -1542,7 +1482,7 @@ impl FileSystemPath {
             bail!(
                 "FileSystemPath(\"{}\").append(\"{}\") must not append '/'",
                 self.path,
-                path
+                path,
             )
         }
         Ok(Self::new_normalized(
@@ -1558,7 +1498,7 @@ impl FileSystemPath {
             bail!(
                 "FileSystemPath(\"{}\").append_to_stem(\"{}\") must not append '/'",
                 self.path,
-                appending
+                appending,
             )
         }
         if let (path, Some(ext)) = self.split_extension() {
@@ -1925,10 +1865,8 @@ impl FileContent {
     /// Performs a comparison of self's data against a disk file's streamed
     /// read.
     async fn streaming_compare(&self, path: &Path) -> Result<FileComparison> {
-        let old_file = extract_disk_access(
-            retry_blocking(path.to_path_buf(), |path| std::fs::File::open(path)).await,
-            path,
-        )?;
+        let old_file =
+            extract_disk_access(retry_blocking(|| std::fs::File::open(path)).await, path)?;
         let Some(old_file) = old_file else {
             return Ok(match self {
                 FileContent::NotFound => FileComparison::Equal,
@@ -1940,14 +1878,7 @@ impl FileContent {
             return Ok(FileComparison::NotEqual);
         };
 
-        let old_meta = extract_disk_access(
-            retry_blocking(path.to_path_buf(), {
-                let file_for_metadata = old_file.try_clone()?;
-                move |_| file_for_metadata.metadata()
-            })
-            .await,
-            path,
-        )?;
+        let old_meta = extract_disk_access(retry_blocking(|| old_file.metadata()).await, path)?;
         let Some(old_meta) = old_meta else {
             // If we failed to get meta, then the old file has been deleted between the
             // handle open. In which case, we just pretend the file never
@@ -3144,7 +3075,7 @@ mod tests {
             }
             create_dir_all(path.join("_symlinks")).unwrap();
 
-            let root: RcStr = path.to_str().unwrap().into();
+            let root = RcStr::from(path.to_str().unwrap());
 
             let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(
                 BackendOptions::default(),
@@ -3248,9 +3179,9 @@ mod tests {
                 .write_all(b"deep secret")
                 .unwrap();
 
-            let root: RcStr = path.to_str().unwrap().into();
+            let root = RcStr::from(path.to_str().unwrap());
             // denied_path should be relative to root, using unix separators
-            let denied_path: RcStr = rcstr!("denied_dir");
+            let denied_path = rcstr!("denied_dir");
 
             (scratch, root, denied_path)
         }

filename: turbopack/crates/turbo-tasks-fs/src/retry.rs
patch: @@ -1,21 +1,36 @@
 use std::{
     io::{self, ErrorKind},
-    path::{Path, PathBuf},
     time::Duration,
 };
 
 const MAX_RETRY_ATTEMPTS: usize = 10;
 
-pub(crate) async fn retry_blocking<R, F>(path: PathBuf, func: F) -> io::Result<R>
-where
-    F: Fn(&Path) -> io::Result<R> + Send + 'static,
-    R: Send + 'static,
-{
-    let mut attempt = 1;
+/// Retries a blocking io operation up to `MAX_RETRY_ATTEMPTS` in a loop. Retries upon
+/// [`ErrorKind::PermissionDenied`] or [`ErrorKind::WouldBlock`]. This default behavior is
+/// implemented by [`can_retry`].
+///
+/// Retry logic is rarely useful on POSIX operating systems, but is often useful on Windows with
+/// NTFS. E.g. a file deletion may fail because an AV process has opened the file, and opened files
+/// cannot be deleted on Windows. Retries are the only mitigation for such issues.
+///
+/// This used to use [`tokio::task::spawn_blocking`], but now the IO operation blocks the current
+/// thread, as we already wrap callers of this function in
+/// [`crate::ConcurrencyLimitedExt::concurrency_limited`] and it
+/// appears to be faster to block the tokio thread than to spawn. Most of our IO operations are very
+/// short anyways. See this PR: <https://github.com/vercel/next.js/pull/87661>.
+pub(crate) async fn retry_blocking<T>(func: impl FnMut() -> io::Result<T>) -> io::Result<T> {
+    retry_blocking_custom(func, can_retry).await
+}
 
+/// A customizable version of [`retry_blocking`] that allows retrying on arbitrary errors.
+pub(crate) async fn retry_blocking_custom<T, E>(
+    mut func: impl FnMut() -> Result<T, E>,
+    mut can_retry: impl FnMut(&E) -> bool,
+) -> Result<T, E> {
+    let mut attempt = 1;
     loop {
-        return match func(&path) {
-            Ok(r) => Ok(r),
+        return match func() {
+            Ok(val) => Ok(val),
             Err(err) => {
                 if attempt < MAX_RETRY_ATTEMPTS && can_retry(&err) {
                     tokio::time::sleep(get_retry_wait_time(attempt)).await;
@@ -29,7 +44,9 @@ where
     }
 }
 
-fn can_retry(err: &io::Error) -> bool {
+/// The default implementation of error checking used by [`retry_blocking`]. Returns true if it
+/// would make sense to retry a failed IO operation.
+pub fn can_retry(err: &io::Error) -> bool {
     matches!(
         err.kind(),
         ErrorKind::PermissionDenied | ErrorKind::WouldBlock