From 0f360f2f1b103c5f24264a632c27ffb6275b420c Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Sat, 21 Feb 2026 05:20:11 +0530 Subject: [PATCH] platform: use FILE_FLAG_WRITE_THROUGH on Windows for SyncFile data durability, fixes #9388 --- src/borg/platform/__init__.py | 2 +- src/borg/platform/base.py | 2 +- src/borg/platform/windows.pyx | 90 ++++++++++++++++++++ src/borg/testsuite/platform/platform_test.py | 1 + src/borg/testsuite/platform/windows_test.py | 72 ++++++++++++++++ 5 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 src/borg/testsuite/platform/windows_test.py diff --git a/src/borg/platform/__init__.py b/src/borg/platform/__init__.py index a4d0696c0..becb60f71 100644 --- a/src/borg/platform/__init__.py +++ b/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 diff --git a/src/borg/platform/base.py b/src/borg/platform/base.py index b9e2f50df..b6f2d0998 100644 --- a/src/borg/platform/base.py +++ b/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): diff --git a/src/borg/platform/windows.pyx b/src/borg/platform/windows.pyx index 714b8b952..c7a192a38 100644 --- a/src/borg/platform/windows.pyx +++ b/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() diff --git a/src/borg/testsuite/platform/platform_test.py b/src/borg/testsuite/platform/platform_test.py index 078bb2958..8372aad9c 100644 --- a/src/borg/testsuite/platform/platform_test.py +++ b/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") diff --git a/src/borg/testsuite/platform/windows_test.py b/src/borg/testsuite/platform/windows_test.py new file mode 100644 index 000000000..eeb6a4d0e --- /dev/null +++ b/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