From 301e06391b77c6ca67dc9f0d4af1ceccb8d60447 Mon Sep 17 00:00:00 2001 From: buzzer-re <22428720+buzzer-re@users.noreply.github.com> Date: Thu, 23 Jul 2026 07:32:05 -0700 Subject: [PATCH 1/4] Retry atomic file replace to survive transient Windows locks On Windows, write_text_atomic's tmp.replace(dst) fails with PermissionError WinError 5 (access denied) or 32 (sharing violation) when antivirus, the search indexer, or OneDrive briefly holds the destination or temp file open without FILE_SHARE_DELETE. Under -j 8 the checkpoint is rewritten on every completed function, so such a collision eventually aborts the whole export (issue #11). Wrap the replace in a bounded retry that backs off and retries only on those transient winerror codes, then re-raises; other platforms and non-transient errors are unaffected (winerror is None off Windows). On final failure the temp file is removed so failures no longer leave .tmp orphans behind. tests/test_atomic_write.py reproduces the exact failure with a real Windows file lock (ctypes CreateFileW, read-share/no delete-share) and covers the transient-recovers, permanent-still-raises, and no-retry-off-Windows cases, plus a portable monkeypatched regression that runs on any platform. Co-Authored-By: Claude Opus 4.8 --- src/tocode/metadata.py | 30 ++++++- tests/test_atomic_write.py | 168 +++++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 tests/test_atomic_write.py diff --git a/src/tocode/metadata.py b/src/tocode/metadata.py index 7b7fb55..6e529ae 100644 --- a/src/tocode/metadata.py +++ b/src/tocode/metadata.py @@ -6,6 +6,7 @@ from pathlib import Path import re import tempfile +import time from typing import Callable from .naming import SHARED_CLUSTER_ID, c_file_name, clean_path_component @@ -756,6 +757,28 @@ def write_json(path: Path, payload: dict[str, object]) -> None: write_text_atomic(path, json.dumps(payload, indent=2, sort_keys=False) + "\n") +_ATOMIC_REPLACE_ATTEMPTS = 10 +_ATOMIC_REPLACE_BASE_DELAY = 0.05 # seconds; ~2.75s worst case across attempts +# Windows returns ERROR_ACCESS_DENIED (5) or ERROR_SHARING_VIOLATION (32) when +# another process (antivirus, the search indexer, OneDrive) briefly holds the +# destination or the temp file open without FILE_SHARE_DELETE at the instant of +# the rename. These are transient, so retry before giving up. +_TRANSIENT_REPLACE_WINERRORS = frozenset({5, 32}) + + +def _replace_with_retry(src: Path, dst: Path) -> None: + for attempt in range(_ATOMIC_REPLACE_ATTEMPTS): + try: + src.replace(dst) + return + except PermissionError as exc: + # winerror is None off Windows, so other platforms re-raise at once. + transient = getattr(exc, "winerror", None) in _TRANSIENT_REPLACE_WINERRORS + if not transient or attempt == _ATOMIC_REPLACE_ATTEMPTS - 1: + raise + time.sleep(_ATOMIC_REPLACE_BASE_DELAY * (attempt + 1)) + + def write_text_atomic(path: Path, text: str) -> None: path.parent.mkdir(parents=True, exist_ok=True) with tempfile.NamedTemporaryFile( @@ -768,4 +791,9 @@ def write_text_atomic(path: Path, text: str) -> None: ) as handle: tmp_path = Path(handle.name) handle.write(text) - tmp_path.replace(path) + try: + _replace_with_retry(tmp_path, path) + except OSError: + # Do not leave the temp file behind when the replace ultimately fails. + tmp_path.unlink(missing_ok=True) + raise diff --git a/tests/test_atomic_write.py b/tests/test_atomic_write.py new file mode 100644 index 0000000..055481f --- /dev/null +++ b/tests/test_atomic_write.py @@ -0,0 +1,168 @@ +"""Reproduction and regression tests for the Windows atomic-write crash (#11). + +`write_text_atomic` renames a temp file over the destination. On Windows that +rename fails with ``PermissionError`` (``WinError 5`` access-denied, or ``32`` +sharing-violation) whenever another process — antivirus, the search indexer, +OneDrive — briefly holds the destination or the temp file open without +``FILE_SHARE_DELETE``. Under ``-j 8`` the checkpoint is rewritten on every +completed function, so such a collision eventually aborts the whole export. + +The portable test reproduces the mechanism on any platform by monkeypatching the +rename; the Windows test reproduces the exact production failure with a real lock. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +from tocode.metadata import write_text_atomic + + +def _win_permission_error(winerror: int) -> PermissionError: + exc = PermissionError(winerror, "Access is denied") + # Mirror the OSError.winerror attribute CPython sets on Windows so the code + # under test can distinguish transient lock errors from real ones. + exc.winerror = winerror # type: ignore[attr-defined] + return exc + + +def _tmp_siblings(path: Path) -> list[Path]: + return [p for p in path.parent.iterdir() if p.name != path.name] + + +def test_replace_retries_past_transient_lock(tmp_path, monkeypatch) -> None: + """A few transient WinError 5 failures must not abort the write.""" + target = tmp_path / "checkpoint.json" + real_replace = Path.replace + calls = {"n": 0} + + def flaky_replace(self: Path, dst): + calls["n"] += 1 + if calls["n"] <= 3: # transient lock clears after a few attempts + raise _win_permission_error(5) + return real_replace(self, dst) + + monkeypatch.setattr(Path, "replace", flaky_replace) + monkeypatch.setattr("tocode.metadata.time.sleep", lambda _s: None) + + write_text_atomic(target, "payload\n") + + assert calls["n"] == 4 # 3 failures + 1 success + assert target.read_text(encoding="utf-8") == "payload\n" + # No orphan .tmp left behind. + assert _tmp_siblings(target) == [] + + +def test_replace_gives_up_and_cleans_tmp_when_lock_never_clears( + tmp_path, monkeypatch +) -> None: + """A permanent lock still raises, but must not litter .tmp orphans.""" + target = tmp_path / "checkpoint.json" + + def always_locked(self: Path, dst): + raise _win_permission_error(5) + + monkeypatch.setattr(Path, "replace", always_locked) + monkeypatch.setattr("tocode.metadata.time.sleep", lambda _s: None) + + with pytest.raises(PermissionError): + write_text_atomic(target, "payload\n") + + assert not target.exists() + assert _tmp_siblings(target) == [] + + +def test_non_windows_permission_error_is_not_retried(tmp_path, monkeypatch) -> None: + """A PermissionError without a transient winerror re-raises immediately.""" + target = tmp_path / "checkpoint.json" + calls = {"n": 0} + + def denied(self: Path, dst): + calls["n"] += 1 + raise PermissionError(13, "Permission denied") # e.g. read-only dir + + monkeypatch.setattr(Path, "replace", denied) + monkeypatch.setattr("tocode.metadata.time.sleep", lambda _s: None) + + with pytest.raises(PermissionError): + write_text_atomic(target, "payload\n") + + assert calls["n"] == 1 # not retried + + +# --- Faithful Windows reproduction with a real file lock ------------------ + +_WINDOWS_ONLY = pytest.mark.skipif( + sys.platform != "win32", reason="reproduces a Windows file-sharing error" +) + + +def _open_read_share_no_delete(path: Path): + """Open ``path`` the way a scanner does: read-share, no delete-share. + + Returns a Windows HANDLE that blocks anyone from replacing/deleting the file + until it is closed. Mirrors antivirus / OneDrive real-time access. + """ + import ctypes + from ctypes import wintypes + + GENERIC_READ = 0x80000000 + FILE_SHARE_READ = 0x00000001 + OPEN_EXISTING = 3 + INVALID_HANDLE_VALUE = ctypes.c_void_p(-1).value + + CreateFileW = ctypes.windll.kernel32.CreateFileW + CreateFileW.restype = wintypes.HANDLE + handle = CreateFileW( + str(path), + GENERIC_READ, + FILE_SHARE_READ, # deliberately no FILE_SHARE_DELETE + None, + OPEN_EXISTING, + 0, + None, + ) + if handle == INVALID_HANDLE_VALUE: + raise ctypes.WinError(ctypes.get_last_error()) + return handle + + +def _close_handle(handle) -> None: + import ctypes + + ctypes.windll.kernel32.CloseHandle(handle) + + +@_WINDOWS_ONLY +def test_windows_permanent_lock_reproduces_winerror5(tmp_path) -> None: + target = tmp_path / "checkpoint.json" + target.write_text("old\n", encoding="utf-8") + handle = _open_read_share_no_delete(target) + try: + with pytest.raises(PermissionError) as info: + write_text_atomic(target, "new\n") + assert getattr(info.value, "winerror", None) in (5, 32) + finally: + _close_handle(handle) + + +@_WINDOWS_ONLY +def test_windows_transient_lock_recovers(tmp_path) -> None: + import threading + import time + + target = tmp_path / "checkpoint.json" + target.write_text("old\n", encoding="utf-8") + handle = _open_read_share_no_delete(target) + + # Release the lock shortly after the write starts, as a scanner would. + threading.Timer(0.15, lambda: _close_handle(handle)).start() + + started = time.monotonic() + write_text_atomic(target, "new\n") # retries until the lock clears + assert target.read_text(encoding="utf-8") == "new\n" + assert time.monotonic() - started >= 0.1 + assert _tmp_siblings(target) == [] From ea741be8d44bb4c8748a620486c666fac7f82fc8 Mon Sep 17 00:00:00 2001 From: buzzer-re <22428720+buzzer-re@users.noreply.github.com> Date: Thu, 23 Jul 2026 07:34:35 -0700 Subject: [PATCH 2/4] Drop the redundant Command log line from the CLI export path Removes the "Command: ..." status log emitted at the start of a standard export; the invocation is already discoverable and the line added noise. Co-Authored-By: Claude Opus 4.8 --- src/tocode/cli.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tocode/cli.py b/src/tocode/cli.py index dee5ec4..7387856 100644 --- a/src/tocode/cli.py +++ b/src/tocode/cli.py @@ -185,9 +185,6 @@ def main(argv: list[str] | None = None) -> int: else binary.parent / default_output_name(binary) ) progress.set_log_path(log_root / "tocode.log") - progress.log( - f"Command: {' '.join(sys.argv if argv is None else ['tocode', *argv])}" - ) started = time.monotonic() try: if not binary.is_file(): From 35b0f392ca735f0df9added3a522e2bca18c140d Mon Sep 17 00:00:00 2001 From: buzzer-re <22428720+buzzer-re@users.noreply.github.com> Date: Thu, 23 Jul 2026 07:58:39 -0700 Subject: [PATCH 3/4] Fix Windows-only mypy and test failures so ci-local passes Two pre-existing Windows portability issues broke the local CI gate on Windows (both pass on the Linux CI, so they were latent): - parallel.py: os.sysconf is POSIX-only, so mypy on Windows flagged the attribute as undefined. Fetch it via getattr so the runtime guard is explicit and static analysis passes on every platform; behavior is unchanged (returns None when sysconf is unavailable). - _relativize_sources: os.path.relpath yields backslash separators on Windows, so exported source paths (and the corresponding test) diverged from the POSIX form. Normalize to forward slashes so exports match across platforms. ci-local.ps1 now reports all five checks passing on Windows. Co-Authored-By: Claude Opus 4.8 --- src/tocode/exporter.py | 8 ++++++-- src/tocode/parallel.py | 11 ++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/tocode/exporter.py b/src/tocode/exporter.py index 2a40f4a..5c67f25 100644 --- a/src/tocode/exporter.py +++ b/src/tocode/exporter.py @@ -3061,13 +3061,17 @@ def _relativize_sources(files: list[str]) -> dict[str, str]: for source_file in files: if os.path.isabs(source_file) and root: try: - result[source_file] = os.path.relpath(source_file, root) + # Emit POSIX separators so exports match across platforms + # (os.path.relpath yields backslashes on Windows). + result[source_file] = os.path.relpath(source_file, root).replace( + os.sep, "/" + ) continue except ValueError: pass result[source_file] = ( source_file.lstrip("/") if os.path.isabs(source_file) else source_file - ) + ).replace(os.sep, "/") return result diff --git a/src/tocode/parallel.py b/src/tocode/parallel.py index 609c376..7b47ace 100644 --- a/src/tocode/parallel.py +++ b/src/tocode/parallel.py @@ -152,10 +152,15 @@ def available_memory_mb() -> int | None: meminfo = _linux_mem_available_mb() if meminfo is not None: return meminfo + # os.sysconf is POSIX-only (absent on Windows); fetch it dynamically so the + # attribute access does not fail static analysis on Windows. + sysconf = getattr(os, "sysconf", None) + if sysconf is None: + return None try: - pages = os.sysconf("SC_AVPHYS_PAGES") - page_size = os.sysconf("SC_PAGE_SIZE") - except (AttributeError, OSError, ValueError): + pages = sysconf("SC_AVPHYS_PAGES") + page_size = sysconf("SC_PAGE_SIZE") + except (OSError, ValueError): return None if not isinstance(pages, int) or not isinstance(page_size, int): return None From 3839dd576035f83b1e859ff0ca291aca800f77f6 Mon Sep 17 00:00:00 2001 From: buzzer-re <22428720+buzzer-re@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:11:40 -0700 Subject: [PATCH 4/4] Make the atomic-write lock test type-check on Linux The Windows reproduction helper used ctypes.windll / ctypes.WinError / wintypes, which are absent from the ctypes stubs on non-Windows platforms, so mypy failed on the Linux CI (it passed locally on Windows). Reach windll via getattr, use c_void_p for the HANDLE, and raise a plain OSError instead of WinError. Behavior on Windows is unchanged; verified with `mypy src tests --platform linux`. Co-Authored-By: Claude Opus 4.8 --- tests/test_atomic_write.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_atomic_write.py b/tests/test_atomic_write.py index 055481f..ccb8d6a 100644 --- a/tests/test_atomic_write.py +++ b/tests/test_atomic_write.py @@ -107,16 +107,18 @@ def _open_read_share_no_delete(path: Path): until it is closed. Mirrors antivirus / OneDrive real-time access. """ import ctypes - from ctypes import wintypes GENERIC_READ = 0x80000000 FILE_SHARE_READ = 0x00000001 OPEN_EXISTING = 3 INVALID_HANDLE_VALUE = ctypes.c_void_p(-1).value - CreateFileW = ctypes.windll.kernel32.CreateFileW - CreateFileW.restype = wintypes.HANDLE - handle = CreateFileW( + # windll is Windows-only (absent from the ctypes stubs on other platforms), + # so reach it dynamically to keep static analysis happy on the Linux CI. A + # HANDLE is a void*, so use c_void_p rather than the win-only wintypes. + kernel32 = getattr(ctypes, "windll").kernel32 + kernel32.CreateFileW.restype = ctypes.c_void_p + handle = kernel32.CreateFileW( str(path), GENERIC_READ, FILE_SHARE_READ, # deliberately no FILE_SHARE_DELETE @@ -126,14 +128,14 @@ def _open_read_share_no_delete(path: Path): None, ) if handle == INVALID_HANDLE_VALUE: - raise ctypes.WinError(ctypes.get_last_error()) + raise OSError(f"CreateFileW failed to lock {path}") return handle def _close_handle(handle) -> None: import ctypes - ctypes.windll.kernel32.CloseHandle(handle) + getattr(ctypes, "windll").kernel32.CloseHandle(handle) @_WINDOWS_ONLY