From 99d3b268a02b4dc364e3773c3eb7138783d7de3a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 23 Oct 2026 18:28:20 +0000 Subject: [PATCH] revert(bigtable): drop the stale-tempdir sweep Change-Id: I9a54c012f86ad1562da507e0cd415f6befd644ed --- .../bigtable/data/_accelerator/_daemon.py | 226 +-------- .../unit/data/test_accelerator_daemon.py | 459 ------------------ 2 files changed, 15 insertions(+), 670 deletions(-) diff --git a/packages/google-cloud-bigtable/google/cloud/bigtable/data/_accelerator/_daemon.py b/packages/google-cloud-bigtable/google/cloud/bigtable/data/_accelerator/_daemon.py index 598164d8a335..5e341d9c76fa 100644 --- a/packages/google-cloud-bigtable/google/cloud/bigtable/data/_accelerator/_daemon.py +++ b/packages/google-cloud-bigtable/google/cloud/bigtable/data/_accelerator/_daemon.py @@ -23,7 +23,6 @@ from __future__ import annotations -import glob import json import math import os @@ -32,7 +31,6 @@ import socket import subprocess import tempfile -import threading import time from typing import Any, Mapping, Sequence @@ -52,26 +50,23 @@ _IDENTITY_FILENAME = "identity.json" # Prefix for the per-daemon tempdir (created by ``tempfile.mkdtemp``) that holds -# the socket, identity, and log. Used both when creating our own and to scope -# the startup sweep of leftovers. +# the socket, identity, and log. +# +# ``close()`` removes this directory; a SIGKILL of *this* process leaves it +# behind. We deliberately do not sweep other processes' leftovers at startup. +# What survives such a crash is only ``daemon.log`` and, if the client died +# before reading it, ``identity.json`` -- a few KB of dead files. There is no +# stray process and no stray socket: the daemon watches its parent PID (and +# its stdin) and exits within ~500ms of this process dying, removing its own +# socket on the way out. Reclaiming the rest would mean deleting files we do +# not own in a world-writable directory, judged by guessing at the liveness of +# processes in other namespaces -- a large hazard for a trivial saving, when +# the OS already clears /tmp on its own schedule. _TEMPDIR_PREFIX = "bt-accel-" # Names of the files a live daemon keeps inside its tempdir. _SOCKET_FILENAME = "sock" _LOG_FILENAME = "daemon.log" -# Absolute POSIX timestamp written by start() so the sweep knows when startup -# for *this specific* daemon was allowed to finish. Each process may have a -# different BIGTABLE_ACCELERATOR_STARTUP_TIMEOUT, so the sweep cannot rely on -# its own env value when judging another process's tempdir. -_STARTUP_DEADLINE_FILENAME = "startup_deadline" -# Identity of the Python process that spawned the daemon, written at start() -# as ``" "``. The sweep signals this PID to decide whether -# the spawner is still alive: if it is, the daemon's stdin is open and the -# daemon is (or is about to be) serving; if it is gone, the daemon's stdin is -# closed and it has exited or is exiting. See ``_sweep_stale_tempdirs`` for -# why the namespace is recorded next to the number. -_PID_FILENAME = "daemon.pid" - # How long to wait for the daemon to start listening on its UDS before giving # up at startup. Cold, constrained environments (e.g. Cloud Run's gVisor # sandbox) spend ~10s in Go runtime init + ADC token fetch from the metadata @@ -86,29 +81,12 @@ _SIGTERM_GRACE_SECONDS = 2.0 _SIGKILL_GRACE_SECONDS = 2.0 -# Grace period past a tempdir's startup deadline before the sweep will judge -# it. A daemon that has not bound by its own deadline has failed, but give it -# this much slack anyway so a daemon another client is mid-way through -# starting is never mistaken for a stale leftover and reaped out from under it. -_STALE_SWEEP_MIN_AGE_SECONDS = 60.0 - -# Written in place of the PID namespace on platforms that do not have one to -# report (no /proc, e.g. macOS). Comparing "none" against "none" is a match -- -# where namespaces do not exist, PIDs cannot be ambiguous between them. -_NO_PID_NAMESPACE = "-" - # How long a single connect() probe of a daemon's socket may take. A UDS # connect against a listening socket completes in the kernel without the # daemon doing anything, so this only ever elapses on a badly overloaded # host — where the honest answer is "I cannot tell", not "it is dead". _SOCKET_PROBE_TIMEOUT_SECONDS = 0.25 -# Upper bound on how long a single sweep may spend walking candidates. The -# sweep runs on a fire-and-forget thread, so nothing is waiting on it; this -# only stops a pathological temp directory (thousands of leftovers, a slow -# network filesystem) from keeping the thread alive indefinitely. -_SWEEP_BUDGET_SECONDS = 5.0 - def _resolve_binary_path() -> str: """Return the path of the bundled daemon binary, raising if it is absent.""" @@ -152,9 +130,7 @@ def _socket_is_live(uds_path: str) -> bool: """Return True if something is currently accepting connections on the UDS. Used by ``_wait_until_ready`` to poll for the daemon coming up, so every - failure simply answers "not yet" and the caller tries again. Note that - this is a *measurement*, which is why the sweep does not use it to decide - whether a daemon has died -- see ``_sweep_stale_tempdirs``. + failure simply answers "not yet" and the caller tries again. """ if not os.path.exists(uds_path): return False @@ -167,132 +143,6 @@ def _socket_is_live(uds_path: str) -> bool: return False -def _pid_namespace() -> str: - """Identify the PID namespace this process's PIDs are numbered in. - - A PID is only meaningful inside the namespace that issued it, but the - tempdir holding it can be shared across namespaces -- two containers with - the same ``/tmp`` volume, or a host ``/tmp`` bind-mounted into one. The - sweep records this alongside the PID so it can tell whether a number it - reads is one it is entitled to interpret. - - Returns ``_NO_PID_NAMESPACE`` where there is nothing to report, which - compares equal to itself: on a platform with no PID namespaces, PIDs - cannot be ambiguous between them. - """ - try: - st = os.stat("/proc/self/ns/pid") - except OSError: - return _NO_PID_NAMESPACE - return f"{st.st_dev}:{st.st_ino}" - - -def _write_metadata_file(path: str, contents: str) -> None: - """Write ``contents`` to ``path`` so a reader never sees a partial file. - - The sweep reads these files out of *other* processes' tempdirs, possibly - while they are still being written. A half-written pid file would be - unreadable or, worse, parse as some other process's PID, and a truncated - deadline could parse as a plausible but wrong time. Writing to a sibling - temp name and renaming into place makes the swap atomic: a reader sees - either no file at all or the complete contents. A leftover ``.tmp`` from - a process killed mid-write goes away with the tempdir. - """ - tmp_path = path + ".tmp" - with open(tmp_path, "w") as _f: - _f.write(contents) - os.replace(tmp_path, path) - - -def _sweep_stale_tempdirs() -> None: - """Best-effort reaping of leftover daemon tempdirs at startup. - - A clean ``close()`` removes its own tempdir; a crash or ``SIGKILL`` leaves - the socket and log behind. We tidy those up on the next start. - - Best-effort in both directions, and that is the whole design: - - * It never breaks startup. Every failure is swallowed, the walk abandons - whatever it has not reached once ``_SWEEP_BUDGET_SECONDS`` is up, and it - runs on a thread nobody waits on. - * It never deletes without proof. The only door to ``rmtree`` is - ``ProcessLookupError`` from ``os.kill(pid, 0)`` -- the kernel stating no - such process exists. Everything else (unreadable file, truncated write, - ``EPERM``, a PID from another namespace) means "could not establish - death", and skips. - - The asymmetry is deliberate: a missed leftover costs a few KB of /tmp - until the next start() looks again, while deleting a live daemon's - directory destroys the socket it is serving on with no way back. So two - leaks are accepted on purpose -- a recycled PID reads as alive and pins - its directory forever, and a pid file truncated mid-write is skipped - forever. Both are cheap; neither is worth a heuristic that can delete a - live daemon. - - Mechanically: candidates are ``bt-accel-*`` directories containing our - ``daemon.log`` marker, so an unrelated directory sharing the prefix is - never touched. The spawning Python process is the liveness signal -- - while it lives the daemon's stdin is open and the daemon is serving; when - it dies the daemon sees EOF and exits. Each tempdir also records the PID - namespace that numbered it (see :func:`_pid_namespace`), since a shared - ``/tmp`` can cross namespaces and a foreign PID would otherwise give a - confident wrong answer; and its own startup deadline, which covers the - window before the pid file exists and keeps the sweep correct whatever - ``BIGTABLE_ACCELERATOR_STARTUP_TIMEOUT`` the sweeping process has. - """ - if not hasattr(socket, "AF_UNIX"): - return - # Start the clock before the glob: on the pathological temp directory this - # budget exists for, enumerating it is itself the slow part. - now = time.time() - budget_ends = now + _SWEEP_BUDGET_SECONDS - own_namespace = _pid_namespace() - root = tempfile.gettempdir() - try: - logs = glob.iglob(os.path.join(root, _TEMPDIR_PREFIX + "*", _LOG_FILENAME)) - for log_path in logs: - if time.time() >= budget_ends: - return - path = os.path.dirname(log_path) - try: - deadline_path = os.path.join(path, _STARTUP_DEADLINE_FILENAME) - try: - with open(deadline_path) as _f: - deadline = float(_f.read()) - except (OSError, ValueError): - # Older tempdir without a deadline file, or one we could - # not read: fall back to a simple age heuristic relative - # to the log's mtime. This gate only decides *when* to - # look; the pid check below still has to prove death. - deadline = os.path.getmtime(log_path) + _STALE_SWEEP_MIN_AGE_SECONDS - if now < deadline + _STALE_SWEEP_MIN_AGE_SECONDS: - continue - pid_path = os.path.join(path, _PID_FILENAME) - try: - with open(pid_path) as _f: - fields = _f.read().split() - pid = int(fields[0]) - if fields[1] != own_namespace: - # Someone else's numbering. IndexError lands in the - # same place: a tempdir written before the namespace - # was recorded cannot be checked either. - continue - os.kill(pid, 0) - continue # spawner alive → daemon alive → skip - except ProcessLookupError: - pass # ESRCH: spawner is dead → stdin closed → daemon gone - except (OSError, ValueError, IndexError): - # Missing, unreadable, empty or truncated pid file, or - # EPERM from os.kill (the PID exists, it just is not ours - # to signal). None of these is proof of death. - continue - except OSError: - continue - shutil.rmtree(path, ignore_errors=True) - except OSError: - return - - class AcceleratorDaemon: """Manages the Go accelerator daemon subprocess. @@ -400,19 +250,6 @@ def start(self) -> None: "Unix domain sockets (AF_UNIX) are not supported on this platform." ) - # Reap tempdirs leaked by daemons from prior runs that crashed or were - # killed before close() could clean up after them. Fire-and-forget on a - # daemon thread: the walk is only stat()s and signal-0 probes, but it - # scales with the number of leftovers, and start() (which the async - # client calls synchronously, on the event loop thread) must not wait - # on it. The sweep bounds its own runtime; see _SWEEP_BUDGET_SECONDS. - # Even spawning the thread is best-effort: a process already at its - # thread ceiling should still get a working client, not an exception - # from the one step of startup that exists purely to tidy up /tmp. - try: - threading.Thread(target=_sweep_stale_tempdirs, daemon=True).start() - except RuntimeError: - pass self._tempdir = tempfile.mkdtemp(prefix=_TEMPDIR_PREFIX) self._uds_path = os.path.join(self._tempdir, _SOCKET_FILENAME) @@ -428,46 +265,13 @@ def start(self) -> None: # The log file handle only needs to live long enough for Popen to dup # it into the child, so it stays local to start() rather than being an # attribute. Startup failures read the tail back from the path. - log_file = None - try: - # Create the log before anything else can fail. It is the marker - # the sweep keys on, so a crash from here on still leaves a - # directory that a later sweep can recognise and reap. - log_file = open(self._log_path, "wb") - # Record the absolute time by which startup must complete. The - # sweep reads this per-directory, so it judges correctly however - # the *sweeping* process has BIGTABLE_ACCELERATOR_STARTUP_TIMEOUT - # set. Written atomically, since a sweep in another process may - # read it at any moment. - _write_metadata_file( - os.path.join(self._tempdir, _STARTUP_DEADLINE_FILENAME), - repr(time.time() + self._startup_timeout), - ) - # Record who owns this tempdir, as " ". The - # namespace qualifies the number: a sweep running where that PID - # would mean something else entirely can see that and decline to - # judge, rather than confidently signalling an unrelated process. - _write_metadata_file( - os.path.join(self._tempdir, _PID_FILENAME), - f"{os.getpid()} {_pid_namespace()}", - ) - except OSError as exc: - if log_file is not None: - try: - log_file.close() - except OSError: - pass - tempdir = self._tempdir # _cleanup_tempdir() clears the attribute. - self._cleanup_tempdir() - raise RuntimeError( - f"Failed to write accelerator daemon metadata to {tempdir}: {exc}" - ) from exc - argv = [self._binary_path, "--uds-path", self._uds_path, *self._cli_flags] + log_file = None env = None if self._extra_env: env = {**os.environ, **self._extra_env} try: + log_file = open(self._log_path, "wb") self._proc = subprocess.Popen( argv, stdin=subprocess.PIPE, diff --git a/packages/google-cloud-bigtable/tests/unit/data/test_accelerator_daemon.py b/packages/google-cloud-bigtable/tests/unit/data/test_accelerator_daemon.py index 8713d01a7931..2c3fb6a0108e 100644 --- a/packages/google-cloud-bigtable/tests/unit/data/test_accelerator_daemon.py +++ b/packages/google-cloud-bigtable/tests/unit/data/test_accelerator_daemon.py @@ -14,15 +14,8 @@ import json import os -import pathlib import re -import shutil import socket -import subprocess -import sys -import tempfile -import threading -import time import unittest.mock import pytest @@ -51,17 +44,6 @@ def _make_daemon(tmp_path, **kwargs): # directly as _real_cleanup_tempdir(daemon_instance). _real_cleanup_tempdir = AcceleratorDaemon._cleanup_tempdir -# Likewise for tempfile.mkdtemp/gettempdir, which the autouse fixture -# redirects into pytest's tmp_path. The sweep tests need real, short /tmp -# paths: they bind Unix sockets, whose paths are capped at ~107 bytes, and -# pytest's per-test directory names alone eat most of that budget. -_real_mkdtemp = tempfile.mkdtemp -_real_gettempdir = tempfile.gettempdir - -# Distinguishes "caller did not ask for a namespace stamp" (use our own, the -# ordinary case) from "caller asked for no stamp at all" (None). -_UNSET = object() - class TestExtraEnv: def test_default_env_none(self, tmp_path, monkeypatch): @@ -220,11 +202,6 @@ def fake_mkdtemp(prefix=""): monkeypatch.setattr(_daemon.tempfile, "mkdtemp", fake_mkdtemp) - # start() kicks off _sweep_stale_tempdirs on a background thread, and the - # sweep reads gettempdir() rather than going through mkdtemp. Without this - # the suite would glob — and rmtree — real bt-accel-* directories in the - # developer's or CI machine's /tmp, including live ones. - monkeypatch.setattr(_daemon.tempfile, "gettempdir", lambda: str(tmp_path)) yield @@ -308,40 +285,6 @@ def bad_popen(argv, **kwargs): with pytest.raises(RuntimeError, match="Failed to spawn"): d.start() - @pytest.mark.parametrize( - "doomed", - [ - _daemon._LOG_FILENAME, - _daemon._STARTUP_DEADLINE_FILENAME, - _daemon._PID_FILENAME, - ], - ) - def test_metadata_write_failure_cleans_up_and_raises( - self, tmp_path, monkeypatch, doomed - ): - # An unwritable tempdir fails start() the same way a failed spawn - # does, whichever of the files start() lays down hits the error. The - # tempdir must not survive a failure before daemon.log exists, since - # the sweep keys off that marker and would never reap it. - monkeypatch.setattr( - AcceleratorDaemon, "_cleanup_tempdir", _real_cleanup_tempdir - ) - real_open = open - - def bad_open(path, *args, **kwargs): - # The deadline file is written to a ".tmp" sibling and renamed, so - # match on the prefix rather than the final name. - if os.path.basename(str(path)).startswith(doomed): - raise OSError("no space left on device") - return real_open(path, *args, **kwargs) - - monkeypatch.setattr(_daemon, "open", bad_open, raising=False) - d = _make_daemon(tmp_path) - with pytest.raises(RuntimeError, match="Failed to write accelerator daemon"): - d.start() - assert d._tempdir is None - assert not (tmp_path / "accel-tmp").exists() - def test_process_exits_during_startup_raises(self, tmp_path, monkeypatch): import subprocess @@ -761,373 +704,6 @@ def test_daemon_constructor_invalid_env_var_raises(self, tmp_path, monkeypatch): not hasattr(_daemon.socket, "AF_UNIX"), reason="Unix domain sockets (AF_UNIX) are not supported on this platform", ) -class TestSweepStaleTempdirs: - """_sweep_stale_tempdirs reaps leftover daemon tempdirs, keyed on the - daemon.log marker, and deletes only on proof that the spawner is gone.""" - - @pytest.fixture - def tmp_root(self, monkeypatch): - # Point the sweep's tempdir root at an isolated directory of our own. - # dir= is explicit because mkdtemp would otherwise resolve its default - # through the gettempdir() the autouse fixture has already patched. - root = pathlib.Path(_real_mkdtemp(prefix="bt-sweep-", dir=_real_gettempdir())) - monkeypatch.setattr(_daemon.tempfile, "gettempdir", lambda: str(root)) - yield root - shutil.rmtree(root, ignore_errors=True) - - @staticmethod - def _dead_pid(): - """A PID that is guaranteed not to resolve to a running process.""" - proc = subprocess.Popen([sys.executable, "-c", ""]) - proc.wait() - return proc.pid - - @staticmethod - def _make_leftover( - root, - name, - *, - with_log=True, - age_seconds=0.0, - deadline_offset=None, - pid=None, - namespace=_UNSET, - ): - """Create a fake leftover daemon tempdir. - - deadline_offset: seconds relative to now for the startup deadline file. - Negative = deadline already passed (stale); positive = still in future - (mid-startup). None = omit the file (tests the fallback heuristic). - pid: if given, write a daemon.pid naming this PID. Pass a dead PID to - simulate a crashed spawner, or os.getpid() for "alive". - namespace: what to stamp next to the PID. Defaults to this process's - own, so the sweep accepts it; pass something else to simulate a - tempdir written in a different PID namespace, or None to omit the - stamp entirely (a tempdir predating it). - """ - d = root / name - d.mkdir() - if with_log: - log = d / _daemon._LOG_FILENAME - log.write_bytes(b"hi") - if age_seconds: - past = time.time() - age_seconds - os.utime(log, (past, past)) - if deadline_offset is not None: - dl = d / _daemon._STARTUP_DEADLINE_FILENAME - dl.write_text(repr(time.time() + deadline_offset)) - if pid is not None: - if namespace is _UNSET: - namespace = _daemon._pid_namespace() - stamp = str(pid) if namespace is None else f"{pid} {namespace}" - (d / _daemon._PID_FILENAME).write_text(stamp) - return d - - # -- the one path that deletes ------------------------------------------- - - def test_removes_stale_dir(self, tmp_root): - # Expired deadline, old log, and the kernel says the spawner's PID - # does not resolve to any process. That ESRCH is the only thing that - # may lead to a delete. - d = self._make_leftover( - tmp_root, - "bt-accel-dead", - age_seconds=120, - deadline_offset=-70, - pid=self._dead_pid(), - ) - _daemon._sweep_stale_tempdirs() - assert not d.exists() - - def test_removes_stale_dir_without_deadline_file(self, tmp_root): - # A tempdir predating the deadline file still gets judged, via the - # log-mtime fallback -- but only because the pid check proves death. - d = self._make_leftover( - tmp_root, - "bt-accel-nodl", - age_seconds=120, - deadline_offset=None, - pid=self._dead_pid(), - ) - _daemon._sweep_stale_tempdirs() - assert not d.exists() - - # -- everything else keeps ------------------------------------------------ - - def test_keeps_dir_with_live_spawner(self, tmp_root): - # Well past every age gate, but the spawning process is alive, so its - # daemon is alive and the directory is not a leftover. - d = self._make_leftover( - tmp_root, - "bt-accel-live", - age_seconds=86400, - deadline_offset=-86400, - pid=os.getpid(), - ) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_keeps_live_dir_from_an_older_client(self, tmp_root): - # A tempdir written by a previously released client has a daemon.log - # and no metadata at all. There is no owner to probe, so there is no - # proof of death, so it must survive -- its daemon may well be - # serving. Mixed versions share one /tmp whenever two virtualenvs (or - # a rolling restart) run side by side. - d = self._make_leftover( - tmp_root, "bt-accel-legacy", age_seconds=86400, deadline_offset=None - ) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_keeps_dir_from_another_pid_namespace(self, tmp_root): - # Containers sharing a /tmp volume have separate PID numbering, so a - # PID written over there means something else (or nothing) here. - # Signalling it would give a confident, repeatable wrong answer, so - # the sweep declines to judge. The dead PID makes the point: without - # the namespace check this directory would be deleted. - d = self._make_leftover( - tmp_root, - "bt-accel-otherns", - age_seconds=86400, - deadline_offset=-86400, - pid=self._dead_pid(), - namespace="4:4026531999", - ) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_keeps_dir_with_unstamped_pid_file(self, tmp_root): - # A pid file written before the namespace was recorded cannot be - # checked either, for the same reason. - d = self._make_leftover( - tmp_root, - "bt-accel-nostamp", - age_seconds=86400, - deadline_offset=-86400, - pid=self._dead_pid(), - namespace=None, - ) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - @pytest.mark.parametrize("contents", ["", " ", "not-a-pid", "garbage stamp"]) - def test_keeps_dir_with_unusable_pid_file(self, tmp_root, contents): - # A crash mid-write can leave the pid file empty or truncated. That is - # a deliberate, permanent leak: unreadable is not proof of death. - d = self._make_leftover( - tmp_root, "bt-accel-badpid", age_seconds=86400, deadline_offset=-86400 - ) - (d / _daemon._PID_FILENAME).write_text(contents) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_keeps_dir_when_pid_file_is_unreadable(self, tmp_root, monkeypatch): - # An OSError other than "missing" tells us nothing about the owner. - d = self._make_leftover( - tmp_root, - "bt-accel-unreadable", - age_seconds=86400, - deadline_offset=-86400, - pid=self._dead_pid(), - ) - real_open = open - - def bad_open(path, *args, **kwargs): - if os.path.basename(str(path)) == _daemon._PID_FILENAME: - raise PermissionError("EACCES") - return real_open(path, *args, **kwargs) - - monkeypatch.setattr(_daemon, "open", bad_open, raising=False) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_keeps_dir_when_pid_is_not_ours_to_signal(self, tmp_root, monkeypatch): - # EPERM from os.kill means the PID exists under another user: alive. - d = self._make_leftover( - tmp_root, - "bt-accel-eperm", - age_seconds=86400, - deadline_offset=-86400, - pid=os.getpid(), - ) - - def eperm(pid, sig): - raise PermissionError("EPERM") - - monkeypatch.setattr(_daemon.os, "kill", eperm) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_keeps_dir_whose_pid_was_recycled(self, tmp_root): - # A recycled PID now belonging to some unrelated long-lived process - # reads as alive, so the directory is pinned forever. That leak is the - # accepted cost of never deleting a live daemon; detecting reuse means - # a heuristic, and a wrong "recycled" verdict deletes a live daemon. - d = self._make_leftover( - tmp_root, - "bt-accel-recycled", - age_seconds=86400, - deadline_offset=-86400, - pid=os.getpid(), - ) - pid_file = d / _daemon._PID_FILENAME - old = time.time() - 86400 - os.utime(pid_file, (old, old)) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_keeps_young_dir(self, tmp_root): - # No deadline file: fallback uses log mtime + buffer; log is fresh. - d = self._make_leftover( - tmp_root, "bt-accel-young", age_seconds=0, pid=self._dead_pid() - ) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_keeps_dir_with_future_deadline(self, tmp_root): - # Log is old (a long-timeout override) but the daemon is still inside - # its own startup budget, when the pid file may not be written yet. - d = self._make_leftover( - tmp_root, "bt-accel-slow", age_seconds=120, deadline_offset=+200 - ) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_ignores_dir_without_log_marker(self, tmp_root): - # Prefix matches but there's no daemon.log, so it isn't one of ours. - d = tmp_root / "bt-accel-foreign" - d.mkdir() - (d / "something-else").write_bytes(b"x") - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_budget_exhausted_leaves_dirs_for_next_sweep(self, tmp_root, monkeypatch): - # Same inputs as test_removes_stale_dir, but the time budget is already - # spent: the walk bails out rather than running long on a huge /tmp. - d = self._make_leftover( - tmp_root, - "bt-accel-dead", - age_seconds=120, - deadline_offset=-70, - pid=self._dead_pid(), - ) - monkeypatch.setattr(_daemon, "_SWEEP_BUDGET_SECONDS", -1.0) - _daemon._sweep_stale_tempdirs() - assert d.exists() - - def test_glob_failure_is_swallowed(self, tmp_root, monkeypatch): - # Enumeration can fail outright (a vanished or unreadable /tmp). The - # sweep is opportunistic, so that must not propagate to start(). - def boom(*args, **kwargs): - raise OSError("EIO") - - monkeypatch.setattr(_daemon.glob, "iglob", boom) - _daemon._sweep_stale_tempdirs() - - def test_no_af_unix_is_a_noop(self, tmp_root, monkeypatch): - # Without AF_UNIX the accelerator cannot have run here at all. - d = self._make_leftover( - tmp_root, - "bt-accel-dead", - age_seconds=120, - deadline_offset=-70, - pid=self._dead_pid(), - ) - monkeypatch.delattr(_daemon.socket, "AF_UNIX") - _daemon._sweep_stale_tempdirs() - assert d.exists() - - # -- wiring into start() -------------------------------------------------- - - def test_start_invokes_sweep(self, tmp_path, monkeypatch): - done = threading.Event() - - def _fake_sweep(): - done.set() - - monkeypatch.setattr(_daemon, "_sweep_stale_tempdirs", _fake_sweep) - - def boom(*args, **kwargs): - raise OSError("refusing to spawn in test") - - monkeypatch.setattr(_daemon.subprocess, "Popen", boom) - daemon = _make_daemon(tmp_path) - with pytest.raises(RuntimeError): - daemon.start() - assert done.wait(timeout=2.0), "sweep thread did not run within 2 s" - - def test_start_survives_sweep_thread_failure(self, tmp_path, monkeypatch): - # At the process thread ceiling, Thread.start() raises. Tidying /tmp - # is the least important thing start() does; it must not be the thing - # that costs the caller a working client. - class NoThreads: - def __init__(self, *args, **kwargs): - pass - - def start(self): - raise RuntimeError("can't start new thread") - - monkeypatch.setattr(_daemon.threading, "Thread", NoThreads) - popen_called = [] - - def boom(*args, **kwargs): - popen_called.append(True) - raise OSError("refusing to spawn in test") - - monkeypatch.setattr(_daemon.subprocess, "Popen", boom) - daemon = _make_daemon(tmp_path) - # Reaches Popen and fails there, rather than at Thread.start(). - with pytest.raises(RuntimeError, match="Failed to spawn"): - daemon.start() - assert popen_called - - def test_start_stamps_pid_and_namespace(self, tmp_path, monkeypatch): - # The file start() writes must be the one the sweep knows how to read. - def boom(*args, **kwargs): - raise OSError("refusing to spawn in test") - - monkeypatch.setattr(_daemon.subprocess, "Popen", boom) - monkeypatch.setattr(AcceleratorDaemon, "_cleanup_tempdir", lambda self: None) - daemon = _make_daemon(tmp_path) - with pytest.raises(RuntimeError, match="Failed to spawn"): - daemon.start() - written = (pathlib.Path(daemon._tempdir) / _daemon._PID_FILENAME).read_text() - assert written.split() == [str(os.getpid()), _daemon._pid_namespace()] - - -class TestPidNamespace: - """_pid_namespace identifies the namespace a process's PIDs belong to.""" - - @pytest.mark.skipif( - not os.path.exists("/proc/self/ns/pid"), reason="requires Linux namespaces" - ) - def test_reports_own_namespace(self): - ns = _daemon._pid_namespace() - assert ns != _daemon._NO_PID_NAMESPACE - # Stable for the life of the process, and shared with our children. - assert ns == _daemon._pid_namespace() - child = subprocess.run( - [ - sys.executable, - "-c", - "import os;st=os.stat('/proc/self/ns/pid');" - "print(f'{st.st_dev}:{st.st_ino}')", - ], - capture_output=True, - text=True, - ) - assert child.stdout.strip() == ns - - def test_falls_back_when_proc_is_absent(self, monkeypatch): - # macOS and friends have no /proc, and also no PID namespaces, so the - # sentinel compares equal to itself and the check is a no-op there. - def no_proc(path, *args, **kwargs): - raise FileNotFoundError(path) - - monkeypatch.setattr(_daemon.os, "stat", no_proc) - assert _daemon._pid_namespace() == _daemon._NO_PID_NAMESPACE - - @pytest.mark.skipif(not hasattr(_daemon.socket, "AF_UNIX"), reason="requires AF_UNIX") class TestSocketIsLive: """_socket_is_live backs the _wait_until_ready poll: is the daemon up?""" @@ -1152,38 +728,3 @@ def test_false_for_socket_file_whose_owner_exited(self, tmp_path): sock.close() assert os.path.exists(path) assert _daemon._socket_is_live(path) is False - - -class TestWriteMetadataFile: - """_write_metadata_file swaps contents in atomically.""" - - def test_writes_contents(self, tmp_path): - target = tmp_path / "daemon.pid" - _daemon._write_metadata_file(str(target), "1234") - assert target.read_text() == "1234" - # The staging file must not be left behind on success. - assert not (tmp_path / "daemon.pid.tmp").exists() - - def test_target_is_absent_until_contents_are_complete(self, tmp_path): - # A reader racing the write sees no file rather than a truncated one: - # everything lands in the ".tmp" sibling and is complete and closed by - # the time the rename publishes it under the real name. - target = tmp_path / "daemon.pid" - observed = {} - real_replace = _daemon.os.replace - - def spy_replace(src, dst): - observed["target_existed"] = target.exists() - observed["staged"] = pathlib.Path(src).read_text() - return real_replace(src, dst) - - with unittest.mock.patch.object(_daemon.os, "replace", spy_replace): - _daemon._write_metadata_file(str(target), "1234") - assert observed == {"target_existed": False, "staged": "1234"} - assert target.read_text() == "1234" - - def test_overwrites_existing_file(self, tmp_path): - target = tmp_path / "daemon.pid" - target.write_text("stale-and-longer") - _daemon._write_metadata_file(str(target), "7") - assert target.read_text() == "7"