Merge pull request #9389 from mr-raj12/win32-syncfile-write-through

platform: use FILE_FLAG_WRITE_THROUGH on Windows for SyncFile data durability, fixes #9388

src/borg/platform/__init__.py

@@ -72,7 +72,7 @@ else:  # pragma: win32 only
     from .base import listxattr, getxattr, setxattr
     from .base import acl_get, acl_set
     from .base import set_flags, get_flags
-    from .base import SyncFile
+    from .windows import SyncFile
     from .windows import process_alive, local_pid_alive
     from .windows import getosusername
     from . import windows_ug as platform_ug

src/borg/platform/base.py

@@ -152,7 +152,7 @@ class SyncFile:
 
     Calling SyncFile(path) for an existing path will raise FileExistsError. See the comment in __init__.
 
-    TODO: A Windows implementation should use CreateFile with FILE_FLAG_WRITE_THROUGH.
+    See platform/windows.pyx for the Windows implementation using CreateFile with FILE_FLAG_WRITE_THROUGH.
     """
 
     def __init__(self, path, *, fd=None, binary=False):

src/borg/platform/windows.pyx

@@ -1,6 +1,12 @@
+import ctypes
+import ctypes.wintypes
+import errno as errno_mod
+import msvcrt
 import os
 import platform
 
+from .base import SyncFile as BaseSyncFile
+
 
 cdef extern from 'windows.h':
     ctypedef void* HANDLE
@@ -13,6 +19,90 @@ cdef extern from 'windows.h':
     cdef extern int PROCESS_QUERY_INFORMATION
 
 
+# Win32 API constants for CreateFileW
+GENERIC_READ = 0x80000000
+GENERIC_WRITE = 0x40000000
+FILE_SHARE_READ = 0x00000001
+CREATE_NEW = 1
+FILE_ATTRIBUTE_NORMAL = 0x80
+FILE_FLAG_WRITE_THROUGH = 0x80000000
+ERROR_FILE_EXISTS = 80
+
+_kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
+_CreateFileW = _kernel32.CreateFileW
+_CreateFileW.restype = ctypes.wintypes.HANDLE
+_CreateFileW.argtypes = [
+    ctypes.wintypes.LPCWSTR,
+    ctypes.wintypes.DWORD,
+    ctypes.wintypes.DWORD,
+    ctypes.c_void_p,
+    ctypes.wintypes.DWORD,
+    ctypes.wintypes.DWORD,
+    ctypes.wintypes.HANDLE,
+]
+_CloseHandle = _kernel32.CloseHandle
+INVALID_HANDLE_VALUE = ctypes.wintypes.HANDLE(-1).value
+
+
+class SyncFile(BaseSyncFile):
+    """
+    Windows SyncFile using FILE_FLAG_WRITE_THROUGH for data durability.
+
+    FILE_FLAG_WRITE_THROUGH instructs Windows to write through any intermediate
+    cache and go directly to disk, providing data durability guarantees similar
+    to fdatasync/F_FULLFSYNC on POSIX/macOS systems.
+
+    When an already-open fd is provided, falls back to base implementation.
+    """
+
+    def __init__(self, path, *, fd=None, binary=False):
+        if fd is not None:
+            # An already-opened fd was provided (e.g., from SaveFile via mkstemp).
+            # We cannot change its flags, so fall back to the base implementation.
+            super().__init__(path, fd=fd, binary=binary)
+            return
+
+        self.path = path
+        handle = _CreateFileW(
+            str(path),
+            GENERIC_READ | GENERIC_WRITE,
+            FILE_SHARE_READ,
+            None,
+            CREATE_NEW,  # fail if file exists, matching Python's 'x' mode
+            FILE_FLAG_WRITE_THROUGH | FILE_ATTRIBUTE_NORMAL,
+            None,
+        )
+        if handle == INVALID_HANDLE_VALUE:
+            error = ctypes.get_last_error()
+            if error == ERROR_FILE_EXISTS:
+                raise FileExistsError(errno_mod.EEXIST, os.strerror(errno_mod.EEXIST), str(path))
+            raise ctypes.WinError(error)
+
+        try:
+            oflags = os.O_BINARY if binary else os.O_TEXT
+            c_fd = msvcrt.open_osfhandle(handle, oflags)
+        except Exception:
+            _CloseHandle(handle)
+            raise
+
+        try:
+            mode = "r+b" if binary else "r+"
+            self.f = os.fdopen(c_fd, mode=mode)
+        except Exception:
+            os.close(c_fd)  # Also closes the underlying Windows handle
+            raise
+        self.fd = self.f.fileno()
+
+    def sync(self):
+        """Flush and sync to persistent storage.
+
+        With FILE_FLAG_WRITE_THROUGH, writes already go to stable storage.
+        We still call os.fsync (FlushFileBuffers) for belt-and-suspenders safety.
+        """
+        self.f.flush()
+        os.fsync(self.fd)
+
+
 def getosusername():
     """Return the OS username."""
     return os.getlogin()

src/borg/testsuite/platform/platform_test.py

@@ -74,6 +74,7 @@ skipif_not_freebsd = pytest.mark.skipif(not is_freebsd, reason="FreeBSD-only tes
 skipif_not_posix = pytest.mark.skipif(not (is_linux or is_freebsd or is_darwin), reason="POSIX-only tests")
 skipif_fakeroot_detected = pytest.mark.skipif(fakeroot_detected(), reason="not compatible with fakeroot")
 skipif_acls_not_working = pytest.mark.skipif(not are_acls_working(), reason="ACLs do not work")
+skipif_not_win32 = pytest.mark.skipif(not is_win32, reason="Windows-only test")
 skipif_no_ubel_user = pytest.mark.skipif(not user_exists("übel"), reason="requires übel user")
 
 

src/borg/testsuite/platform/windows_test.py

@@ -0,0 +1,72 @@
+import tempfile
+
+import pytest
+
+from .platform_test import skipif_not_win32
+from ...platform import SyncFile
+
+# Set module-level skips
+pytestmark = [skipif_not_win32]
+
+
+def test_syncfile_basic(tmp_path):
+    """Integration: SyncFile creates file and writes data correctly."""
+    path = tmp_path / "testfile"
+    with SyncFile(path, binary=True) as sf:
+        sf.write(b"hello borg")
+    assert path.read_bytes() == b"hello borg"
+
+
+def test_syncfile_file_exists_error(tmp_path):
+    """SyncFile raises FileExistsError if file already exists."""
+    path = tmp_path / "testfile"
+    path.touch()
+    with pytest.raises(FileExistsError):
+        SyncFile(path, binary=True)
+
+
+def test_syncfile_text_mode(tmp_path):
+    """SyncFile works in text mode."""
+    path = tmp_path / "testfile.txt"
+    with SyncFile(path) as sf:
+        sf.write("hello text")
+    assert path.read_text() == "hello text"
+
+
+def test_syncfile_fd_fallback(tmp_path):
+    """SyncFile with fd falls back to base implementation (mirrors SaveFile usage)."""
+    fd, fpath = tempfile.mkstemp(dir=tmp_path)
+    with SyncFile(fpath, fd=fd, binary=True) as sf:
+        sf.write(b"fallback test")
+    with open(fpath, "rb") as f:
+        assert f.read() == b"fallback test"
+
+
+def test_syncfile_sync(tmp_path):
+    """Explicit sync() does not raise."""
+    path = tmp_path / "testfile"
+    with SyncFile(path, binary=True) as sf:
+        sf.write(b"sync test data")
+        sf.sync()
+
+
+def test_syncfile_uses_write_through(tmp_path, monkeypatch):
+    """Verify CreateFileW is called with FILE_FLAG_WRITE_THROUGH."""
+    from borg.platform import windows
+
+    calls = []
+    original = windows._CreateFileW
+
+    def mock_create(*args):
+        calls.append(args)
+        return original(*args)
+
+    monkeypatch.setattr(windows, "_CreateFileW", mock_create)
+
+    path = tmp_path / "testfile"
+    with windows.SyncFile(path, binary=True) as sf:
+        sf.write(b"write-through test")
+
+    assert len(calls) == 1
+    flags_attrs = calls[0][5]  # 6th arg: dwFlagsAndAttributes
+    assert flags_attrs & windows.FILE_FLAG_WRITE_THROUGH