fix: judge a cached large file from one read

_fetch_asset hashed the cache copy, then hashed the path again for the
release check. A copy another run promoted in between answered the second
question, and the caller was told the cache held nothing when it held
the file asked for. Both checks now read one snapshot, taken while the
inode stays the same.

scripts/largefiles.py

@@ -164,18 +164,15 @@ def _fetch_asset(
 
     if os.path.isdir(cached):
         return None
+    if os.path.exists(cached) and not (expected_sha1 or expected_md5):
+        return cached
     if os.path.exists(cached):
-        try:
-            hashes = compute_hashes(cached) if (expected_sha1 or expected_md5) else {}
-        except FileNotFoundError:
-            hashes = None
-        if hashes is None:
+        snapshot = _read_once(cached)
+        if snapshot is None:
             pass
-        elif not (expected_sha1 or expected_md5) or _matches(
-            hashes, expected_sha1, expected_md5
-        ):
+        elif _matches(snapshot[0], expected_sha1, expected_md5):
             return cached
-        elif not _replaced_on_release(name, cached, offline):
+        elif not _replaced_on_release(name, snapshot[0]["sha256"], snapshot[1], offline):
             # A copy of this asset that answers another hash: the caller
             # wants a different revision under the same name, and the
             # release still serves this one. Keeping it is what lets the
@@ -249,8 +246,32 @@ def _asset_urls(name: str) -> list[tuple[str, str]]:
     ]
 
 
-def _replaced_on_release(name: str, cached: str, offline: bool) -> bool:
-    """Whether the release now serves other bytes than the cached copy.
+def _read_once(path: str) -> tuple[dict[str, str], int] | None:
+    """Digests and size of the file at *path*, all read from one file, or
+    None when it is gone.
+
+    Every judgement on a cache copy reads this one snapshot. Hashed, then
+    hashed again by path for the release check, a copy another run promoted
+    in between answered the second question: the copy the caller wanted was
+    in the cache and the caller was told there was none.
+    """
+    # A promotion is an os.replace, so it changes the inode: the same inode
+    # on both sides of the read means the digests describe that one file.
+    for _attempt in range(3):
+        try:
+            before = os.stat(path)
+            hashes = compute_hashes(path)
+            after = os.stat(path)
+        except FileNotFoundError:
+            return None
+        if (before.st_ino, before.st_mtime_ns) == (after.st_ino, after.st_mtime_ns):
+            return hashes, after.st_size
+    return None
+
+
+def _replaced_on_release(name: str, sha256: str, size: int, offline: bool) -> bool:
+    """Whether the release now serves other bytes than the cached copy,
+    whose *sha256* and *size* the caller read.
 
     The release lists a sha256 per asset: a dump rebuilt at the same size
     (the eight 251658304-byte DSi NANDs) differs only there, and judged by
@@ -266,9 +287,9 @@ def _replaced_on_release(name: str, cached: str, offline: bool) -> bool:
         None,
     )
     if digest:
-        return compute_hashes(cached, frozenset({"sha256"}))["sha256"] != digest
+        return sha256 != digest
     served = _served_size(name)
-    return served is not None and served != os.path.getsize(cached)
+    return served is not None and served != size
 
 
 _digests_cache: dict[str, str] | None = None

tests/test_large_file_cache.py

@@ -691,7 +691,10 @@ class ASameSizeReuploadIsSeen(unittest.TestCase):
     def replaced(self, digests: dict, size: int | None) -> bool:
         with mock.patch.object(largefiles, "_served_digests", lambda: digests), \
                 mock.patch.object(largefiles, "_served_size", lambda name: size):
-            return largefiles._replaced_on_release("dsi_nand.bin", str(self.cached), False)
+            return largefiles._replaced_on_release(
+                "dsi_nand.bin", hashlib.sha256(self.cached.read_bytes()).hexdigest(),
+                self.cached.stat().st_size, False,
+            )
 
     def test_the_digest_decides(self):
         same = hashlib.sha256(b"OLD NAND").hexdigest()
@@ -703,6 +706,30 @@ class ASameSizeReuploadIsSeen(unittest.TestCase):
         self.assertFalse(self.replaced({}, 8))
         self.assertTrue(self.replaced({}, 9))
 
+    def test_a_copy_promoted_during_the_check_is_the_one_judged(self):
+        """Another run promotes the served revision while this one hashes the
+        stale copy: the release check reread the path, found the served
+        bytes, and the caller was told the cache held no such file."""
+        new = b"NEW NAND"
+        real = largefiles.compute_hashes
+
+        def promote_while_hashing(path, *args, **kwargs):
+            result = real(path, *args, **kwargs)
+            if Path(path).read_bytes() != new:
+                promoted = self.dir / "promoted.tmp"
+                promoted.write_bytes(new)
+                os.replace(promoted, self.cached)
+            return result
+
+        digests = {"dsi_nand.bin": hashlib.sha256(new).hexdigest()}
+        with mock.patch.object(largefiles, "compute_hashes", promote_while_hashing), \
+                mock.patch.object(largefiles, "_served_digests", lambda: digests):
+            found = largefiles._fetch_asset(
+                "dsi_nand.bin", str(self.dir), hashlib.sha1(new).hexdigest(), "",
+                offline=False,
+            )
+        self.assertEqual(found, str(self.cached))
+
     def test_the_release_check_compares_digests_at_equal_size(self):
         expected = {"bios/Nintendo/DSi/dsi_nand.bin": 8}
         assets = {"dsi_nand.bin": 8}