fix: lock to ensure atomicity of remove + rename operations

Author: fengmk2

.github/workflows/ci.yml

@@ -172,7 +172,7 @@ jobs:
 
       - name: Run CLI E2E tests
         run: |
-          pnpm test
+          RUST_BACKTRACE=1 pnpm test
           git diff --exit-code
 
   install-e2e-test:

Cargo.lock

@@ -41,6 +41,7 @@ criterion = { version = "0.7", features = ["html_reports"] }
 crossterm = { version = "0.29.0", features = ["event-stream"] }
 directories = "6.0.0"
 flate2 = "1.0.35"
+fs4 = "0.13"
 futures-util = "0.3.31"
 hex = "0.4.3"
 httpmock = "0.7"

Cargo.toml

@@ -11,6 +11,7 @@ rust-version.workspace = true
 backon = { workspace = true }
 directories = { workspace = true }
 flate2 = { workspace = true }
+fs4 = { workspace = true }
 futures-util = { workspace = true }
 hex = { workspace = true }
 indoc = { workspace = true }

crates/vite_install/Cargo.toml

@@ -7,6 +7,9 @@ use std::{
     process::{ExitStatus, Stdio},
 };
 
+use fs4::fs_std::FileExt;
+#[cfg(not(target_os = "windows"))]
+use nix::sys::statfs::statfs;
 use semver::{Version, VersionReq};
 use serde::{Deserialize, Serialize};
 use tokio::{fs::remove_dir_all, process::Command};
@@ -320,6 +323,65 @@ async fn get_latest_version(package_manager_type: PackageManagerType) -> Result<
     Ok(package_json.version)
 }
 
+/// Check if a path is on an NFS filesystem.
+/// NFS file locking can be unreliable, so we skip locking on NFS.
+#[cfg(not(target_os = "windows"))]
+fn is_on_nfs(path: &Path) -> bool {
+    match statfs(path) {
+        Ok(stat) => {
+            // NFS magic numbers:
+            // - Linux: 0x6969 (NFS_SUPER_MAGIC)
+            // - macOS/BSD: Different approach - check f_fstypename
+            #[cfg(target_os = "linux")]
+            {
+                stat.filesystem_type().0 == nix::sys::statfs::NFS_SUPER_MAGIC.0
+            }
+            #[cfg(not(target_os = "linux"))]
+            {
+                // On macOS and BSD, check the filesystem type name
+                let fs_type = stat.filesystem_type_name();
+                fs_type == "nfs"
+            }
+        }
+        Err(_) => {
+            // If we can't determine, assume it's not NFS and try to lock
+            false
+        }
+    }
+}
+
+#[cfg(target_os = "windows")]
+fn is_on_nfs(_path: &Path) -> bool {
+    // On Windows, we could check for network drives, but for simplicity
+    // we'll assume local filesystem. Windows file locking is generally reliable.
+    false
+}
+
+/// A file lock guard that holds the lock until dropped.
+/// If the filesystem is NFS, no actual lock is acquired.
+struct FileLockGuard {
+    _file: Option<File>,
+}
+
+/// Acquire a file lock at the specified path.
+/// Automatically detects NFS filesystems and skips locking if on NFS.
+/// Returns a guard that releases the lock when dropped.
+fn acquire_file_lock(lock_path: &Path) -> Result<FileLockGuard, Error> {
+    // Check if the path is on NFS
+    if is_on_nfs(lock_path) {
+        tracing::debug!("Path {:?} is on NFS filesystem, skipping file lock", lock_path);
+        return Ok(FileLockGuard { _file: None });
+    }
+
+    tracing::debug!("Acquiring lock file: {:?}", lock_path);
+    let lock_file = File::create(lock_path)?;
+    // Acquire exclusive lock (blocks until available)
+    lock_file.lock_exclusive()?;
+    tracing::debug!("Lock acquired: {:?}", lock_path);
+
+    Ok(FileLockGuard { _file: Some(lock_file) })
+}
+
 /// Download the package manager and extract it to the cache directory.
 /// Return the install directory, e.g. $`CACHE_DIR/vite/package_manager/pnpm/10.0.0/pnpm`
 async fn download_package_manager(
@@ -374,9 +436,17 @@ async fn download_package_manager(
     tracing::debug!("Rename package dir to {}", bin_name);
     tokio::fs::rename(&target_dir_tmp.join("package"), &target_dir_tmp.join(&bin_name)).await?;
 
-    // check bin_file again, for the concurrent download cases
+    // Use a file-based lock to ensure atomicity of remove + rename operations
+    // This prevents DirectoryNotEmpty exceptions when multiple processes/threads
+    // try to install the same package manager version concurrently.
+    // The lock is automatically skipped on NFS filesystems where locking is unreliable.
+    let lock_path = parent_dir.join(format!("{version}.lock"));
+    let _lock_guard = acquire_file_lock(lock_path.as_ref())?;
+
+    // Check again after acquiring the lock, in case another thread completed
+    // the installation while we were downloading
     if is_exists_file(&bin_file)? {
-        tracing::debug!("bin_file already exists, skip rename");
+        tracing::debug!("bin_file already exists after lock acquisition, skip rename");
         return Ok(install_dir);
     }
 
@@ -395,12 +465,13 @@ async fn download_package_manager(
 /// Remove the directory and all its contents.
 /// Ignore the error if the directory is not found.
 async fn remove_dir_all_force(path: impl AsRef<Path>) -> Result<(), std::io::Error> {
-    match remove_dir_all(path).await {
+    match remove_dir_all(path.as_ref()).await {
         Ok(()) => Ok(()),
         Err(e) => {
             if e.kind() == std::io::ErrorKind::NotFound {
                 Ok(())
             } else {
+                tracing::error!("remove_dir_all_force path: {:?} error: {e:?}", path.as_ref());
                 Err(e)
             }
         }

crates/vite_install/src/package_manager.rs

@@ -126,6 +126,8 @@ const DEFAULT_PASSTHROUGH_ENVS = [
   '*_TOKEN',
   // oxc specific
   'OXLINT_*',
+  // Rust specific
+  'RUST_*',
 ].map(env => new Minimatch(env));
 
 export function isPassThroughEnv(env: string) {