From 1258eff42d807d784da2802f276e31fc09d38392 Mon Sep 17 00:00:00 2001 From: Leo Vasanko Date: Tue, 28 Jul 2026 01:14:15 +0000 Subject: [PATCH] Fix preview worker pool leak: ffmpeg must not inherit worker stdin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ffmpeg fallback in the preview worker inherited the worker's stdin pipe (the framed request protocol). When a slow conversion was killed at the 10s timeout, the orphaned ffmpeg grandchild kept that pipe open, so the parent's proc.wait() blocked forever waiting for pipe EOF — permanently sticking one dispatcher per event until the whole pool starved and every preview request (pdf, image, office) returned 503. - Run ffmpeg with stdin=DEVNULL (also stops it eating protocol bytes) - Drop start_new_session (only needed for group kills, POSIX-only) - Stop logging the master secret at worker startup --- cista/preview.py | 4 +++- cista/preview_worker.py | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/cista/preview.py b/cista/preview.py index aa3aac0..8c92335 100644 --- a/cista/preview.py +++ b/cista/preview.py @@ -145,6 +145,9 @@ class _PreviewWorker: async def kill(self) -> None: if self.proc.returncode is None: + # Safe to hard-kill: the worker is stateless per request, and its + # subprocesses (ffmpeg) use stdin=DEVNULL so they never hold the + # worker's pipes open — proc.wait() cannot hang on pipe EOF. with contextlib.suppress(ProcessLookupError): self.proc.kill() await self.proc.wait() @@ -179,7 +182,6 @@ class _PreviewWorkerPool: stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, - start_new_session=True, ) _active_procs.add(proc) try: diff --git a/cista/preview_worker.py b/cista/preview_worker.py index 288aa96..7300aac 100644 --- a/cista/preview_worker.py +++ b/cista/preview_worker.py @@ -219,7 +219,18 @@ def _image_via_ffmpeg(path: Path, maxsize: int, quality: int) -> bytes: cmd.insert(5, f"{new_w}x{new_h}") try: try: - subprocess.run(cmd, capture_output=True, check=True, shell=False) # noqa: S603 + # stdin=DEVNULL is critical: ffmpeg must not inherit the worker's + # stdin, which carries the framed request protocol. An inherited + # stdin lets ffmpeg eat protocol bytes and, if the worker is + # killed mid-conversion, keeps the orphaned ffmpeg holding the + # pipe open so the parent's proc.wait() hangs forever. + subprocess.run( # noqa: S603 + cmd, + capture_output=True, + check=True, + shell=False, + stdin=subprocess.DEVNULL, + ) except subprocess.CalledProcessError as e: shell_cmd = shlex.join(cmd) stderr = (e.stderr or b"").decode(errors="replace").strip() @@ -540,11 +551,7 @@ def main() -> None: logging.basicConfig(stream=sys.stderr, level=logging.INFO) try: config.load_config() - logger.warning( - "preview-worker config=%s master_secret=%s", - config.conffile, - config.config.secret, - ) + logger.info("preview-worker config=%s", config.conffile) except Exception: logger.exception("preview-worker failed to load config at startup") if len(sys.argv) > 1: