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)
This commit is contained in:
赵桂雄
+141
-9
@@ -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,18 +190,144 @@ 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 <libproc.h>. ``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("<i", listing.raw, offset)[0]
|
||||
record = ctypes.create_string_buffer(_DARWIN_FD_RECORD_SIZE)
|
||||
if lib.proc_pidfdinfo(pid, fd, _DARWIN_PIDFDVNODEPATHINFO, record,
|
||||
_DARWIN_FD_RECORD_SIZE) <= 0:
|
||||
continue
|
||||
raw = record.raw
|
||||
identity = (struct.unpack_from("<I", raw, _DARWIN_FD_DEV_OFFSET)[0],
|
||||
struct.unpack_from("<Q", raw, _DARWIN_FD_INO_OFFSET)[0])
|
||||
target = raw[_DARWIN_FD_PATH_OFFSET:].split(b"\x00", 1)[0].decode("utf-8", "replace")
|
||||
yield pid, fd, target, identity
|
||||
|
||||
|
||||
def _iter_darwin_sidecar_holders(db_path, *, own_pid: int, skip_fds=frozenset()) -> 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/<pid>/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()
|
||||
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
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user