diff --git a/scripts/pack_probe.py b/scripts/pack_probe.py index bb4adbfe1..7c466c7b1 100644 --- a/scripts/pack_probe.py +++ b/scripts/pack_probe.py @@ -27,16 +27,21 @@ the verdict, every file the core opened mapped to the pack entry that put it there, and the findings: a file the core looked for that the layout lacks, or holds under another path. +Each core_probe run holds the host's probe lock (~/.local/state/retrobios/ +probe.lock, RETROBIOS_PROBE_LOCK) and releases it before the next: other +probes run between ours. Building and extracting the pack take no lock. + Usage: - flock ~/.local/state/retrobios/probe.lock \\ - python scripts/pack_probe.py run --platform retroarch [--system sony-playstation] + python scripts/pack_probe.py run --platform retroarch [--system sony-playstation] python scripts/pack_probe.py plan --platform batocera """ from __future__ import annotations import argparse +import contextlib import datetime as dt +import fcntl import hashlib import json import os @@ -80,6 +85,8 @@ VERDICTS = ("boots", "bios_error", "crash", "hang", "unknown") MAX_ROUNDS = 6 PROBE_TIMEOUT = 900 IDLE_WAIT = 1800 +PROBE_LOCK = Path(os.environ.get("RETROBIOS_PROBE_LOCK") + or Path.home() / ".local" / "state" / "retrobios" / "probe.lock") EXIT_BUSY = 3 # The hardware target each platform is probed for: the one whose libretro @@ -452,17 +459,33 @@ def wait_idle(limit: float = IDLE_WAIT, poll: float = 30.0) -> None: waited += poll +@contextlib.contextmanager +def probe_lock(path: Path | None = None): + """The host's probe lock, for one run: one probe at a time on the host, + released between runs so a pass never holds it for its whole length.""" + path = path or PROBE_LOCK + path.parent.mkdir(parents=True, exist_ok=True) + with path.open("a") as handle: + fcntl.flock(handle, fcntl.LOCK_EX) + try: + yield + finally: + fcntl.flock(handle, fcntl.LOCK_UN) + + def core_probe(argv: list[str], staged: set[str], layout: Layout, work: Path) -> tuple[int, dict | None, str]: - """core_probe.py run, the system files passed in an argument file.""" - if "--record" in argv: - wait_idle() + """core_probe.py run under the probe lock, the system files passed in an + argument file.""" work.mkdir(parents=True, exist_ok=True) argfile = work / "system-files.args" argfile.write_text("".join(f"--system-file\n{rel}={layout.root / rel}\n" for rel in sorted(staged)), encoding="utf-8") command = [sys.executable, str(ROOT / "scripts" / "core_probe.py"), *argv, f"@{argfile}"] - proc = subprocess.run(command, capture_output=True, text=True, check=False, cwd=ROOT, - timeout=PROBE_TIMEOUT * 2 + 120) + with probe_lock(): + if "--record" in argv: + wait_idle() + proc = subprocess.run(command, capture_output=True, text=True, check=False, cwd=ROOT, + timeout=PROBE_TIMEOUT * 2 + 120) record = None if proc.returncode == 0: try: diff --git a/tests/test_pack_probe.py b/tests/test_pack_probe.py index fbca00844..ec02bdf79 100644 --- a/tests/test_pack_probe.py +++ b/tests/test_pack_probe.py @@ -246,6 +246,41 @@ class SitePages(unittest.TestCase): self.assertEqual(sm.pack_tested_section([]), []) +class Lock(unittest.TestCase): + """The probe lock is held for one core_probe run and released after it.""" + + def test_held_for_each_run_only(self) -> None: + import fcntl + import subprocess + import unittest.mock + + work = TMP / "lock" + shutil.rmtree(work, ignore_errors=True) + lock = work / "probe.lock" + held = [] + + def try_lock() -> bool: + with lock.open("a") as handle: + try: + fcntl.flock(handle, fcntl.LOCK_EX | fcntl.LOCK_NB) + except BlockingIOError: + return True + fcntl.flock(handle, fcntl.LOCK_UN) + return False + + def fake_run(command, **kwargs): + held.append(try_lock()) + return subprocess.CompletedProcess(command, 2, "", "refused") + + lay = layout({"a.bin": 1}) + with unittest.mock.patch.object(pp, "PROBE_LOCK", lock), \ + unittest.mock.patch.object(pp.subprocess, "run", fake_run): + for _ in range(2): + self.assertEqual(pp.core_probe(["run"], {"a.bin"}, lay, work), (2, None, "refused")) + self.assertEqual(held, [True, True]) + self.assertFalse(try_lock()) + + def collection_on_disk() -> bool: return (REPO_ROOT / "bios" / "Sony" / "PlayStation" / "scph5501.bin").is_file() diff --git a/wiki/tools.md b/wiki/tools.md index 63a845827..09db531fc 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -1078,12 +1078,13 @@ Schemas: `campaign-finding.schema.json` and `campaign-run.schema.json`. Tests what the project promises: a platform's pack, installed the way a user installs it, starts that platform's games. A maintainer starts it, nothing -runs it on a schedule. Run it under the probe lock, one run on the host at a -time: +runs it on a schedule. It takes the host's probe lock +(`~/.local/state/retrobios/probe.lock`, `RETROBIOS_PROBE_LOCK`) around each +`core_probe` run and releases it before the next, so other probes run between +its runs. Never wrap the whole command in `flock`: ```bash -flock ~/.local/state/retrobios/probe.lock \ - python scripts/pack_probe.py run --platform retroarch +python scripts/pack_probe.py run --platform retroarch python scripts/pack_probe.py run --platform batocera --system sega-saturn --install pack python scripts/pack_probe.py plan --platform retrobat # systems, games and cores, nothing run ``` @@ -1094,7 +1095,7 @@ python scripts/pack_probe.py plan --platform retrobat # systems, games and cor | Games | `tests/campaign/content.yml` and `tests/campaign/pack_content.yml`, same format, read from `.cache/test-content` of the main checkout and checked against their `identity.json`. `--fetch` gets a missing one through romfetch | | Cores | `resolve_platform_cores` with the target of the platform (`linux-x86_64` for RetroArch, `x86_64` for Batocera, `windows` for RetroBat), kept when the profile serves the system and its libretro build runs there: not a `standalone_cores` entry, unless `dual_mode_cores` names it. The `.so` is the buildbot's x86_64 Linux nightly, every build the target names, else the one named after the profile, else the first of its `cores:`. Kept in `.cache/pack-probe/cores` with its URL and `Last-Modified` | | Staging | `core_probe.py` copies what it stages, and a pack holds gigabytes, so the probe stages what the core can reach: the profile's libretro entries (path or name, and aliases), the platform's list for the system, every file of the folders those sit in, and for a profile with `found_by: content` every root file of a size it declares. A file or folder the core then looks for that the layout holds is staged and the run repeated, up to 6 runs | -| Run | `core_probe.py run --require-sandbox --record`, the game's frames and frame sampling from its content entry, no core option set. The first run is recorded. When the staging grew, the closed staging runs again, recorded with `--supersedes` the first. Before a recorded run the probe waits for the 1-minute load to fall to a quarter of the CPUs, up to 30 minutes, then stops with exit 3 | +| Run | `core_probe.py run --require-sandbox --record`, the game's frames and frame sampling from its content entry, no core option set. The first run is recorded. When the staging grew, the closed staging runs again, recorded with `--supersedes` the first. Each run holds the probe lock. Before a recorded run, lock held, the probe waits for the 1-minute load to fall to a quarter of the CPUs, up to 30 minutes, then stops with exit 3 | Each probe is one line of `measurements/pack-probes.jsonl` (`pack-probe.schema.json`): the platform and its version, the install path,