fs: Fix no-overwrite rename races (#51090)

Smit Barmase created

Closes #46661

This PR changes `fs.rename` to use the platform’s atomic no-overwrite
rename on all platforms when `overwrite` is `false`. This fixes a case
where concurrent renames to the same target could race past a separate
metadata check and end up overwriting each other.

In Project Panel, we can still rename entries in parallel without
worrying about OS internals not handling it correctly or making these
renames sequential.

Release Notes:

- Fixed an issue in the Project Panel where conflicting file moves could
overwrite each other instead of leaving the losing file in place.

Change summary

crates/fs/src/fs.rs               | 118 +++++++++++++++++++++++++++++++-
crates/fs/tests/integration/fs.rs |  59 ++++++++++++++++
2 files changed, 170 insertions(+), 7 deletions(-)

Detailed changes

crates/fs/src/fs.rs 🔗

@@ -15,10 +15,14 @@ use gpui::Global;
 use gpui::ReadGlobal as _;
 use gpui::SharedString;
 use std::borrow::Cow;
+#[cfg(unix)]
+use std::ffi::CString;
 use util::command::new_command;
 
 #[cfg(unix)]
 use std::os::fd::{AsFd, AsRawFd};
+#[cfg(unix)]
+use std::os::unix::ffi::OsStrExt;
 
 #[cfg(unix)]
 use std::os::unix::fs::{FileTypeExt, MetadataExt};
@@ -506,6 +510,63 @@ impl RealFs {
     }
 }
 
+#[cfg(any(target_os = "macos", target_os = "linux"))]
+fn rename_without_replace(source: &Path, target: &Path) -> io::Result<()> {
+    let source = path_to_c_string(source)?;
+    let target = path_to_c_string(target)?;
+
+    #[cfg(target_os = "macos")]
+    let result = unsafe { libc::renamex_np(source.as_ptr(), target.as_ptr(), libc::RENAME_EXCL) };
+
+    #[cfg(target_os = "linux")]
+    let result = unsafe {
+        libc::syscall(
+            libc::SYS_renameat2,
+            libc::AT_FDCWD,
+            source.as_ptr(),
+            libc::AT_FDCWD,
+            target.as_ptr(),
+            libc::RENAME_NOREPLACE,
+        )
+    };
+
+    if result == 0 {
+        Ok(())
+    } else {
+        Err(io::Error::last_os_error())
+    }
+}
+
+#[cfg(target_os = "windows")]
+fn rename_without_replace(source: &Path, target: &Path) -> io::Result<()> {
+    use std::os::windows::ffi::OsStrExt;
+
+    use windows::Win32::Storage::FileSystem::{MOVE_FILE_FLAGS, MoveFileExW};
+    use windows::core::PCWSTR;
+
+    let source: Vec<u16> = source.as_os_str().encode_wide().chain(Some(0)).collect();
+    let target: Vec<u16> = target.as_os_str().encode_wide().chain(Some(0)).collect();
+
+    unsafe {
+        MoveFileExW(
+            PCWSTR(source.as_ptr()),
+            PCWSTR(target.as_ptr()),
+            MOVE_FILE_FLAGS::default(),
+        )
+    }
+    .map_err(|_| io::Error::last_os_error())
+}
+
+#[cfg(any(target_os = "macos", target_os = "linux"))]
+fn path_to_c_string(path: &Path) -> io::Result<CString> {
+    CString::new(path.as_os_str().as_bytes()).map_err(|_| {
+        io::Error::new(
+            io::ErrorKind::InvalidInput,
+            format!("path contains interior NUL: {}", path.display()),
+        )
+    })
+}
+
 #[async_trait::async_trait]
 impl Fs for RealFs {
     async fn create_dir(&self, path: &Path) -> Result<()> {
@@ -588,7 +649,56 @@ impl Fs for RealFs {
     }
 
     async fn rename(&self, source: &Path, target: &Path, options: RenameOptions) -> Result<()> {
-        if !options.overwrite && smol::fs::metadata(target).await.is_ok() {
+        if options.create_parents {
+            if let Some(parent) = target.parent() {
+                self.create_dir(parent).await?;
+            }
+        }
+
+        if options.overwrite {
+            smol::fs::rename(source, target).await?;
+            return Ok(());
+        }
+
+        let use_metadata_fallback = {
+            #[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))]
+            {
+                let source = source.to_path_buf();
+                let target = target.to_path_buf();
+                match self
+                    .executor
+                    .spawn(async move { rename_without_replace(&source, &target) })
+                    .await
+                {
+                    Ok(()) => return Ok(()),
+                    Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {
+                        if options.ignore_if_exists {
+                            return Ok(());
+                        }
+                        return Err(error.into());
+                    }
+                    Err(error)
+                        if error.raw_os_error().is_some_and(|code| {
+                            code == libc::ENOSYS
+                                || code == libc::ENOTSUP
+                                || code == libc::EOPNOTSUPP
+                        }) =>
+                    {
+                        // For case when filesystem or kernel does not support atomic no-overwrite rename.
+                        true
+                    }
+                    Err(error) => return Err(error.into()),
+                }
+            }
+
+            #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))]
+            {
+                // For platforms which do not have an atomic no-overwrite rename yet.
+                true
+            }
+        };
+
+        if use_metadata_fallback && smol::fs::metadata(target).await.is_ok() {
             if options.ignore_if_exists {
                 return Ok(());
             } else {
@@ -596,12 +706,6 @@ impl Fs for RealFs {
             }
         }
 
-        if options.create_parents {
-            if let Some(parent) = target.parent() {
-                self.create_dir(parent).await?;
-            }
-        }
-
         smol::fs::rename(source, target).await?;
         Ok(())
     }

crates/fs/tests/integration/fs.rs 🔗

@@ -523,6 +523,65 @@ async fn test_rename(executor: BackgroundExecutor) {
     );
 }
 
+#[gpui::test]
+#[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))]
+async fn test_realfs_parallel_rename_without_overwrite_preserves_losing_source(
+    executor: BackgroundExecutor,
+) {
+    let temp_dir = TempDir::new().unwrap();
+    let root = temp_dir.path();
+    let source_a = root.join("dir_a/shared.txt");
+    let source_b = root.join("dir_b/shared.txt");
+    let target = root.join("shared.txt");
+
+    std::fs::create_dir_all(source_a.parent().unwrap()).unwrap();
+    std::fs::create_dir_all(source_b.parent().unwrap()).unwrap();
+    std::fs::write(&source_a, "from a").unwrap();
+    std::fs::write(&source_b, "from b").unwrap();
+
+    let fs = RealFs::new(None, executor);
+    let (first_result, second_result) = futures::future::join(
+        fs.rename(&source_a, &target, RenameOptions::default()),
+        fs.rename(&source_b, &target, RenameOptions::default()),
+    )
+    .await;
+
+    assert_ne!(first_result.is_ok(), second_result.is_ok());
+    assert!(target.exists());
+    assert_eq!(source_a.exists() as u8 + source_b.exists() as u8, 1);
+}
+
+#[gpui::test]
+#[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))]
+async fn test_realfs_rename_ignore_if_exists_leaves_source_and_target_unchanged(
+    executor: BackgroundExecutor,
+) {
+    let temp_dir = TempDir::new().unwrap();
+    let root = temp_dir.path();
+    let source = root.join("source.txt");
+    let target = root.join("target.txt");
+
+    std::fs::write(&source, "from source").unwrap();
+    std::fs::write(&target, "from target").unwrap();
+
+    let fs = RealFs::new(None, executor);
+    let result = fs
+        .rename(
+            &source,
+            &target,
+            RenameOptions {
+                ignore_if_exists: true,
+                ..Default::default()
+            },
+        )
+        .await;
+
+    assert!(result.is_ok());
+
+    assert_eq!(std::fs::read_to_string(&source).unwrap(), "from source");
+    assert_eq!(std::fs::read_to_string(&target).unwrap(), "from target");
+}
+
 #[gpui::test]
 #[cfg(unix)]
 async fn test_realfs_broken_symlink_metadata(executor: BackgroundExecutor) {