From 2f19908d9a78e615ba061e5616f1fcd35a944dbc Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Tue, 27 Oct 2026 09:57:38 -0700 Subject: [PATCH] fix(test): remove /dev/shm reset scratch left by killed browser servers (#384) A server killed outright never runs its cleanup, and tmpfs keeps the scratch folder in memory until reboot: the Python suite's server tests left one per killed server. Scratch folders now carry the owner's pid, and each new one first removes those whose process is gone (POSIX only). --- tests/browser/fast_reset.py | 39 ++++++++++++++++++++++++++------ tests/test_browser_fast_reset.py | 24 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/tests/browser/fast_reset.py b/tests/browser/fast_reset.py index 5f2634bc..c305c686 100644 --- a/tests/browser/fast_reset.py +++ b/tests/browser/fast_reset.py @@ -43,6 +43,8 @@ MEMORY_ROOT = Path("/dev/shm") # the browsers and the other workers; with less free than this, the scratch # copy goes on disk instead. MEMORY_FREE_BYTES = 32 * 1024 * 1024 +# Scratch folders are named -. +SCRATCH_PREFIX = "pm-reset-" def _quote(name: str) -> str: @@ -94,6 +96,31 @@ def table_digests(conn: sqlite3.Connection, schema: str = "main") -> dict[str, s return result +def _process_alive(pid: int) -> bool: + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + except OSError: + return True # it exists but belongs to someone else + return True + + +def remove_orphaned_scratch(root: Path) -> None: + """Remove scratch folders of servers that died without cleaning up. + + A server killed outright (SIGKILL, a crash) never runs ``close``, and + tmpfs keeps its folder in memory until reboot. The folder name carries + the owner's process id; a folder whose process is gone is removed. + """ + if os.name != "posix": + return # os.kill(pid, 0) would terminate the process on Windows + for folder in root.glob(f"{SCRATCH_PREFIX}*-*"): + pid = folder.name[len(SCRATCH_PREFIX) :].split("-", 1)[0] + if pid.isdigit() and not _process_alive(int(pid)): + shutil.rmtree(folder, ignore_errors=True) + + class _SeedApi: """The live API, except that ``db`` is the scratch database. @@ -247,12 +274,10 @@ class ScratchSeeder: kernel is writing back the pages they rewrite. """ root = self._memory_root - if ( - root is not None - and root.is_dir() - and os.access(root, os.W_OK) - and shutil.disk_usage(root).free >= MEMORY_FREE_BYTES - ): - return Path(tempfile.mkdtemp(prefix="pm-reset-", dir=root)) + if root is not None and root.is_dir() and os.access(root, os.W_OK): + remove_orphaned_scratch(root) + if shutil.disk_usage(root).free >= MEMORY_FREE_BYTES: + prefix = f"{SCRATCH_PREFIX}{os.getpid()}-" + return Path(tempfile.mkdtemp(prefix=prefix, dir=root)) self._fallback.mkdir(parents=True, exist_ok=True) return self._fallback diff --git a/tests/test_browser_fast_reset.py b/tests/test_browser_fast_reset.py index ba53a41c..6be9a54a 100644 --- a/tests/test_browser_fast_reset.py +++ b/tests/test_browser_fast_reset.py @@ -1,7 +1,10 @@ """Browser-test resets seed a scratch copy and publish it in one commit (#384).""" import hashlib +import os import sqlite3 +import subprocess +import sys import tempfile import threading import time @@ -300,6 +303,27 @@ class TestScratchSeeder(unittest.TestCase): self.assertEqual(list(memory.iterdir()), []) self.assertTrue((self.root / "fallback" / "seed.db").exists()) + @unittest.skipUnless(os.name == "posix", "process probe is POSIX only") + def test_scratch_folders_of_dead_servers_are_removed(self): + memory = self.root / "memory" + memory.mkdir() + dead = subprocess.Popen([sys.executable, "-c", "pass"]) + dead.wait() + orphan = memory / f"pm-reset-{dead.pid}-abc" + alive = memory / f"pm-reset-{os.getpid()}-def" + other = memory / "pm-reset-notapid" + for folder in (orphan, alive, other): + folder.mkdir() + (folder / "seed.db").write_bytes(b"x") + seeder = ScratchSeeder(self.api, self.root / "fallback", memory_root=memory) + self.addCleanup(seeder.close) + seeder.run(reseed) + self.assertFalse(orphan.exists()) + self.assertTrue(alive.exists()) + self.assertTrue(other.exists()) + own = [p for p in memory.glob(f"pm-reset-{os.getpid()}-*") if p != alive] + self.assertEqual(len(own), 1) + def test_close_releases_the_scratch_files(self): self.seeder.run(reseed) self.seeder.close()