diff --git a/obliteratus/persistence_contracts.py b/obliteratus/persistence_contracts.py index a4e691c..9ff91e4 100644 --- a/obliteratus/persistence_contracts.py +++ b/obliteratus/persistence_contracts.py @@ -23,6 +23,16 @@ else: _DIRECTORY_SYNC_FLAGS = os.O_RDONLY | os.O_DIRECTORY +def _file_sync_flags(platform_name: str, binary_flag: int) -> int: + """Return flags for a descriptor that the platform can durably flush.""" + if platform_name == "nt": + return os.O_RDWR | binary_flag + return os.O_RDONLY + + +_FILE_SYNC_FLAGS = _file_sync_flags(os.name, getattr(os, "O_BINARY", 0)) + + class SizedTensor(Protocol): """Structural subset used to estimate a serialized state dictionary.""" @@ -160,7 +170,7 @@ def _sync_file(path: Path) -> None: mode = path.lstat().st_mode if not stat.S_ISREG(mode): raise OSError(f"Checkpoint artifact is not a regular file: {path}") - descriptor = os.open(path, os.O_RDONLY) + descriptor = os.open(path, _FILE_SYNC_FLAGS) try: os.fsync(descriptor) finally: diff --git a/tests/test_persistence_contracts.py b/tests/test_persistence_contracts.py index 9454b78..e04bfc3 100644 --- a/tests/test_persistence_contracts.py +++ b/tests/test_persistence_contracts.py @@ -498,6 +498,54 @@ def test_atomic_checkpoint_flushes_nested_directories(tmp_path, monkeypatch): assert "nested" in synced_directories +def test_file_sync_flags_use_writable_binary_handle_on_windows(): + binary_flag = 0x8000 + expected = os.O_RDWR | binary_flag + + assert persistence._file_sync_flags("nt", binary_flag) == expected + assert expected != os.O_RDONLY + + +def test_file_sync_flags_use_read_handle_on_posix(): + assert persistence._file_sync_flags("posix", 0x8000) == os.O_RDONLY + + +def test_sync_file_uses_platform_flags_and_closes_descriptor(tmp_path, monkeypatch): + artifact = tmp_path / "artifact.bin" + artifact.write_bytes(b"checkpoint") + open_file = MagicMock(return_value=17) + sync_file = MagicMock() + close_file = MagicMock() + monkeypatch.setattr(persistence, "_FILE_SYNC_FLAGS", 123) + monkeypatch.setattr(persistence.os, "open", open_file) + monkeypatch.setattr(persistence.os, "fsync", sync_file) + monkeypatch.setattr(persistence.os, "close", close_file) + + persistence._sync_file(artifact) + + open_file.assert_called_once_with(artifact, 123) + sync_file.assert_called_once_with(17) + close_file.assert_called_once_with(17) + + +def test_sync_file_closes_descriptor_after_fsync_failure(tmp_path, monkeypatch): + artifact = tmp_path / "artifact.bin" + artifact.write_bytes(b"checkpoint") + close_file = MagicMock() + monkeypatch.setattr(persistence.os, "open", MagicMock(return_value=17)) + monkeypatch.setattr( + persistence.os, + "fsync", + MagicMock(side_effect=OSError("simulated fsync failure")), + ) + monkeypatch.setattr(persistence.os, "close", close_file) + + with pytest.raises(OSError, match="simulated fsync failure"): + persistence._sync_file(artifact) + + close_file.assert_called_once_with(17) + + def test_sync_directory_uses_platform_flags_and_closes_descriptor( tmp_path, monkeypatch,