Skip to content

Commit 59dcf0e

Browse files
committed
platform: use FILE_FLAG_WRITE_THROUGH on Windows for SyncFile data durability, fixes #9388
1 parent 9f89e16 commit 59dcf0e

File tree

5 files changed

+182
-2
lines changed

5 files changed

+182
-2
lines changed

src/borg/platform/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
from .base import listxattr, getxattr, setxattr
7373
from .base import acl_get, acl_set
7474
from .base import set_flags, get_flags
75-
from .base import SyncFile
75+
from .windows import SyncFile
7676
from .windows import process_alive, local_pid_alive
7777
from .windows import getosusername
7878
from . import windows_ug as platform_ug

src/borg/platform/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class SyncFile:
151151
152152
Calling SyncFile(path) for an existing path will raise FileExistsError. See the comment in __init__.
153153
154-
TODO: A Windows implementation should use CreateFile with FILE_FLAG_WRITE_THROUGH.
154+
See platform/windows.pyx for the Windows implementation using CreateFile with FILE_FLAG_WRITE_THROUGH.
155155
"""
156156

157157
def __init__(self, path, *, fd=None, binary=False):

src/borg/platform/windows.pyx

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1+
import ctypes
2+
import ctypes.wintypes
3+
import errno as errno_mod
4+
import msvcrt
15
import os
26
import platform
37

8+
from .base import SyncFile as BaseSyncFile
9+
410

511
cdef extern from 'windows.h':
612
ctypedef void* HANDLE
@@ -13,6 +19,89 @@ cdef extern from 'windows.h':
1319
cdef extern int PROCESS_QUERY_INFORMATION
1420

1521

22+
# Win32 API constants for CreateFileW
23+
GENERIC_WRITE = 0x40000000
24+
FILE_SHARE_READ = 0x00000001
25+
CREATE_NEW = 1
26+
FILE_ATTRIBUTE_NORMAL = 0x80
27+
FILE_FLAG_WRITE_THROUGH = 0x80000000
28+
ERROR_FILE_EXISTS = 80
29+
30+
_kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
31+
_CreateFileW = _kernel32.CreateFileW
32+
_CreateFileW.restype = ctypes.wintypes.HANDLE
33+
_CreateFileW.argtypes = [
34+
ctypes.wintypes.LPCWSTR,
35+
ctypes.wintypes.DWORD,
36+
ctypes.wintypes.DWORD,
37+
ctypes.c_void_p,
38+
ctypes.wintypes.DWORD,
39+
ctypes.wintypes.DWORD,
40+
ctypes.wintypes.HANDLE,
41+
]
42+
_CloseHandle = _kernel32.CloseHandle
43+
INVALID_HANDLE_VALUE = ctypes.wintypes.HANDLE(-1).value
44+
45+
46+
class SyncFile(BaseSyncFile):
47+
"""
48+
Windows SyncFile using FILE_FLAG_WRITE_THROUGH for data durability.
49+
50+
FILE_FLAG_WRITE_THROUGH instructs Windows to write through any intermediate
51+
cache and go directly to disk, providing data durability guarantees similar
52+
to fdatasync/F_FULLFSYNC on POSIX/macOS systems.
53+
54+
When an already-open fd is provided, falls back to base implementation.
55+
"""
56+
57+
def __init__(self, path, *, fd=None, binary=False):
58+
if fd is not None:
59+
# An already-opened fd was provided (e.g., from SaveFile via mkstemp).
60+
# We cannot change its flags, so fall back to the base implementation.
61+
super().__init__(path, fd=fd, binary=binary)
62+
return
63+
64+
self.path = path
65+
handle = _CreateFileW(
66+
str(path),
67+
GENERIC_WRITE,
68+
FILE_SHARE_READ,
69+
None,
70+
CREATE_NEW, # fail if file exists, matching Python's 'x' mode
71+
FILE_FLAG_WRITE_THROUGH | FILE_ATTRIBUTE_NORMAL,
72+
None,
73+
)
74+
if handle == INVALID_HANDLE_VALUE:
75+
error = ctypes.get_last_error()
76+
if error == ERROR_FILE_EXISTS:
77+
raise FileExistsError(errno_mod.EEXIST, os.strerror(errno_mod.EEXIST), str(path))
78+
raise ctypes.WinError(error)
79+
80+
try:
81+
oflags = os.O_WRONLY | (os.O_BINARY if binary else os.O_TEXT)
82+
c_fd = msvcrt.open_osfhandle(handle, oflags)
83+
except Exception:
84+
_CloseHandle(handle)
85+
raise
86+
87+
try:
88+
mode = "wb" if binary else "w"
89+
self.f = os.fdopen(c_fd, mode=mode)
90+
except Exception:
91+
os.close(c_fd) # Also closes the underlying Windows handle
92+
raise
93+
self.fd = self.f.fileno()
94+
95+
def sync(self):
96+
"""Flush and sync to persistent storage.
97+
98+
With FILE_FLAG_WRITE_THROUGH, writes already go to stable storage.
99+
We still call os.fsync (FlushFileBuffers) for belt-and-suspenders safety.
100+
"""
101+
self.f.flush()
102+
os.fsync(self.fd)
103+
104+
16105
def getosusername():
17106
"""Return the OS username."""
18107
return os.getlogin()

src/borg/testsuite/platform/platform_test.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ def are_acls_working():
7474
skipif_not_posix = pytest.mark.skipif(not (is_linux or is_freebsd or is_darwin), reason="POSIX-only tests")
7575
skipif_fakeroot_detected = pytest.mark.skipif(fakeroot_detected(), reason="not compatible with fakeroot")
7676
skipif_acls_not_working = pytest.mark.skipif(not are_acls_working(), reason="ACLs do not work")
77+
skipif_not_win32 = pytest.mark.skipif(not is_win32, reason="Windows-only test")
7778
skipif_no_ubel_user = pytest.mark.skipif(not user_exists("übel"), reason="requires übel user")
7879

7980

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import os
2+
import tempfile
3+
4+
import pytest
5+
6+
from .platform_test import skipif_not_win32
7+
8+
# Set module-level skips
9+
pytestmark = [skipif_not_win32]
10+
11+
12+
def test_syncfile_basic():
13+
"""Integration: SyncFile creates file and writes data correctly."""
14+
from ...platform.windows import SyncFile
15+
16+
with tempfile.TemporaryDirectory() as tmpdir:
17+
path = os.path.join(tmpdir, "testfile")
18+
with SyncFile(path, binary=True) as sf:
19+
sf.write(b"hello borg")
20+
with open(path, "rb") as f:
21+
assert f.read() == b"hello borg"
22+
23+
24+
def test_syncfile_file_exists_error():
25+
"""SyncFile raises FileExistsError if file already exists."""
26+
from ...platform.windows import SyncFile
27+
28+
with tempfile.TemporaryDirectory() as tmpdir:
29+
path = os.path.join(tmpdir, "testfile")
30+
open(path, "w").close()
31+
with pytest.raises(FileExistsError):
32+
SyncFile(path, binary=True)
33+
34+
35+
def test_syncfile_text_mode():
36+
"""SyncFile works in text mode."""
37+
from ...platform.windows import SyncFile
38+
39+
with tempfile.TemporaryDirectory() as tmpdir:
40+
path = os.path.join(tmpdir, "testfile.txt")
41+
with SyncFile(path) as sf:
42+
sf.write("hello text")
43+
with open(path, "r") as f:
44+
assert f.read() == "hello text"
45+
46+
47+
def test_syncfile_fd_fallback():
48+
"""SyncFile with fd falls back to base implementation (mirrors SaveFile usage)."""
49+
from ...platform.windows import SyncFile
50+
51+
with tempfile.TemporaryDirectory() as tmpdir:
52+
fd, path = tempfile.mkstemp(dir=tmpdir)
53+
with SyncFile(path, fd=fd, binary=True) as sf:
54+
sf.write(b"fallback test")
55+
with open(path, "rb") as f:
56+
assert f.read() == b"fallback test"
57+
58+
59+
def test_syncfile_sync():
60+
"""Explicit sync() does not raise."""
61+
from ...platform.windows import SyncFile
62+
63+
with tempfile.TemporaryDirectory() as tmpdir:
64+
path = os.path.join(tmpdir, "testfile")
65+
with SyncFile(path, binary=True) as sf:
66+
sf.write(b"sync test data")
67+
sf.sync()
68+
69+
70+
def test_syncfile_uses_write_through(monkeypatch):
71+
"""Verify CreateFileW is called with FILE_FLAG_WRITE_THROUGH."""
72+
from ...platform import windows
73+
74+
calls = []
75+
original = windows._CreateFileW
76+
77+
def mock_create(*args):
78+
calls.append(args)
79+
return original(*args)
80+
81+
monkeypatch.setattr(windows, "_CreateFileW", mock_create)
82+
83+
with tempfile.TemporaryDirectory() as tmpdir:
84+
path = os.path.join(tmpdir, "testfile")
85+
with windows.SyncFile(path, binary=True) as sf:
86+
sf.write(b"write-through test")
87+
88+
assert len(calls) == 1
89+
flags_attrs = calls[0][5] # 6th arg: dwFlagsAndAttributes
90+
assert flags_attrs & windows.FILE_FLAG_WRITE_THROUGH

0 commit comments

Comments
 (0)