From e58415a9feb351b3a14931383c5cf18b40c53759 Mon Sep 17 00:00:00 2001 From: Jeff Crouse Date: Tue, 10 Nov 2026 10:38:52 -0500 Subject: [PATCH] fix(server): pool workers exit with the server; shutdown stops the scan pool An idle multiprocessing pool worker holds a write end of its own task queue, so it never sees EOF when its parent dies; it leaves only on the pool's stop message. The library-scan pool's worker outlives each sync, and nothing stopped that pool at shutdown (only an atexit handler, which did not deliver the message): after a SIGTERM it was re-parented to PID 1 with its database connection open. Four such orphans held test databases for a day. Familiar Server stops and restarts the server the same way. - process_setup.exit_with_parent: a daemon thread in every pool worker _exits once its parent PID changes (macOS has no PR_SET_PDEATHSIG). scan_worker_init and analysis_worker_init both start it. - BackgroundManager.shutdown stops the scan pool too. - tests/test_workers_exit_with_parent.py: the real scan pool and the analysis initialiser each take a task, the parent is SIGKILLed, the worker must be gone within 10 s. Against the previous code both fail ("worker outlived its killed parent"). A real server synced then SIGTERMed leaves no child and no database connection. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UpUascYWiPxuH2vtav72eV --- CHANGELOG.md | 6 ++ backend/app/process_setup.py | 43 ++++++++++- backend/app/services/background/manager.py | 5 ++ backend/app/services/tasks/library_sync.py | 18 ++++- .../tests/test_workers_exit_with_parent.py | 75 +++++++++++++++++++ 5 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 backend/tests/test_workers_exit_with_parent.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e6ba1f04..b701505a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ them from stored failures. ### Fixed +- **Stopping the server no longer leaves a library-scan process behind** holding a database + connection. The scan pool's worker outlives each sync, and an idle pool worker never notices its + parent has gone; nothing stopped that pool at shutdown, so a SIGTERM (how Familiar Server stops + and restarts the server) or a crash orphaned it. Every pool worker now exits within a second of + losing its parent, and shutdown stops the scan pool. Docker was unaffected: stopping a container + ends every process in it. - **Track Edit's Analysis tab no longer shows "NaN%"** for a detected feature the server left out (found moving tracks to generated types, ADR-0173). - **Adding a track to a one-track playlist no longer ties it with the first** (found extracting diff --git a/backend/app/process_setup.py b/backend/app/process_setup.py index 2bf4d440..0ff1c413 100644 --- a/backend/app/process_setup.py +++ b/backend/app/process_setup.py @@ -15,6 +15,12 @@ that read. Python's `mimetypes.init` does not catch the `PermissionError`, so the first static file the server sent, the web admin's own page, answered 500. Found on the first install against a real library, 2026-10-01; the integration check never fetched a page, only the API. +- **Workers die with the server.** A `multiprocessing` pool worker waits on its task queue, and it + holds a write end of that queue itself, so when the server goes it never sees end-of-file. It + leaves only if the pool sends it a stop message on the way out, which a crash or SIGKILL never + does, and which on 2026-11-10 a clean SIGTERM did not deliver to the scan pool's idle worker + either. Those orphans, re-parented to PID 1, held database connections for a day. macOS has no + `PR_SET_PDEATHSIG`, so `exit_with_parent` polls instead. - **Background priority (ADR-0138 point 3).** `nice(10)` decides who gets the CPU. On Darwin the background QoS clamp (`PRIO_DARWIN_BG`) also moves the work onto efficiency cores and throttles its I/O, which is what keeps a laptop cool while it analyses a library. @@ -26,6 +32,8 @@ import multiprocessing import os import sys +import threading +import time SEMAPHORE_PREFIX_ENV = "FAMILIAR_SEMAPHORE_PREFIX" @@ -94,8 +102,41 @@ def darwin_background_clamped() -> bool: return libc.getpriority(_PRIO_DARWIN_PROCESS, 0) > 0 +#: How often a pool worker checks that the server is still its parent. +PARENT_POLL_S = 1.0 + + +def exit_with_parent(poll_s: float = PARENT_POLL_S) -> threading.Thread: + """End this process soon after its parent is gone, however the parent went. + + A daemon thread compares `os.getppid()` with the parent at start; when the parent dies the + process is re-parented (to 1, or to a subreaper) and the thread calls `os._exit`. `_exit`, not + `sys.exit`: the worker may be blocked in a pipe read or a native call, and nothing it holds is + worth a clean shutdown once the server that asked for the work is gone. An open transaction + rolls back when PostgreSQL sees the connection close. + """ + parent = os.getppid() + + def watch() -> None: + while True: + time.sleep(poll_s) + if os.getppid() != parent: + os._exit(0) + + thread = threading.Thread(target=watch, name="exit-with-parent", daemon=True) + thread.start() + return thread + + +def scan_worker_init() -> None: + """Initialiser for the library-scan pool: the platform's semaphore names; exit with the server.""" + apply_semaphore_prefix() + exit_with_parent() + + def analysis_worker_init() -> None: - """Initialiser for the analysis pools: the platform's semaphore names, then background priority.""" + """Initialiser for the analysis pools: semaphore names, exit with the server, background priority.""" apply_semaphore_prefix() + exit_with_parent() lower_priority() logging.info(f"Analysis worker started at background priority (PID {os.getpid()})") diff --git a/backend/app/services/background/manager.py b/backend/app/services/background/manager.py index 39976268..65446e17 100644 --- a/backend/app/services/background/manager.py +++ b/backend/app/services/background/manager.py @@ -395,6 +395,11 @@ async def shutdown(self) -> None: self._executor.shutdown(wait=False) if self._ondemand_executor: self._ondemand_executor.shutdown(wait=False) + # The scan pool is a module singleton, not one of this manager's, and was never stopped + # here: its idle worker outlived every server that started it (2026-11-10). + from app.services.tasks.library_sync import shutdown_scan_executor + + shutdown_scan_executor() logger.info("BackgroundManager shutdown complete") diff --git a/backend/app/services/tasks/library_sync.py b/backend/app/services/tasks/library_sync.py index bc33f8be..10ec56ff 100644 --- a/backend/app/services/tasks/library_sync.py +++ b/backend/app/services/tasks/library_sync.py @@ -127,6 +127,17 @@ def _cleanup_scan_executor() -> None: pass +def shutdown_scan_executor() -> None: + """Stop the scan pool with the server. Its worker would otherwise wait for work for ever.""" + global _scan_executor + if _scan_executor is not None: + try: + _scan_executor.shutdown(wait=False, cancel_futures=True) + except Exception: + pass + _scan_executor = None + + def _get_scan_executor() -> ProcessPoolExecutor: """Lazy singleton ProcessPoolExecutor for library scanning. @@ -137,13 +148,14 @@ def _get_scan_executor() -> ProcessPoolExecutor: import atexit import multiprocessing as mp - from app.process_setup import apply_semaphore_prefix + from app.process_setup import scan_worker_init _scan_executor = ProcessPoolExecutor( max_workers=1, mp_context=mp.get_context("spawn"), - # Sandbox-safe semaphore names in the worker too (ADR-0136 point 3). - initializer=apply_semaphore_prefix, + # Sandbox-safe semaphore names in the worker too (ADR-0136 point 3), and a worker that + # exits with the server: this one outlives each sync, so it is the one that was orphaned. + initializer=scan_worker_init, ) if not _scan_atexit_registered: diff --git a/backend/tests/test_workers_exit_with_parent.py b/backend/tests/test_workers_exit_with_parent.py new file mode 100644 index 00000000..bf667f03 --- /dev/null +++ b/backend/tests/test_workers_exit_with_parent.py @@ -0,0 +1,75 @@ +"""Pool workers exit when the server that started them is gone (2026-11-10). + +An idle `multiprocessing` pool worker holds a write end of its own task queue, so it never sees +end-of-file when its parent dies; it leaves only if the pool sends it a stop message. The library +scan pool's worker outlives each sync, and four of them, orphaned to PID 1 by E2E servers that had +been stopped, held test databases open for a day and blocked recreating one. Here the parent is +killed with SIGKILL, which sends nothing at all, and the worker must still go. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +import textwrap +import time +from pathlib import Path + +import pytest + +BACKEND = Path(__file__).resolve().parent.parent + +HARNESS = { + "scan pool": """ + from app.services.tasks.library_sync import _get_scan_executor + executor = _get_scan_executor() + """, + "analysis pool initialiser": """ + import multiprocessing as mp + from concurrent.futures import ProcessPoolExecutor + from app.services.background.executors import _analysis_worker_init + executor = ProcessPoolExecutor(1, mp_context=mp.get_context("spawn"), initializer=_analysis_worker_init) + """, +} + + +def _alive(pid: int) -> bool: + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + # A zombie still answers kill(0) until reaped; only a process that is running counts. + state = subprocess.run(["ps", "-o", "stat=", "-p", str(pid)], capture_output=True, text=True) + return bool(state.stdout.strip()) and not state.stdout.strip().startswith("Z") + + +@pytest.mark.parametrize("pool", list(HARNESS)) +def test_an_idle_worker_exits_after_its_parent_is_killed(pool, tmp_path): + pid_file = tmp_path / "worker.pid" + script = textwrap.dedent(HARNESS[pool]) + textwrap.dedent(f""" + import os, signal + worker = executor.submit(os.getpid).result(timeout=60) + open({str(pid_file)!r}, "w").write(str(worker)) + os.kill(os.getpid(), signal.SIGKILL) # no atexit, no shutdown, no stop message + """) + # The PID goes through a file, and the harness's output nowhere: a worker that outlives it + # inherits its stdout, and a pipe here would hold this test open until the worker died. + result = subprocess.run( + [sys.executable, "-c", script], + cwd=BACKEND, + env={**os.environ, "PYTHONPATH": str(BACKEND)}, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + timeout=120, + ) + assert result.returncode == -9, "the harness did not reach its own SIGKILL" + worker = int(pid_file.read_text()) + + deadline = time.monotonic() + 10 + while _alive(worker) and time.monotonic() < deadline: + time.sleep(0.2) + alive = _alive(worker) + if alive: + os.kill(worker, 9) # never leave the orphan this test exists to prevent + assert not alive, f"worker {worker} outlived its killed parent ({pool})"