fix: handle Windows PermissionError on directory fsync
On Windows, opening a directory with O_RDONLY raises PermissionError. Wrap the directory fsync in a try/except PermissionError — NTFS journals metadata synchronously so the directory sync is unnecessary there. Also adjust test assertions to expect 1 fsync call (file only) on Windows vs 2 (file + directory) on POSIX.
This commit is contained in:
@@ -296,11 +296,17 @@ class SessionManager:
|
|||||||
|
|
||||||
if fsync:
|
if fsync:
|
||||||
# fsync the directory so the rename is durable.
|
# fsync the directory so the rename is durable.
|
||||||
fd = os.open(str(path.parent), os.O_RDONLY)
|
# On Windows, opening a directory with O_RDONLY raises
|
||||||
|
# PermissionError — skip the dir sync there (NTFS
|
||||||
|
# journals metadata synchronously).
|
||||||
try:
|
try:
|
||||||
os.fsync(fd)
|
fd = os.open(str(path.parent), os.O_RDONLY)
|
||||||
finally:
|
try:
|
||||||
os.close(fd)
|
os.fsync(fd)
|
||||||
|
finally:
|
||||||
|
os.close(fd)
|
||||||
|
except PermissionError:
|
||||||
|
pass # Windows — directory fsync not supported
|
||||||
except BaseException:
|
except BaseException:
|
||||||
tmp_path.unlink(missing_ok=True)
|
tmp_path.unlink(missing_ok=True)
|
||||||
raise
|
raise
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import sys
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
@@ -9,6 +10,8 @@ import pytest
|
|||||||
|
|
||||||
from nanobot.session.manager import SessionManager
|
from nanobot.session.manager import SessionManager
|
||||||
|
|
||||||
|
_IS_WINDOWS = sys.platform == "win32"
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def sessions_dir(tmp_path: Path) -> Path:
|
def sessions_dir(tmp_path: Path) -> Path:
|
||||||
@@ -39,8 +42,9 @@ class TestSaveFsync:
|
|||||||
|
|
||||||
with patch("os.fsync") as mock_fsync:
|
with patch("os.fsync") as mock_fsync:
|
||||||
manager.save(session, fsync=True)
|
manager.save(session, fsync=True)
|
||||||
# Should be called twice: once for the file, once for the directory
|
# File fsync always runs; directory fsync only on non-Windows.
|
||||||
assert mock_fsync.call_count == 2
|
expected = 1 if _IS_WINDOWS else 2
|
||||||
|
assert mock_fsync.call_count == expected
|
||||||
|
|
||||||
def test_save_default_no_fsync(self, manager: SessionManager):
|
def test_save_default_no_fsync(self, manager: SessionManager):
|
||||||
"""Default save() should not fsync (backward compat)."""
|
"""Default save() should not fsync (backward compat)."""
|
||||||
@@ -77,8 +81,9 @@ class TestFlushAll:
|
|||||||
|
|
||||||
with patch("os.fsync") as mock_fsync:
|
with patch("os.fsync") as mock_fsync:
|
||||||
manager.flush_all()
|
manager.flush_all()
|
||||||
# file fsync + directory fsync
|
# file fsync always; directory fsync only on non-Windows
|
||||||
assert mock_fsync.call_count == 2
|
expected = 1 if _IS_WINDOWS else 2
|
||||||
|
assert mock_fsync.call_count == expected
|
||||||
|
|
||||||
def test_flush_all_continues_on_error(self, manager: SessionManager):
|
def test_flush_all_continues_on_error(self, manager: SessionManager):
|
||||||
"""One broken session should not prevent others from flushing."""
|
"""One broken session should not prevent others from flushing."""
|
||||||
|
|||||||
Reference in New Issue
Block a user