fix: never hold open an archive the run replaces

A --from folder holding the BIOS folder kept the list's own archive open
after listing its members, and Windows then refused to move it aside or
replace it. The index now notes the places a run writes, closes the
archives among them and opens them only for the read at hand.

install.py

@@ -7198,6 +7198,10 @@ class FolderIndex:
         # was opened again, its directory parsed again, for each member.
         self._zips: dict[Path, zipfile.ZipFile] = {}
         self._zip_lock = threading.Lock()
+        # The places this run writes, resolved. An archive among them is
+        # never kept open: Windows refuses to move aside or replace a file
+        # a process holds, and a --from folder can hold the BIOS folder.
+        self._written: set[Path] = set()
         # (path, member or "") -> (its size or None when unknown, why) of each
         # file, member or folder below root that could not be read.
         self.unreadable: dict[tuple[Path, str], tuple[int | None, str]] = {}
@@ -7275,7 +7279,7 @@ class FolderIndex:
         A few stay open, the least recently read closed first: a card of
         MAME sets would otherwise hold a descriptor per archive. Closing
         one leaves a member being read open until it is closed itself.
-        Only archives the list does not install are read this way.
+        An archive this run writes is never read this way (see writes).
         """
         archive = self._zips.pop(path, None)
         if archive is None:
@@ -7285,15 +7289,38 @@ class FolderIndex:
         self._zips[path] = archive
         return archive
 
+    def writes(self, paths: Collection[Path]) -> None:
+        """Note the places this run writes, resolved, and close the archives
+        among them: the next read of one opens it and closes it at once."""
+        with self._zip_lock:
+            self._written.update(paths)
+            for held in [held for held in self._zips if self._is_written(held)]:
+                self._zips.pop(held).close()
+
+    def _is_written(self, path: Path) -> bool:
+        return bool(self._written) and (
+            path in self._written or Path(os.path.realpath(path)) in self._written)
+
     def infolist(self, path: Path) -> list[zipfile.ZipInfo]:
         with self._zip_lock:
+            if self._is_written(path):
+                with zipfile.ZipFile(path) as archive:
+                    return archive.infolist()
             return self._archive(path).infolist()
 
     @contextlib.contextmanager
     def open_member(self, path: Path, member: str) -> Iterator[BinaryIO]:
         """A member of an archive under the folder, read from any thread."""
         with self._zip_lock:
-            handle = self._archive(path).open(member)
+            if self._is_written(path):
+                archive = zipfile.ZipFile(path)
+                try:
+                    handle = archive.open(member)
+                finally:
+                    # The member holds the file until it is closed itself.
+                    archive.close()
+            else:
+                handle = self._archive(path).open(member)
         try:
             yield handle
         finally:
@@ -7913,6 +7940,8 @@ def plan_local_copies(
             written.add(Path(os.path.realpath(_entry_path(bios_path, entry))))
         except ValueError:
             continue
+    for index in ([bios_index] if bios_index is not None else []) + folders:
+        index.writes(written)
     copies: dict[int, LocalCopy] = {}
     for entry in entries:
         copy = None

tests/test_install_from.py

@@ -854,6 +854,67 @@ class AFolderAroundTheBiosFolder(FromFolder):
                          ["old card/copy.zip, inside as inner.bin"])
 
 
+    @unittest.skipUnless(Path("/proc/self/fd").is_dir(), "reads the open files from /proc")
+    def test_an_archive_the_list_replaces_is_not_held_open(self):
+        """Windows refuses to rename, replace or delete a file a process
+        holds open. The --from index kept the list's own wrong archive open
+        once its members had been listed, and the run failed to move it
+        aside, every run."""
+        good = io.BytesIO()
+        with zipfile.ZipFile(good, "w") as archive:
+            archive.writestr("a.bin", b"A" * 300)
+        good = good.getvalue()
+        card = self.root / "card"
+        bios = card / "bios"
+        _zip(bios / "neogeo.zip", {"x.bin": b"X" * 77})
+        (card / "backup").mkdir()
+        (card / "backup" / "neogeo.zip").write_bytes(good)
+        _zip(card / "backup" / "pack.zip", {"renamed.bin": GBA})
+        self.list.write_text(json.dumps({
+            "manifest_version": 2, "platform": "retroarch",
+            "files": [_entry("neogeo.zip", good, ["snk-neogeo"]),
+                      _entry("gba_bios.bin", GBA, ["nintendo-gba"])]}))
+
+        def held() -> set[str]:
+            out = set()
+            for fd in os.listdir("/proc/self/fd"):
+                with contextlib.suppress(OSError):
+                    out.add(os.path.realpath(os.readlink(f"/proc/self/fd/{fd}")))
+            return out
+
+        real_replace, real_rename, real_unlink = os.replace, os.rename, os.unlink
+
+        def refusing(real):
+            def call(*paths, **kwargs):
+                files = paths[:2] if real is not real_unlink else paths[:1]
+                if any(os.path.realpath(path) in held() for path in files):
+                    raise PermissionError(13, "The process cannot access the file",
+                                          str(files[0]))
+                return real(*paths, **kwargs)
+            return call
+
+        argv = ["install.py", "--platform", "retroarch", "--dest", str(bios),
+                "--manifest", str(self.list), "--from", str(card), "--no-input", "--yes"]
+        out = io.StringIO()
+        with mock.patch.object(sys, "argv", argv), \
+                mock.patch.dict(os.environ, {"RETROBIOS_INSTALL_OFFLINE": "1"}), \
+                mock.patch.object(os, "replace", refusing(real_replace)), \
+                mock.patch.object(os, "rename", refusing(real_rename)), \
+                mock.patch.object(os, "unlink", refusing(real_unlink)), \
+                mock.patch.object(os, "remove", refusing(real_unlink)), \
+                contextlib.redirect_stdout(out), contextlib.redirect_stderr(out):
+            try:
+                install.main()
+                code = 0
+            except SystemExit as exc:
+                code = exc.code
+            finally:
+                install.LOCAL.reset()
+        self.assertEqual(code, 0, out.getvalue())
+        self.assertEqual((bios / "neogeo.zip").read_bytes(), good)
+        self.assertEqual((bios / "gba_bios.bin").read_bytes(), GBA)
+
+
 class ManySameSizedFiles(FromFolder):
     """A card of 16 KB ROMs and a list of 16 KB entries: each entry sorted
     and walked every file of its size."""