From d4169e85533596c159a0ecc06096fd0aa6b44f72 Mon Sep 17 00:00:00 2001 From: Thomas Kittelmann Date: Fri, 9 Oct 2026 11:46:49 +0000 Subject: [PATCH 1/3] Write the cached registry file atomically GitHubRegistry wrote its cached registry file in place, so another process reading it at the same time could get an empty or truncated registry, or fail with "ValueError: not enough values to unpack". It is now written to a temporary file which is then renamed. Co-Authored-By: Claude Opus 5.5 --- src/mccode_antlr/reader/registry.py | 10 ++++++++-- tests/test_registry_file_parallel.py | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 tests/test_registry_file_parallel.py diff --git a/src/mccode_antlr/reader/registry.py b/src/mccode_antlr/reader/registry.py index af83b0b..f7e8e4d 100644 --- a/src/mccode_antlr/reader/registry.py +++ b/src/mccode_antlr/reader/registry.py @@ -426,9 +426,15 @@ def _build_pooch(self): last, check = check, check.parent # check is now a directory that exists, it may be the root of the filesystem if access(check, W_OK): + from os import getpid registry_file_path.parent.mkdir(parents=True, exist_ok=True) - with registry_file_path.open('w', encoding='utf-8') as file: - file.writelines('\n'.join([f'{k} {v}' for k, v in registry.items()])) + # Write-then-rename so a concurrent reader never sees a partial file. + tmp = registry_file_path.with_name(f'{registry_file_path.name}.{getpid()}.tmp') + tmp.write_text('\n'.join([f'{k} {v}' for k, v in registry.items()]), encoding='utf-8') + try: + tmp.replace(registry_file_path) + except OSError: # e.g. on Windows, if another process is reading it + tmp.unlink(missing_ok=True) else: logger.warning(f'Can not output {registry_file_path}, you lack write permissions for {check}') diff --git a/tests/test_registry_file_parallel.py b/tests/test_registry_file_parallel.py new file mode 100644 index 0000000..fafd97d --- /dev/null +++ b/tests/test_registry_file_parallel.py @@ -0,0 +1,22 @@ +"""Registries cached by several processes at the same time.""" +import multiprocessing +from pathlib import Path + +N = 20000 + + +def _registry_size(cache): + import pooch + from mccode_antlr.reader.registry import GitHubRegistry + pooch.os_cache = lambda name: Path(cache, name) + files = {f'dir{i}/Comp{i}.comp': 'sha256:' + 'ab' * 32 for i in range(N)} + try: + return len(GitHubRegistry('test', 'https://example.invalid/x', 'v1.0', registry=files).pooch.registry) + except Exception as error: + return repr(error) + + +def test_parallel_registry_file(tmp_path): + with multiprocessing.get_context('spawn').Pool(8) as pool: + for i in range(5): + assert pool.map(_registry_size, [str(tmp_path / str(i))] * 8) == [N] * 8 From db0fa1b76989625a1560457d7e0111f787f27912 Mon Sep 17 00:00:00 2001 From: Thomas Kittelmann Date: Fri, 9 Oct 2026 12:17:51 +0000 Subject: [PATCH 2/3] Retry pooch fetches failing with PermissionError on Windows When several processes download the same file, pooch moves each download into place with shutil.move, which on Windows falls back to copying onto the file when another process has already put it there, and fails with WinError 32 if a process has the file open at that moment. pooch_fetch now retries the fetch a few times, which then finds the finished file. Co-Authored-By: Claude Opus 5.5 --- src/mccode_antlr/reader/registry.py | 10 +++++++++- tests/test_pooch_fetch_parallel.py | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/mccode_antlr/reader/registry.py b/src/mccode_antlr/reader/registry.py index f7e8e4d..1344b0c 100644 --- a/src/mccode_antlr/reader/registry.py +++ b/src/mccode_antlr/reader/registry.py @@ -67,7 +67,15 @@ def pooch_fetch(pooch_instance, filename: str) -> Path: directory = getattr(pooch_instance, 'abspath', None) if directory is not None: Path(directory, filename).parent.mkdir(parents=True, exist_ok=True) - return Path(pooch_instance.fetch(filename)) + for attempt in range(5): + try: + return Path(pooch_instance.fetch(filename)) + except PermissionError: + # On Windows, another process may be moving the same file into place + # (not fixed by the pooch PR above): + if attempt == 4: + raise + sleep(0.1 * (attempt + 1)) def _dedupe_identical_registry_entries(pooch_instance, names: list[str]) -> str | None: diff --git a/tests/test_pooch_fetch_parallel.py b/tests/test_pooch_fetch_parallel.py index 5304283..ef91524 100644 --- a/tests/test_pooch_fetch_parallel.py +++ b/tests/test_pooch_fetch_parallel.py @@ -48,3 +48,19 @@ def test_parallel_fetch_into_new_directories(tmp_path): assert [f for failed in results for f in failed] == [] for name in NAMES: assert (tmp_path / 'cache' / name).read_text() == name + + +def test_fetch_retried_on_permission_error(tmp_path): + class FlakyPooch: + abspath = tmp_path + calls = 0 + + def fetch(self, filename): + self.calls += 1 + if self.calls < 3: + raise PermissionError('in use by another process') + return str(tmp_path / filename) + + flaky = FlakyPooch() + assert pooch_fetch(flaky, 'd/f.txt') == tmp_path / 'd' / 'f.txt' + assert flaky.calls == 3 From 7c81f1e89925a6c0c41d6df9300245fbdd947ab0 Mon Sep 17 00:00:00 2001 From: Thomas Kittelmann Date: Fri, 9 Oct 2026 12:30:13 +0000 Subject: [PATCH 3/3] Retry reading the cached registry file on Windows On Windows, opening the registry file can fail with PermissionError while another process replaces it with its own copy, as seen in test_parallel_registry_file. The read is now retried a few times. Co-Authored-By: Claude Opus 5.5 --- src/mccode_antlr/reader/registry.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mccode_antlr/reader/registry.py b/src/mccode_antlr/reader/registry.py index 1344b0c..f6d819a 100644 --- a/src/mccode_antlr/reader/registry.py +++ b/src/mccode_antlr/reader/registry.py @@ -417,8 +417,15 @@ def _build_pooch(self): cache_path = pooch.os_cache(f'mccodeantlr/{safe_name}') registry_file_path = cache_path.joinpath(safe_version, safe_file) if registry_file_path.exists() and registry_file_path.is_file() and access(registry_file_path, R_OK): - with registry_file_path.open('r', encoding='utf-8') as file: - registry = {k: v for k, v in [x.strip().split(maxsplit=1) for x in file.readlines() if len(x)]} + for attempt in range(5): + try: + with registry_file_path.open('r', encoding='utf-8') as file: + registry = {k: v for k, v in [x.strip().split(maxsplit=1) for x in file.readlines() if len(x)]} + break + except PermissionError: # On Windows, while another process replaces it + if attempt == 4: + raise + sleep(0.1 * (attempt + 1)) else: # We allow a full-dictionary to be provided, otherwise we expect the registry file to be available from the # base_url where all subsequent files are also expected to be available