fix(ecstore): allow Windows renames under guarded parents (#5663)

Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com>

.github/workflows/windows-filesystem.yml

@@ -0,0 +1,86 @@
+# Copyright 2024 RustFS Team
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: Windows Filesystem Tests
+
+on:
+  push:
+    branches: [ main ]
+    paths:
+      - "crates/ecstore/src/disk/**"
+      - "crates/ecstore/src/store/init_format.rs"
+      - "crates/ecstore/Cargo.toml"
+      - "Cargo.toml"
+      - "Cargo.lock"
+      - ".github/actions/setup/**"
+      - ".github/workflows/windows-filesystem.yml"
+  pull_request:
+    branches: [ main ]
+    paths:
+      - "crates/ecstore/src/disk/**"
+      - "crates/ecstore/src/store/init_format.rs"
+      - "crates/ecstore/Cargo.toml"
+      - "Cargo.toml"
+      - "Cargo.lock"
+      - ".github/actions/setup/**"
+      - ".github/workflows/windows-filesystem.yml"
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+env:
+  CARGO_TERM_COLOR: always
+  FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true"
+  RUST_BACKTRACE: 1
+
+jobs:
+  rename-safety:
+    name: Rename Safety
+    runs-on: windows-latest
+    timeout-minutes: 60
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
+        with:
+          persist-credentials: false
+
+      - name: Setup Rust environment
+        uses: ./.github/actions/setup
+        with:
+          rust-version: stable
+          cache-shared-key: build-x86_64-pc-windows-msvc
+          cache-save-if: 'false'
+          install-build-packaging-tools: 'false'
+          install-test-tools: 'false'
+
+      - name: Test guarded rename publication
+        shell: pwsh
+        run: cargo test -p rustfs-ecstore --lib rename_all_ -- --nocapture
+
+      - name: Test Windows handle guards
+        shell: pwsh
+        run: cargo test -p rustfs-ecstore --lib windows_ -- --nocapture
+
+      - name: Test startup temporary-directory cleanup
+        shell: pwsh
+        run: cargo test -p rustfs-ecstore --lib cleanup_tmp_on_startup_ -- --nocapture
+
+      - name: Test fresh format publication
+        shell: pwsh
+        run: cargo test -p rustfs-ecstore --lib fresh_format_load_initializes_all_disks -- --nocapture

crates/ecstore/src/disk/os.rs

@@ -677,21 +677,22 @@ pub(crate) type ExistingBaseDirectoryGuard = ();
 #[cfg(windows)]
 fn lock_windows_directory(path: &Path) -> io::Result<winapi_util::Handle> {
     use std::os::windows::fs::OpenOptionsExt;
+    use windows_sys::Win32::Storage::FileSystem::{
+        FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_REPARSE_POINT, FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT,
+        FILE_SHARE_READ, FILE_SHARE_WRITE,
+    };
 
-    const FILE_ATTRIBUTE_DIRECTORY: u64 = 0x10;
-    const FILE_ATTRIBUTE_REPARSE_POINT: u32 = 0x400;
-    const FILE_FLAG_BACKUP_SEMANTICS: u32 = 0x0200_0000;
-    const FILE_FLAG_OPEN_REPARSE_POINT: u32 = 0x0020_0000;
-    const FILE_SHARE_READ: u32 = 0x1;
-
+    // Relative child publication requires write sharing on every guarded
+    // ancestor. Omitting delete sharing still prevents any directory in the
+    // resolved path from being renamed or removed before the commit finishes.
     let file = std::fs::OpenOptions::new()
         .read(true)
-        .share_mode(FILE_SHARE_READ)
+        .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE)
         .custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT)
         .open(path)?;
     let handle = winapi_util::Handle::from_file(file);
     let info = winapi_util::file::information(&handle)?;
-    if info.file_attributes() & FILE_ATTRIBUTE_DIRECTORY == 0
+    if info.file_attributes() & u64::from(FILE_ATTRIBUTE_DIRECTORY) == 0
         || info.file_attributes() & u64::from(FILE_ATTRIBUTE_REPARSE_POINT) != 0
     {
         return Err(io::Error::from(io::ErrorKind::NotADirectory));
@@ -1223,23 +1224,53 @@ mod tests {
 
     #[cfg(windows)]
     #[test]
-    fn windows_parent_guard_blocks_base_and_intermediate_replacement() {
+    fn windows_parent_guard_blocks_parent_replacement() {
         let temp_dir = tempdir().expect("create temp dir");
         let base = temp_dir.path().join("bucket");
         std::fs::create_dir(&base).expect("create destination base");
         let parent = base.join("object").join("nested");
         let guard = mkdir_all_below_existing_base_std(&parent, &base).expect("create and lock destination parents");
 
+        std::fs::read_dir(&parent).expect("the locked parent must remain readable");
         std::fs::rename(&base, temp_dir.path().join("replacement-base"))
             .expect_err("the locked base must not be replaceable before commit");
         std::fs::rename(base.join("object"), base.join("replacement-object"))
             .expect_err("a locked intermediate directory must not be replaceable before commit");
+        std::fs::rename(&parent, base.join("replacement-parent"))
+            .expect_err("the locked destination parent must not be replaceable before commit");
+        assert!(parent.is_dir(), "failed replacement must leave the guarded parent in place");
 
         drop(guard);
         std::fs::rename(base.join("object"), base.join("replacement-object"))
             .expect("replacement should succeed after the commit guard is released");
     }
 
+    #[cfg(windows)]
+    #[tokio::test]
+    async fn windows_guarded_parent_allows_same_and_descendant_publication() {
+        let temp_dir = tempdir().expect("create temp dir");
+        let base = temp_dir.path().join("bucket");
+        let parent = base.join("object");
+        std::fs::create_dir_all(&parent).expect("create destination parent");
+        let _guard = mkdir_all_below_existing_base_std(&parent, &base).expect("guard destination parent");
+        let first_src = temp_dir.path().join("first-stage");
+        let second_src = temp_dir.path().join("second-stage");
+        std::fs::write(&first_src, b"first").expect("write first source");
+        std::fs::write(&second_src, b"second").expect("write second source");
+
+        rename_all(&first_src, parent.join("first"), &base)
+            .await
+            .expect("same-parent rename must succeed while a guard is held");
+        rename_all(&second_src, parent.join("nested").join("second"), &base)
+            .await
+            .expect("descendant-parent rename must succeed while an ancestor guard is held");
+        assert_eq!(std::fs::read(parent.join("first")).expect("read first destination"), b"first");
+        assert_eq!(
+            std::fs::read(parent.join("nested").join("second")).expect("read second destination"),
+            b"second"
+        );
+    }
+
     #[cfg(unix)]
     #[tokio::test]
     async fn rename_parent_creation_rejects_symlinked_base() {

crates/ecstore/src/store/init_format.rs

@@ -1279,6 +1279,22 @@ mod tests {
         assert!(disks[2].is_none(), "the malformed outlier must be isolated");
     }
 
+    #[tokio::test]
+    async fn fresh_format_load_initializes_all_disks() {
+        let (_temp_dir, mut disks) = local_disks(3).await;
+
+        let format = connect_load_init_formats(true, &mut disks, 1, 3, None)
+            .await
+            .expect("fresh disks should receive a storage format");
+
+        let (formats, errors) = load_format_erasure_all(&disks, false).await;
+        assert!(errors.iter().all(Option::is_none), "every disk should load its fresh format: {errors:?}");
+        assert!(
+            formats_match_reference_slots(&formats, &format, 0),
+            "fresh format publication must preserve every disk slot"
+        );
+    }
+
     #[tokio::test]
     async fn fresh_format_load_does_not_initialize_with_a_missing_disk() {
         let (_temp_dir, mut disks) = two_local_disks_with_missing_third().await;