From 68b10bbbf9cc70d041cd910117199bd40fcbe248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B5=B5=E6=A1=82=E9=9B=84?= Date: Sun, 13 Sep 2026 15:10:08 +0800 Subject: [PATCH] fix(state): enumerate deleted-WAL sidecar holders on macOS via libproc iter_deleted_sqlite_sidecar_holders() returned [] on every non-Linux platform, so refuse_deleted_wal_generation() -- the pre-connect refusal that stops a second opener from minting a replacement WAL under a live writer -- was a permanent no-op on macOS. The reporter of #109641 hit exactly that: after an update/restart took the sidecars away, a fresh opener minted a new generation at the path, the still-live writer's next write raised DeletedWalGenerationError, and each event copied the whole database (16 halts / 13 minutes / 4.2 GB of captures). macOS has no /proc and never reports a " (deleted)" suffix, which is why the scan was restricted to Linux, but libproc does describe other processes' descriptors: proc_pidinfo(PROC_PIDLISTFDS) lists a process's fds and proc_pidfdinfo(PROC_PIDFDVNODEPATHINFO) returns each vnode fd's (st_dev, st_ino) plus the vnode's last pathname -- for same-user processes, without elevation. Both survive unlink, which is also why psutil.Process.open_files() cannot stand in for it (it hides unlinked descriptors, so the retired generation is structurally invisible). The judgement itself is unchanged and now shared: a descriptor counts only when it names a watched sidecar path while its identity no longer matches what that path holds, i.e. _fd_is_truly_unlinked()'s identity test (#108082), never a path suffix or a link count. Only the source of that identity differs per platform -- readlink on /proc for Linux, libproc for macOS -- and the darwin side resolves symlinks before comparing paths because libproc reports the kernel's path (/private/var/... where the caller opened /var/...). Scope is this one function: the enumeration legs, the gate (Windows still returns [] -- it cannot unlink a held sidecar) and the stale docstring reason. Enumeration failures keep the existing fail-open behaviour (logged at debug, no holders), and the new tests are marked macos_only so the existing Linux-only ones stay untouched. Cost, measured on macOS 26.4 (darwin 25.4.0) with 721 processes / 4383 vnode descriptors: ~20 ms per full enumeration, versus the Linux leg's ~11 ms / ~4.4k syscalls measured in #108910 -- the same order, paid once per open, on the platform where the guard previously did nothing at all. (cherry picked from commit f1501dfe7141e3c2521c5192857c37d7b60a922b) --- hermes_state_dbfile.py | 164 ++++++++++++++++-- .../test_deleted_wal_generation_guard.py | 56 ++++++ 2 files changed, 204 insertions(+), 16 deletions(-) diff --git a/hermes_state_dbfile.py b/hermes_state_dbfile.py index 2c2302e559..a4d7dc8107 100644 --- a/hermes_state_dbfile.py +++ b/hermes_state_dbfile.py @@ -144,6 +144,16 @@ def _watched_sqlite_sidecar_paths(db_path) -> Dict[str, str]: return {_canonical_sqlite_path(path): path for path in literal} +def _identity_is_truly_unlinked(identity: "Tuple[int, int]", watched_path: str) -> bool: + """The shared verdict: does ``(st_dev, st_ino)`` name a generation the watched path no longer + holds? See :func:`_fd_is_truly_unlinked` for why the test is identity and never link count.""" + try: + watched_stat = os.stat(watched_path) + except OSError: + return True + return identity != (watched_stat.st_dev, watched_stat.st_ino) + + def _fd_is_truly_unlinked(fd_path: str, watched_path: str) -> bool: """Confirm a `` (deleted)`` /proc fd target really names an orphaned generation, not the CURRENT watched sidecar. @@ -161,11 +171,7 @@ def _fd_is_truly_unlinked(fd_path: str, watched_path: str) -> bool: fd_stat = os.stat(fd_path) except OSError: return True - try: - watched_stat = os.stat(watched_path) - except OSError: - return True - return (fd_stat.st_dev, fd_stat.st_ino) != (watched_stat.st_dev, watched_stat.st_ino) + return _identity_is_truly_unlinked((fd_stat.st_dev, fd_stat.st_ino), watched_path) def _iter_proc_fd_targets(): @@ -184,25 +190,151 @@ def _iter_proc_fd_targets(): yield int(pid_str), os.readlink(fd_path), fd_path +# ── macOS fd enumeration (libproc) ────────────────────────────────────────────────────────────── +# +# macOS has no ``/proc`` and never reports a `` (deleted)`` suffix, but libproc can still describe +# ANOTHER process's descriptors: ``proc_listpids`` names the processes, ``proc_pidinfo +# (PROC_PIDLISTFDS)`` lists one process's fds, and ``proc_pidfdinfo(PROC_PIDFDVNODEPATHINFO)`` +# returns, for a vnode fd, its ``(st_dev, st_ino)`` together with the vnode's last pathname. Both +# survive unlink (the path keeps its last name, ``st_nlink`` drops to 0), for same-user processes +# without elevation — which is the darwin counterpart of the Linux readlink leg, and the reason +# ``psutil.Process.open_files()`` cannot stand in for it (it hides unlinked descriptors). The +# identity judgement is shared (:func:`_identity_is_truly_unlinked`); only its source differs. +_DARWIN_ALL_PIDS = 1 +_DARWIN_PIDLISTFDS = 1 +_DARWIN_PIDFDVNODEPATHINFO = 2 +_DARWIN_PROC_FD_INFO_SIZE = 8 # struct proc_fdinfo { int32 proc_fd; uint32 proc_fdtype; } +_DARWIN_FD_RECORD_SIZE = 1200 # PROC_PIDFDVNODEPATHINFO_SIZE +# Field offsets inside that 1200-byte record. libproc does not lay the header's structs out +# verbatim (it pads ``vnode_info`` and sizes the record to 1200, not to ``sizeof``'s 1192), so +# these are pinned against live descriptors rather than derived from . ``vst_dev`` / +# ``vst_ino`` are the same two fields the Linux leg compares; the path is the vnode's last +# pathname, resolved by the kernel (``/private/var/...`` where the caller opened ``/var/...``). +_DARWIN_FD_DEV_OFFSET = 24 +_DARWIN_FD_INO_OFFSET = 32 +_DARWIN_FD_PATH_OFFSET = 176 +_DARWIN_LIBPROC = None + + +def _darwin_libproc(): + """libproc's fd-enumeration entry points, loaded once per process. + + macOS exports these from libSystem, so this process's own handle resolves them; a failure + here propagates to the caller's ``except Exception`` and keeps the guard fail-open.""" + global _DARWIN_LIBPROC + lib = _DARWIN_LIBPROC + if lib is not None: + return lib + import ctypes + + lib = ctypes.CDLL(None, use_errno=True) + lib.proc_listpids.restype = ctypes.c_int + lib.proc_listpids.argtypes = (ctypes.c_uint32, ctypes.c_uint32, ctypes.c_void_p, ctypes.c_uint32) + lib.proc_pidinfo.restype = ctypes.c_int + lib.proc_pidinfo.argtypes = (ctypes.c_int, ctypes.c_int, ctypes.c_uint64, ctypes.c_void_p, ctypes.c_int) + lib.proc_pidfdinfo.restype = ctypes.c_int + lib.proc_pidfdinfo.argtypes = (ctypes.c_int, ctypes.c_int, ctypes.c_int, ctypes.c_void_p, ctypes.c_int) + _DARWIN_LIBPROC = lib + return lib + + +def _darwin_all_pids(lib) -> List[int]: + """Every pid ``proc_listpids`` will name (the kernel silently omits ones we may not inspect).""" + import ctypes + + size = 4096 * 8 + while True: + buffer = ctypes.create_string_buffer(size) + used = lib.proc_listpids(_DARWIN_ALL_PIDS, 0, buffer, size) + if used <= 0: + return [] + if used < size: + return [pid for pid in struct.unpack_from(f"<{used // 4}i", buffer.raw) if pid > 0] + size *= 2 + + +def _iter_darwin_fd_targets(): + """Yield ``(pid, fd, last pathname, (st_dev, st_ino))`` for every vnode fd libproc reports. + + The pathname and the identity both stay readable after the path is unlinked, which is what + makes an orphaned WAL generation visible at all on macOS. Processes that cannot be inspected + (gone, or not ours) and descriptors that are not vnodes are skipped silently.""" + import ctypes + + lib = _darwin_libproc() + for pid in _darwin_all_pids(lib): + size = 4096 + while True: + listing = ctypes.create_string_buffer(size) + used = lib.proc_pidinfo(pid, _DARWIN_PIDLISTFDS, 0, listing, size) + if used <= 0: + break # process gone, or not ours to inspect + if used < size: + break + size *= 2 + else: + continue + for offset in range(0, used - _DARWIN_PROC_FD_INFO_SIZE + 1, _DARWIN_PROC_FD_INFO_SIZE): + fd = struct.unpack_from(" List[Tuple[int, str]]: + """The macOS leg of :func:`iter_deleted_sqlite_sidecar_holders`: libproc enumeration matched + against the watched sidecar paths, judged by identity. + + The watched side is resolved with ``os.path.realpath`` because libproc reports the kernel's + path for the vnode, while ``os.path.abspath`` does not resolve symlinks -- a textual compare + of the two silently misses every sidecar under a symlinked prefix (on macOS ``/var`` itself).""" + base = os.path.realpath(os.path.abspath(os.fspath(db_path))) + watched = {os.path.normcase(path): path for path in (base + "-wal", base + "-shm")} + holders: List[Tuple[int, str]] = [] + for pid, fd, target, identity in _iter_darwin_fd_targets(): + if pid == own_pid and fd in skip_fds: + continue # our lock guard's descriptor (hermes_state_lockguard), not a SQLite connection + literal = watched.get(os.path.normcase(target)) + if literal is not None and _identity_is_truly_unlinked(identity, literal): + holders.append((pid, target)) + return holders + + def iter_deleted_sqlite_sidecar_holders(db_path) -> List[Tuple[int, str]]: - """Return processes holding an unlinked ``state.db-wal`` / ``-shm``. Linux-only; ``[]`` - elsewhere (Windows cannot unlink a held sidecar, macOS has no `` (deleted)`` suffix). + """Return processes holding an unlinked ``state.db-wal`` / ``-shm`` sidecar for *db_path*. + + Linux enumerates ``/proc//fd`` (using the `` (deleted)`` suffix as a cheap pre-filter); + macOS enumerates descriptors through libproc, because a retired generation there keeps its + last pathname and no suffix marks it. Either way the verdict is the same identity comparison: + the fd must name a watched sidecar path while its ``(st_dev, st_ino)`` no longer matches what + that path holds. Windows returns ``[]`` -- it cannot unlink a held sidecar, so no retired + generation can exist; any other platform returns ``[]`` as before. + Includes this process: on the open/write refuse path the in-process writer holding the orphan inode must not mint a replacement WAL (``_foreign_state_db_holders`` skips this PID).""" - if not sys.platform.startswith("linux"): + if sys.platform == "win32": return [] holders: List[Tuple[int, str]] = [] - watched = _watched_sqlite_sidecar_paths(db_path) try: from hermes_state_lockguard import owned_fds own_pid, guard_fds = os.getpid(), owned_fds() - for pid, target, fd_path in _iter_proc_fd_targets(): - if pid == own_pid and int(fd_path.rsplit("/", 1)[1]) in guard_fds: - continue # our lock guard's descriptor, not a connection on a dead generation - canonical = _canonical_sqlite_path(target) - if (" (deleted)" in target and canonical in watched - and _fd_is_truly_unlinked(fd_path, watched[canonical])): - holders.append((pid, target)) + if sys.platform == "darwin": + holders = _iter_darwin_sidecar_holders(db_path, own_pid=own_pid, skip_fds=guard_fds) + elif sys.platform.startswith("linux"): + watched = _watched_sqlite_sidecar_paths(db_path) + for pid, target, fd_path in _iter_proc_fd_targets(): + if pid == own_pid and int(fd_path.rsplit("/", 1)[1]) in guard_fds: + continue # our lock guard's descriptor, not a connection on a dead generation + canonical = _canonical_sqlite_path(target) + if (" (deleted)" in target and canonical in watched + and _fd_is_truly_unlinked(fd_path, watched[canonical])): + holders.append((pid, target)) except Exception as exc: logger.debug("deleted-WAL holder scan failed for %s: %s", db_path, exc) return holders diff --git a/tests/hermes_state/test_deleted_wal_generation_guard.py b/tests/hermes_state/test_deleted_wal_generation_guard.py index 914643d25d..a5ab040822 100644 --- a/tests/hermes_state/test_deleted_wal_generation_guard.py +++ b/tests/hermes_state/test_deleted_wal_generation_guard.py @@ -455,3 +455,59 @@ def test_refuse_helper_raises_while_deleted_wal_held(tmp_path, force_wal): assert not wal.exists() finally: raw.close() + + +# ── macOS leg (#109641): the same holder scan through libproc ─────────────────────────────────── +# +# macOS has no /proc and no `` (deleted)`` suffix, so these exercise the libproc enumeration +# (``proc_pidinfo(PROC_PIDLISTFDS)`` + ``proc_pidfdinfo(PROC_PIDFDVNODEPATHINFO)``) that supplies +# each descriptor's ``(st_dev, st_ino)``. The judgement is unchanged: identity, never path text. + +@pytest.mark.macos_only +def test_iter_finds_self_after_wal_unlink_on_darwin(tmp_path, force_wal): + path = tmp_path / "state.db" + db = make_db(path, "s", "held") + wal = require_wal(db) + inode_before = wal.stat().st_ino + lose_sidecars(path, rename=False) + holders = iter_deleted_sqlite_sidecar_holders(path) + try: + assert holders, "expected this process to still hold the deleted WAL inode" + assert any(pid == os.getpid() for pid, _target in holders) + assert any(target.endswith(("-wal", "-shm")) for _pid, target in holders) + assert not wal.exists() or wal.stat().st_ino != inode_before + finally: + db.close() + + +@pytest.mark.macos_only +def test_iter_finds_no_holder_while_sidecars_stay_linked_on_darwin(tmp_path, force_wal): + """The scan must not report the CURRENT generation: a linked sidecar is not a retired one.""" + path = tmp_path / "state.db" + db = make_db(path, "s", "linked") + require_wal(db) + try: + assert iter_deleted_sqlite_sidecar_holders(path) == [] + finally: + db.close() + + +@pytest.mark.macos_only +def test_iter_darwin_judges_by_identity_not_by_pathname(tmp_path): + """A retired generation stays a holder after the path names a DIFFERENT inode: libproc reports + the vnode's last pathname with no `` (deleted)`` marker, so only ``(st_dev, st_ino)`` can tell + the orphan apart from the replacement that now owns the path.""" + path = tmp_path / "state.db" + sidecar = Path(str(path) + "-wal") + sidecar.write_bytes(b"retired generation") + held = sidecar.open("rb") + try: + retired_ino = os.fstat(held.fileno()).st_ino + sidecar.unlink() + sidecar.write_bytes(b"replacement generation") + assert sidecar.stat().st_ino != retired_ino + holders = iter_deleted_sqlite_sidecar_holders(path) + assert any(pid == os.getpid() for pid, _target in holders) + assert any(target == os.path.realpath(str(sidecar)) for _pid, target in holders) + finally: + held.close()