fix: hold data directories while packs read them

scripts/common.py

@@ -716,8 +716,17 @@ _data_dir_index: dict[str, dict[str, list[tuple[int, str]]]] = {}
 
 
 def _data_dir_basenames(cache_dir: str) -> dict[str, list[tuple[int, str]]]:
-    """Casefolded basename -> (walk order, path) of every file in a cache."""
-    key = os.path.realpath(cache_dir)
+    """Casefolded basename -> (walk order, path) of every file in a cache.
+
+    Keyed by the tree's identity as well as its path: a refresh swaps in a
+    new directory under the same name, and an index of the old one named
+    files that were gone.
+    """
+    try:
+        stat = os.stat(cache_dir)
+    except OSError:
+        return {}
+    key = f"{os.path.realpath(cache_dir)}:{stat.st_dev}:{stat.st_ino}"
     if key not in _data_dir_index:
         index: dict[str, list[tuple[int, str]]] = {}
         order = 0

scripts/generate_pack.py

@@ -64,7 +64,7 @@ from common import (
     yaml_load,
 )
 import packresolve
-from refresh_data_dirs import cache_lock
+from refresh_data_dirs import cache_lock, hold_data_dirs
 import region as region_mod
 from manifestselect import SelectionIndex
 import slot as slot_mod
@@ -2777,7 +2777,11 @@ def _given_flags(args) -> set[str]:
 
 
 def main():
+    with contextlib.ExitStack() as held:
+        _main(held)
 
+
+def _main(held: contextlib.ExitStack) -> None:
     parser = argparse.ArgumentParser(description="Generate platform BIOS ZIP packs")
     parser.add_argument("--platform", "-p", help="Platform name (e.g., retroarch)")
     parser.add_argument(
@@ -2936,6 +2940,10 @@ def main():
     _validate_args(args, parser)
 
     # Hash lookup / pack mode
+    # A platform run may refresh the trees below, under the exclusive lock:
+    # it takes the shared ones after that. Every other mode reads them now.
+    if not (args.platform or args.all) or args.offline or args.no_refresh_data:
+        hold_data_dirs(held, load_data_dir_registry(args.platforms_dir))
     has_from_md5 = bool(args.from_md5 or getattr(args, "from_md5_file", None))
     if has_from_md5:
         hashes = (
@@ -3049,6 +3057,8 @@ def main():
         failed = sorted(k for k, v in results.items() if v is None)
         if failed:
             print(f"WARNING: data directory refresh failed: {', '.join(failed)}")
+    if data_registry:
+        hold_data_dirs(held, data_registry)
 
     emu_profiles = load_emulator_profiles(args.emulators_dir)
 

scripts/refresh_data_dirs.py

@@ -62,6 +62,21 @@ def cache_lock(cache_dir: str | Path, shared: bool = False):
     return _file_lock(cache_dir.with_name(f".{cache_dir.name}.lock"), shared=shared)
 
 
+def hold_data_dirs(stack: contextlib.ExitStack, registry: dict) -> None:
+    """Hold every cached data directory shared for the life of *stack*.
+
+    A pack resolves names against a tree, then copies the file minutes
+    later; a refresh swapping the tree in between shipped bytes nobody
+    verified, or died on a file the new tree no longer has. A refresh waits
+    for the holders. Never called before a refresh in the same process:
+    the exclusive lock would wait on this one.
+    """
+    for entry in registry.values():
+        cache = entry.get("local_cache", "")
+        if cache and os.path.isdir(cache):
+            stack.enter_context(cache_lock(cache, shared=True))
+
+
 def _load_versions(versions_path: str = VERSIONS_FILE) -> dict[str, dict]:
     path = Path(versions_path)
     if not path.exists():

scripts/verify.py

@@ -25,6 +25,7 @@ Usage:
 from __future__ import annotations
 
 import argparse
+import contextlib
 import json
 import os
 import sys
@@ -2077,6 +2078,11 @@ def _run_listing(args: argparse.Namespace, parser: argparse.ArgumentParser) -> N
 
 
 def main():
+    with contextlib.ExitStack() as held:
+        _main(held)
+
+
+def _main(held: contextlib.ExitStack) -> None:
     parser = argparse.ArgumentParser(description="Platform-native BIOS verification")
     parser.add_argument("--platform", "-p", help="Platform name")
     parser.add_argument(
@@ -2146,6 +2152,10 @@ def main():
             "--platform, --all, --emulator, and --system are mutually exclusive"
         )
     _refuse_mode_flags(args, parser)
+    # The trees stay put while names resolve against them and files are read.
+    from refresh_data_dirs import hold_data_dirs
+
+    hold_data_dirs(held, load_data_dir_registry(args.platforms_dir))
 
     with open(args.db) as f:
         db = json.load(f)

tests/test_audit_regressions.py

@@ -3190,3 +3190,57 @@ class AnEmptyTargetIsNotEveryCore(unittest.TestCase):
         self.assertEqual(published["board"], sorted(builder))
         self.assertEqual(published["board"], ["gpsp"])
         self.assertIsNone(published["full"])
+
+
+class ADataDirectoryIsHeldWhileRead(unittest.TestCase):
+    """A pack resolved names against data/bluemsx, then a concurrent refresh
+    swapped the tree: the index named files that were gone and the copy
+    died on FileNotFoundError, or shipped bytes nobody had verified."""
+
+    def setUp(self):
+        self.tmp = Path(tempfile.mkdtemp(dir=TMP_ROOT))
+        self.addCleanup(lambda: __import__("shutil").rmtree(self.tmp, True))
+
+    def test_the_index_follows_a_swapped_tree(self):
+        from scripts import common
+
+        cache = self.tmp / "cache"
+        cache.mkdir()
+        (cache / "old.rom").write_bytes(b"old")
+        self.assertIn("old.rom", common._data_dir_basenames(str(cache)))
+        cache.rename(self.tmp / "gone")
+        cache.mkdir()
+        (cache / "new.rom").write_bytes(b"new")
+        index = common._data_dir_basenames(str(cache))
+        self.assertIn("new.rom", index)
+        self.assertNotIn("old.rom", index)
+
+    def test_a_holder_makes_the_refresh_lock_wait(self):
+        import fcntl
+
+        from scripts import refresh_data_dirs
+
+        cache = self.tmp / "bluemsx"
+        cache.mkdir()
+        lock = cache.with_name(f".{cache.name}.lock")
+
+        def exclusive_free() -> bool:
+            with open(lock, "a") as handle:
+                try:
+                    fcntl.flock(handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
+                except BlockingIOError:
+                    return False
+                fcntl.flock(handle, fcntl.LOCK_UN)
+                return True
+
+        with contextlib.ExitStack() as held:
+            refresh_data_dirs.hold_data_dirs(held, {"bluemsx": {"local_cache": str(cache)}})
+            self.assertFalse(exclusive_free())
+        self.assertTrue(exclusive_free())
+
+    def test_both_entry_points_hold_the_trees(self):
+        pack = (ROOT / "scripts" / "generate_pack.py").read_text()
+        verify_src = (ROOT / "scripts" / "verify.py").read_text()
+        self.assertIn("hold_data_dirs(held,", verify_src)
+        refresh = pack.index("results = refresh_all(")
+        self.assertIn("hold_data_dirs(held, data_registry)", pack[refresh:])