windows: Fix atomic write (#30234)

Superseded #30222

On Windows, `MoveFileExW` fails if another process is holding a handle
to the file. This PR fixes that issue by switching to `ReplaceFileW`
instead.

I’ve also added corresponding tests.

According to [this Microsoft research
paper](https://www.microsoft.com/en-us/research/wp-content/uploads/2006/04/tr-2006-45.pdf)
and the [official
documentation](https://learn.microsoft.com/en-us/windows/win32/fileio/deprecation-of-txf#applications-updating-a-single-file-with-document-like-data),
`ReplaceFileW` is considered an atomic operation. even though the
official docs don’t explicitly state whether `MoveFileExW` or
`ReplaceFileW` is guaranteed to be atomic.

Release Notes:

- N/A

crates/fs/src/fs.rs

@@ -33,7 +33,7 @@ use std::{
     sync::Arc,
     time::{Duration, SystemTime, UNIX_EPOCH},
 };
-use tempfile::{NamedTempFile, TempDir};
+use tempfile::TempDir;
 use text::LineEnding;
 
 #[cfg(any(test, feature = "test-support"))]
@@ -525,45 +525,19 @@ impl Fs for RealFs {
         Ok(bytes)
     }
 
+    #[cfg(not(target_os = "windows"))]
     async fn atomic_write(&self, path: PathBuf, data: String) -> Result<()> {
         smol::unblock(move || {
             let mut tmp_file = if cfg!(any(target_os = "linux", target_os = "freebsd")) {
                 // Use the directory of the destination as temp dir to avoid
                 // invalid cross-device link error, and XDG_CACHE_DIR for fallback.
                 // See https://github.com/zed-industries/zed/pull/8437 for more details.
-                NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir()))
-            } else if cfg!(target_os = "windows") {
-                // If temp dir is set to a different drive than the destination,
-                // we receive error:
-                //
-                // failed to persist temporary file:
-                // The system cannot move the file to a different disk drive. (os error 17)
-                //
-                // So we use the directory of the destination as a temp dir to avoid it.
-                // https://github.com/zed-industries/zed/issues/16571
-                NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir()))
+                tempfile::NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir()))
             } else {
-                NamedTempFile::new()
+                tempfile::NamedTempFile::new()
             }?;
             tmp_file.write_all(data.as_bytes())?;
-
-            let result = tmp_file.persist(&path);
-            if cfg!(target_os = "windows") {
-                // If file handle is already in used we receive error:
-                //
-                // failed to persist temporary file:
-                // Access is denied. (os error 5)
-                //
-                // So we use direct fs write instead to avoid it.
-                // https://github.com/zed-industries/zed/issues/30054
-                if let Err(persist_err) = &result {
-                    if persist_err.error.raw_os_error() == Some(5) {
-                        return std::fs::write(&path, data.as_bytes()).map_err(Into::into);
-                    }
-                }
-            }
-            result?;
-
+            tmp_file.persist(path)?;
             Ok::<(), anyhow::Error>(())
         })
         .await?;
@@ -571,6 +545,35 @@ impl Fs for RealFs {
         Ok(())
     }
 
+    #[cfg(target_os = "windows")]
+    async fn atomic_write(&self, path: PathBuf, data: String) -> Result<()> {
+        smol::unblock(move || {
+            // If temp dir is set to a different drive than the destination,
+            // we receive error:
+            //
+            // failed to persist temporary file:
+            // The system cannot move the file to a different disk drive. (os error 17)
+            //
+            // This is because `ReplaceFileW` does not support cross volume moves.
+            // See the remark section: "The backup file, replaced file, and replacement file must all reside on the same volume."
+            // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-replacefilew#remarks
+            //
+            // So we use the directory of the destination as a temp dir to avoid it.
+            // https://github.com/zed-industries/zed/issues/16571
+            let temp_dir = TempDir::new_in(path.parent().unwrap_or(paths::temp_dir()))?;
+            let temp_file = {
+                let temp_file_path = temp_dir.path().join("temp_file");
+                let mut file = std::fs::File::create_new(&temp_file_path)?;
+                file.write_all(data.as_bytes())?;
+                temp_file_path
+            };
+            atomic_replace(path.as_path(), temp_file.as_path())?;
+            Ok::<(), anyhow::Error>(())
+        })
+        .await?;
+        Ok(())
+    }
+
     async fn save(&self, path: &Path, text: &Rope, line_ending: LineEnding) -> Result<()> {
         let buffer_size = text.summary().len.min(10 * 1024);
         if let Some(path) = path.parent() {
@@ -2486,6 +2489,31 @@ async fn file_id(path: impl AsRef<Path>) -> Result<u64> {
     .await
 }
 
+#[cfg(target_os = "windows")]
+fn atomic_replace<P: AsRef<Path>>(
+    replaced_file: P,
+    replacement_file: P,
+) -> windows::core::Result<()> {
+    use windows::{
+        Win32::Storage::FileSystem::{REPLACE_FILE_FLAGS, ReplaceFileW},
+        core::HSTRING,
+    };
+
+    // If the file does not exist, create it.
+    let _ = std::fs::File::create_new(replaced_file.as_ref());
+
+    unsafe {
+        ReplaceFileW(
+            &HSTRING::from(replaced_file.as_ref().to_string_lossy().to_string()),
+            &HSTRING::from(replacement_file.as_ref().to_string_lossy().to_string()),
+            None,
+            REPLACE_FILE_FLAGS::default(),
+            None,
+            None,
+        )
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -2905,4 +2933,37 @@ mod tests {
             "B"
         );
     }
+
+    #[gpui::test]
+    async fn test_realfs_atomic_write(executor: BackgroundExecutor) {
+        // With the file handle still open, the file should be replaced
+        // https://github.com/zed-industries/zed/issues/30054
+        let fs = RealFs {
+            git_binary_path: None,
+            executor,
+        };
+        let temp_dir = TempDir::new().unwrap();
+        let file_to_be_replaced = temp_dir.path().join("file.txt");
+        let mut file = std::fs::File::create_new(&file_to_be_replaced).unwrap();
+        file.write_all(b"Hello").unwrap();
+        // drop(file);  // We still hold the file handle here
+        let content = std::fs::read_to_string(&file_to_be_replaced).unwrap();
+        assert_eq!(content, "Hello");
+        smol::block_on(fs.atomic_write(file_to_be_replaced.clone(), "World".into())).unwrap();
+        let content = std::fs::read_to_string(&file_to_be_replaced).unwrap();
+        assert_eq!(content, "World");
+    }
+
+    #[gpui::test]
+    async fn test_realfs_atomic_write_non_existing_file(executor: BackgroundExecutor) {
+        let fs = RealFs {
+            git_binary_path: None,
+            executor,
+        };
+        let temp_dir = TempDir::new().unwrap();
+        let file_to_be_replaced = temp_dir.path().join("file.txt");
+        smol::block_on(fs.atomic_write(file_to_be_replaced.clone(), "Hello".into())).unwrap();
+        let content = std::fs::read_to_string(&file_to_be_replaced).unwrap();
+        assert_eq!(content, "Hello");
+    }
 }