fix(scratch): stuck-entry records recover from nested fixes, survive concurrent pruners

Review of f77f733 found the skip record could outlive the problem and lose
updates:

- Recovery: the record only fingerprinted the top dir, so `chown -R lane/ro`
  left it skipped forever. Records now keep the directories that refused
  access (from rmtree's onexc / the idle scan) and are dropped once all of
  them are accessible to this euid; they also expire after 7 days, and a
  record made by another euid is ignored.
- Concurrency: the state file's read-merge-write now runs under an flock on
  `.prune_stuck.json.lock`, publishes via a mkstemp temp + os.replace, and a
  torn/invalid file reads as empty. A shared `.tmp` name plus stale
  snapshots lost records and could leave `Extra data` JSON.
- Unreadable root-owned 0700 descendants failed the idle scan and never
  reached the recorder, so they were re-scanned hourly with no hint. A scan
  that fails with EACCES/EPERM is now recorded once (fail-closed: never
  deleted) and skipped like the rest.
- The docker hint lookup goes through hermes_platform's locate_command (bare
  shutil.which failed test_bare_which_and_known_path_tables_are_allowlisted).
- The test module carries pytest.mark.platforms("posix") so the macOS lane
  runs it.
This commit is contained in:
Teknium committed 2026-10-18 01:32:18 -07:00
1 parent 78e298e94f
commit 3cbe7b19db
4 files changed
+229 -98

No files matched your search

+31 -17
View File
@@ -111,29 +111,36 @@ def subtree_touch_state(path: Path, cutoff: float) -> str:
Same walk, same stop-at-first-recent shortcut, same symlink rules. A hardlinked file
(``st_nlink > 1``) does not count either: its mtime is the inode's, and
``git clone --local`` hardlinks packs that git freshens from the source repo."""
return _scan_subtree(path, cutoff)[0]
def _scan_subtree(path: Path, cutoff: float) -> tuple[str, str | None]:
"""``(state, denied_dir)``: *denied_dir* is the directory below *path* that refused
listing with EACCES/EPERM when that is why the scan failed (else None)."""
try:
if os.lstat(path).st_mtime >= cutoff:
return _TOUCHED
return _TOUCHED, None
if not path.is_dir() or path.is_symlink():
return _IDLE
return _IDLE, None
except OSError:
return _SCAN_FAILED
return _SCAN_FAILED, None
stack = [str(path)]
while stack:
current = stack.pop()
try:
with os.scandir(stack.pop()) as it:
with os.scandir(current) as it:
for child in it:
try:
st = child.stat(follow_symlinks=False)
except OSError:
return _SCAN_FAILED
except OSError as exc:
return _SCAN_FAILED, current if stuck.is_permission_error(exc) else None
if st.st_mtime >= cutoff and not (st.st_nlink > 1 and child.is_file(follow_symlinks=False)):
return _TOUCHED
return _TOUCHED, None
if child.is_dir(follow_symlinks=False):
stack.append(child.path)
except OSError:
return _SCAN_FAILED
return _IDLE
except OSError as exc:
return _SCAN_FAILED, current if stuck.is_permission_error(exc) else None
return _IDLE, None
def subtree_touched_since(path: Path, cutoff: float) -> bool:
@@ -364,15 +371,22 @@ def _prune_idle_entries(
root: Path, max_idle_hours: float, skip_names: frozenset[str], audit: logging.Logger,
) -> int:
cutoff = time.time() - max_idle_hours * 3600
# Entries already known to hold files this user cannot delete are skipped before any
# walk, until their fingerprint changes (hermes_constants_scratch_stuck).
stuck_state, stuck_names = stuck.load_skips(root)
skip_names = skip_names | stuck_names | {stuck.STUCK_STATE_NAME}
# Entries already known to hold files this user cannot read or delete are skipped before
# any walk, until that is fixed (hermes_constants_scratch_stuck).
skip_names = skip_names | stuck.load_skips(root) | {stuck.STUCK_STATE_NAME, stuck.STUCK_LOCK_NAME}
try:
entries = [e for e in root.iterdir() if e.name not in skip_names]
except OSError:
return 0
doomed = [e for e in entries if not subtree_touched_since(e, cutoff)]
doomed = []
for e in entries:
state, denied_dir = _scan_subtree(e, cutoff)
if state == _IDLE:
doomed.append(e)
elif denied_dir is not None and stuck.tracking_supported():
# A dir this user may not list (root-owned 0700) fails every idle scan: kept
# (fail-closed) as before, but reported once and no longer re-scanned hourly.
stuck.record(root, e, [denied_dir], audit)
# Runs even with nothing to delete: orphans whose cwd was removed by an earlier pass
# (or by hand) are found by the deleted-cwd rule, not by membership in ``doomed``.
try:
@@ -427,7 +441,7 @@ def _prune_idle_entries(
os.fspath(entry),
)
continue
denied = False
denied: list[str] = []
try:
if entry.is_dir() and not entry.is_symlink():
denied = stuck.rmtree_collecting_permission_errors(entry)
@@ -444,7 +458,7 @@ def _prune_idle_entries(
try:
os.lstat(entry)
if denied and stuck.tracking_supported():
stuck.record(root, stuck_state, entry, audit)
stuck.record(root, entry, denied, audit)
else:
audit.info("scratch prune: could not fully remove %r", os.fspath(entry))
continue
+115 -47
View File
@@ -1,13 +1,16 @@
"""Scratch entries the prune cannot delete as this user: remembered once, skipped until they change.
"""Scratch entries the prune cannot delete as this user: remembered once, skipped until fixed.
A container that ran as root and wrote into a bind mount under scratch (the Docker terminal
backend without ``docker_run_as_host_user``, a test or review lane's ``docker run -v``) leaves
files this user cannot unlink. ``rmtree(ignore_errors=True)`` removes what it can, the entry
stays idle, and every hourly pass re-walked the whole subtree and logged the same
"could not fully remove" line again (218 times per entry on one host). Here such an entry is
recorded with a fingerprint of its top directory, the user gets ONE line naming the command
that clears it, and later passes skip it before walking it until the fingerprint changes or
the entry is gone. Nothing is escalated or shelled out to: the hint is for the user to run.
files this user cannot unlink, or directories it cannot even read. ``rmtree(ignore_errors=True)``
removes what it can, the entry stays idle, and every hourly pass re-walked the whole subtree and
logged the same "could not fully remove" line again (218 times per entry on one host). Here such
an entry is recorded with the directories that refused access, the user gets ONE line naming the
command that clears it, and later passes skip it before walking it. A record is dropped (and
the entry judged normally again) when the entry is gone or its top directory changed, when
every recorded blocking directory is accessible now (``chown``/``chmod`` of a nested dir), when
it was made by another uid, or after :data:`RECHECK_AFTER_SECONDS` in any case. Nothing is
escalated or shelled out to: the hint is for the user to run.
POSIX only: without ``os.getuid`` (Windows) a PermissionError is usually a transient open
handle, so the prune keeps retrying there exactly as before.
@@ -21,15 +24,26 @@ import os
import shlex
import shutil
import sys
import tempfile
import time
from pathlib import Path
STUCK_STATE_NAME = ".prune_stuck.json"
STUCK_LOCK_NAME = STUCK_STATE_NAME + ".lock"
# Bounded retry: even a record whose blockers still look inaccessible is re-judged weekly, so a
# fix this module cannot see (ACLs, a sticky bit cleared, a changed group) is never skipped forever.
RECHECK_AFTER_SECONDS = 7 * 24 * 3600
# A record keeps this many blocking dirs; checking a subset can only re-judge early, never skip longer.
_MAX_BLOCKERS = 16
_PERMISSION_ERRNOS = frozenset({errno.EACCES, errno.EPERM})
def tracking_supported() -> bool:
return os.name == "posix" and hasattr(os, "getuid")
return os.name == "posix" and hasattr(os, "geteuid")
def is_permission_error(exc: BaseException) -> bool:
return isinstance(exc, OSError) and exc.errno in _PERMISSION_ERRNOS
def _fingerprint(entry: Path) -> list[int] | None:
@@ -41,61 +55,113 @@ def _fingerprint(entry: Path) -> list[int] | None:
return [st.st_ino, st.st_mtime_ns, st.st_ctime_ns]
def _state_path(root: Path) -> Path:
return root / STUCK_STATE_NAME
def _accessible(path: str) -> bool:
"""True when this user can now list and delete inside *path* (or it is gone)."""
if not os.path.lexists(path):
return True
kw = {"effective_ids": True} if os.access in os.supports_effective_ids else {}
return os.access(path, os.R_OK | os.W_OK | os.X_OK, **kw)
def _still_stuck(root: Path, name: str, rec: dict, now: float) -> bool:
blockers = rec.get("blockers")
return (
rec.get("uid") == os.geteuid() # windows-footgun: ok — only reached when tracking_supported()
and now - float(rec.get("since") or 0) < RECHECK_AFTER_SECONDS
and rec.get("fp") == _fingerprint(root / name)
and isinstance(blockers, list) and bool(blockers)
and not all(_accessible(b) for b in blockers if isinstance(b, str))
)
def _read(root: Path) -> dict[str, dict]:
# A torn or hand-edited file reads as empty: the worst case is one re-walk and one line.
try:
data = json.loads(_state_path(root).read_text(encoding="utf-8-sig"))
data = json.loads((root / STUCK_STATE_NAME).read_text(encoding="utf-8-sig"))
except (OSError, ValueError):
return {}
return {k: v for k, v in data.items() if isinstance(v, dict)} if isinstance(data, dict) else {}
def _write(root: Path, state: dict[str, dict]) -> None:
path = _state_path(root)
with contextlib.suppress(OSError):
if not state:
"""Publish *state* via a unique temp file + ``os.replace``: a reader sees old or new, never torn."""
path = root / STUCK_STATE_NAME
if not state:
with contextlib.suppress(OSError):
path.unlink(missing_ok=True)
return
tmp = path.with_name(path.name + ".tmp")
tmp.write_text(json.dumps(state, indent=1, sort_keys=True), encoding="utf-8")
return
try:
fd, tmp = tempfile.mkstemp(dir=root, prefix=STUCK_STATE_NAME + ".", suffix=".tmp")
except OSError:
return
try:
with os.fdopen(fd, "w", encoding="utf-8") as f:
json.dump(state, f, indent=1, sort_keys=True)
os.replace(tmp, path)
except OSError:
with contextlib.suppress(OSError):
os.unlink(tmp)
def load_skips(root: Path, *, persist: bool = True) -> tuple[dict[str, dict], frozenset[str]]:
"""``(state, names to skip)``. Records whose entry vanished or whose fingerprint changed
(someone touched or partly cleaned it) are dropped, so that entry is judged normally."""
if not tracking_supported():
return {}, frozenset()
state = _read(root)
live = {name: rec for name, rec in state.items() if rec.get("fp") == _fingerprint(root / name)}
if persist and live != state:
_write(root, live)
return live, frozenset(live)
@contextlib.contextmanager
def _locked(root: Path):
"""Exclusive cross-process lock around a read-merge-write of the state file. Two pruners of
one home (separate processes past the ``.last_prune`` throttle together) otherwise overwrite
each other's records from stale snapshots. POSIX only, like the tracking itself."""
import fcntl
fd = os.open(root / STUCK_LOCK_NAME, os.O_RDWR | os.O_CREAT, 0o600)
try:
fcntl.flock(fd, fcntl.LOCK_EX)
yield
finally:
os.close(fd) # closing the descriptor releases the flock
def rmtree_collecting_permission_errors(entry: Path) -> bool:
"""``rmtree`` that keeps going past errors (like ``ignore_errors=True``) and returns True
when any of them was EACCES/EPERM, i.e. what is left is not removable by this user."""
denied = False
def load_skips(root: Path, *, persist: bool = True) -> frozenset[str]:
"""Names to skip this pass. Records that no longer hold (see module doc) are dropped, so
that entry is judged normally again."""
if not tracking_supported() or not (root / STUCK_STATE_NAME).exists():
return frozenset()
now = time.time()
if not persist:
state = _read(root)
return frozenset(n for n, rec in state.items() if _still_stuck(root, n, rec, now))
try:
with _locked(root):
state = _read(root)
live = {n: rec for n, rec in state.items() if _still_stuck(root, n, rec, now)}
if live != state:
_write(root, live)
except OSError:
return frozenset()
return frozenset(live)
def _onexc(_func, _path, exc):
nonlocal denied
if isinstance(exc, OSError) and exc.errno in _PERMISSION_ERRNOS:
denied = True
def rmtree_collecting_permission_errors(entry: Path) -> list[str]:
"""``rmtree`` that keeps going past errors (like ``ignore_errors=True``) and returns the
directories that refused access with EACCES/EPERM (empty: nothing was denied)."""
blockers: list[str] = []
def _onexc(func, path, exc):
if is_permission_error(exc):
# scandir/open of a dir fails on the dir itself; unlink/rmdir on its parent's perms.
blocker = path if func in (os.scandir, os.open, os.lstat) else os.path.dirname(path)
if blocker not in blockers:
blockers.append(blocker)
if sys.version_info >= (3, 12):
shutil.rmtree(entry, onexc=_onexc)
else: # pragma: no cover - 3.11
shutil.rmtree(entry, onerror=lambda f, p, info: _onexc(f, p, info[1]))
return denied
return blockers
def removal_hint(entry: Path) -> str:
from hermes_platform.resolver import locate_command
hint = f"sudo rm -rf {shlex.quote(os.fspath(entry))}"
if shutil.which("docker"):
if locate_command("docker").command:
hint += (
f" (or without sudo: docker run --rm -v {shlex.quote(os.fspath(entry.parent) + ':/s')}"
f" alpine rm -rf {shlex.quote('/s/' + entry.name)})"
@@ -103,22 +169,24 @@ def removal_hint(entry: Path) -> str:
return hint
def record(root: Path, state: dict[str, dict], entry: Path, audit) -> None:
def record(root: Path, entry: Path, blockers: list[str], audit) -> None:
"""Remember *entry* as not removable by this user and log the one actionable line."""
fp = _fingerprint(entry)
if fp is None:
if fp is None or not blockers:
return
state[entry.name] = {"fp": fp, "first_seen": time.time()}
_write(root, state)
rec = {"fp": fp, "since": time.time(), "uid": os.geteuid(), "blockers": blockers[:_MAX_BLOCKERS]} # windows-footgun: ok — only reached when tracking_supported()
with contextlib.suppress(OSError), _locked(root):
state = _read(root) # re-read under the lock: merge, never publish a stale snapshot
state[entry.name] = rec
_write(root, state)
audit.info(
"scratch prune: cannot remove %r: it holds files this user (uid %d) may not delete, "
"usually root-owned output of a Docker container; skipping it until it changes. "
"Remove it with: %s",
os.fspath(entry), os.getuid(), removal_hint(entry), # windows-footgun: ok — only reached when tracking_supported()
"scratch prune: cannot remove %r: it holds files this user (uid %d) may not read or "
"delete, usually root-owned output of a Docker container; skipping it until that is "
"fixed (re-checked weekly). Remove it with: %s",
os.fspath(entry), rec["uid"], removal_hint(entry),
)
def stuck_entries(root: Path) -> list[Path]:
"""Entries the prune is currently skipping as not removable (for ``hermes doctor``)."""
_state, names = load_skips(root, persist=False)
return [root / name for name in sorted(names)]
return [root / name for name in sorted(load_skips(root, persist=False))]
+76 -30
View File
@@ -1,5 +1,6 @@
"""An idle scratch entry this user cannot fully delete is reported once and not re-walked hourly."""
"""An idle scratch entry this user cannot read or delete is reported once and not re-walked hourly."""
import json
import os
import stat
import time
@@ -8,18 +9,21 @@ import pytest
import hermes_constants_scratch as scratch
pytestmark = pytest.mark.skipif(
not hasattr(os, "getuid") or os.getuid() == 0,
reason="needs a POSIX non-root user: root deletes through a read-only dir",
)
pytestmark = [
pytest.mark.platforms("posix"),
pytest.mark.skipif(
not hasattr(os, "getuid") or os.getuid() == 0,
reason="needs a non-root user: root deletes through a read-only dir",
),
]
OLD = time.time() - 48 * 3600
def _undeletable_entry(root):
"""``lane/ro/blob`` with ``ro`` mode 0500: rmtree removes everything else and fails
def _undeletable_entry(root, name="lane"):
"""``<name>/ro/blob`` with ``ro`` mode 0500: rmtree removes everything else and fails
on ``blob`` with EACCES, the same residue a root-owned container write leaves."""
entry = root / "lane"
entry = root / name
ro = entry / "ro"
ro.mkdir(parents=True)
(entry / "deletable.txt").write_text("x", encoding="utf-8")
@@ -30,55 +34,97 @@ def _undeletable_entry(root):
return entry, ro
def _walk_spy(monkeypatch):
def _walk_spy(monkeypatch, root):
"""Every directory listing under *root* other than root itself (idle scan, size walk, rmtree)."""
walked = []
real_bytes, real_state = scratch._tree_bytes, scratch.subtree_touch_state
monkeypatch.setattr(scratch, "_tree_bytes", lambda p: walked.append(p) or real_bytes(p))
monkeypatch.setattr(scratch, "subtree_touch_state", lambda p, c: walked.append(p) or real_state(p, c))
real_scandir = os.scandir
def scandir(path=".", *a, **k):
if isinstance(path, (str, os.PathLike)) and os.fspath(path).startswith(str(root) + os.sep):
walked.append(os.fspath(path))
return real_scandir(path, *a, **k)
monkeypatch.setattr(scratch.os, "scandir", scandir)
monkeypatch.setattr(scratch, "reap_processes_rooted_in", lambda *_a, **_k: 0)
return walked
def test_undeletable_entry_logged_once_then_skipped_without_walk(tmp_path, monkeypatch):
def _lines(log_file, entry):
return [ln for ln in log_file.read_text(encoding="utf-8-sig").splitlines() if repr(str(entry)) in ln]
def _walked(walked, entry):
return any(p == str(entry) or p.startswith(str(entry) + os.sep) for p in walked)
def test_undeletable_or_unreadable_entry_logged_once_then_skipped_without_walk(tmp_path, monkeypatch):
root = tmp_path / "scratch"
entry, ro = _undeletable_entry(root)
# A dir this user may not even list (root-owned 0700): the idle scan itself fails.
sealed = root / "sealed"
locked = sealed / "locked"
locked.mkdir(parents=True)
(locked / "blob").write_text("unreadable", encoding="utf-8")
for p in (locked / "blob", locked, sealed):
os.utime(p, (OLD, OLD))
locked.chmod(0)
log_file = tmp_path / "logs" / "scratch-prune.log"
walked = _walk_spy(monkeypatch)
walked = _walk_spy(monkeypatch, root)
try:
assert scratch.prune_idle_entries(root, 24, frozenset(), log_file) == 0
assert entry in walked
assert _walked(walked, entry) and _walked(walked, sealed)
walked.clear()
# A day later: pass 1's partial delete no longer counts as recent activity, so on
# main the entry is selected, walked and logged again (and again every hour after).
# main both are walked again (and the sealed one never gets a hint) every hour.
later = time.time() + 48 * 3600
monkeypatch.setattr(scratch.time, "time", lambda: later)
assert scratch.prune_idle_entries(root, 24, frozenset(), log_file) == 0
assert entry not in walked, "a known-undeletable entry must not be re-walked every pass"
lines = [ln for ln in log_file.read_text(encoding="utf-8-sig").splitlines() if repr(str(entry)) in ln]
assert len(lines) == 1, lines
assert "sudo rm -rf" in lines[0] and str(entry) in lines[0], lines[0]
for e in (entry, sealed):
assert not _walked(walked, e), f"a known-stuck entry must not be re-walked every pass: {e}"
lines = _lines(log_file, e)
assert len(lines) == 1, lines
assert f"sudo rm -rf {e}" in lines[0], lines[0]
assert (ro / "blob").exists()
finally:
ro.chmod(0o700)
locked.chmod(0o700)
def test_changed_or_fixed_entry_is_judged_again(tmp_path, monkeypatch):
def test_fixed_expired_or_other_uid_entry_is_judged_again(tmp_path, monkeypatch):
root = tmp_path / "scratch"
entry, ro = _undeletable_entry(root)
other, other_ro = _undeletable_entry(root, "other")
log_file = tmp_path / "logs" / "scratch-prune.log"
walked = _walk_spy(monkeypatch)
walked = _walk_spy(monkeypatch, root)
now = time.time()
try:
assert scratch.prune_idle_entries(root, 24, frozenset(), log_file) == 0
# The one line tells the user how to clear it.
assert f"sudo rm -rf {entry}" in log_file.read_text(encoding="utf-8-sig")
# The user fixes the permissions; the entry's own mtime changing is what un-skips it.
# The user fixes the nested dir only; the top dir's own metadata is not touched.
top_before = os.lstat(entry)
ro.chmod(0o700)
os.utime(entry, (OLD + 60, OLD + 60))
assert os.lstat(entry).st_ctime_ns == top_before.st_ctime_ns
walked.clear()
monkeypatch.setattr(scratch.time, "time", lambda: now + 48 * 3600)
assert scratch.prune_idle_entries(root, 24, frozenset(), log_file) == 1
assert entry in walked
assert not entry.exists()
assert not (root / ".prune_stuck.json").exists()
assert not _walked(walked, other)
# Still stuck after a week: re-checked once (bounded retry) and reported again.
monkeypatch.setattr(scratch.time, "time", lambda: now + 9 * 24 * 3600)
assert scratch.prune_idle_entries(root, 24, frozenset(), log_file) == 0
assert _walked(walked, other) and len(_lines(log_file, other)) == 2
# A record made under another uid is not trusted either.
state_file = root / ".prune_stuck.json"
state = json.loads(state_file.read_text(encoding="utf-8-sig"))
state["other"]["uid"] += 1 # a record made by another uid
state_file.write_text(json.dumps(state), encoding="utf-8")
walked.clear()
assert scratch.prune_idle_entries(root, 24, frozenset(), log_file) == 0
assert _walked(walked, other)
other_ro.chmod(0o700)
monkeypatch.setattr(scratch.time, "time", lambda: now + 11 * 24 * 3600)
assert scratch.prune_idle_entries(root, 24, frozenset(), log_file) == 1
assert not state_file.exists()
finally:
if ro.exists():
ro.chmod(0o700)
for d in (ro, other_ro):
if d.exists():
d.chmod(0o700)
+7 -4
View File
@@ -271,10 +271,13 @@ still their child) is never stopped: an idle entry one of them is working inside
instead. Each entry is re-checked right before it is deleted, so
one written to while the prune was stopping processes is kept. Every removal, stopped
process and kept entry is recorded in `~/.hermes/logs/scratch-prune.log`. An entry holding
files your user may not delete (typically root-owned output of a Docker container that wrote
into a bind mount) is logged once with the command that removes it (`sudo rm -rf '<path>'`, or
a `docker run … rm -rf` alternative when Docker is installed) and then skipped, not re-scanned
every hour, until it changes or is gone; Hermes never runs that command itself. `hermes doctor`
files your user may not read or delete (typically root-owned output of a Docker container that
wrote into a bind mount) is never deleted partially-blind: it is logged once with the command that
removes it (`sudo rm -rf '<path>'`, or a `docker run … rm -rf` alternative when Docker is
installed) and then skipped, not re-scanned every hour, until it is gone, its top directory
changes, or the directories that refused access become accessible to you (for example after
`sudo chown -R`); it is also re-checked once a week regardless. Hermes never runs that command
itself. `hermes doctor`
reports the directory, its size and any such skipped entries, and
warns about directories over 1 GB elsewhere under `cache/` that no pruner covers.