Fix preview worker pool leak: ffmpeg must not inherit worker stdin
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
This commit is contained in:
+3
-1
@@ -145,6 +145,9 @@ class _PreviewWorker:
|
|||||||
|
|
||||||
async def kill(self) -> None:
|
async def kill(self) -> None:
|
||||||
if self.proc.returncode is 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):
|
with contextlib.suppress(ProcessLookupError):
|
||||||
self.proc.kill()
|
self.proc.kill()
|
||||||
await self.proc.wait()
|
await self.proc.wait()
|
||||||
@@ -179,7 +182,6 @@ class _PreviewWorkerPool:
|
|||||||
stdin=asyncio.subprocess.PIPE,
|
stdin=asyncio.subprocess.PIPE,
|
||||||
stdout=asyncio.subprocess.PIPE,
|
stdout=asyncio.subprocess.PIPE,
|
||||||
stderr=asyncio.subprocess.PIPE,
|
stderr=asyncio.subprocess.PIPE,
|
||||||
start_new_session=True,
|
|
||||||
)
|
)
|
||||||
_active_procs.add(proc)
|
_active_procs.add(proc)
|
||||||
try:
|
try:
|
||||||
|
|||||||
+13
-6
@@ -219,7 +219,18 @@ def _image_via_ffmpeg(path: Path, maxsize: int, quality: int) -> bytes:
|
|||||||
cmd.insert(5, f"{new_w}x{new_h}")
|
cmd.insert(5, f"{new_w}x{new_h}")
|
||||||
try:
|
try:
|
||||||
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:
|
except subprocess.CalledProcessError as e:
|
||||||
shell_cmd = shlex.join(cmd)
|
shell_cmd = shlex.join(cmd)
|
||||||
stderr = (e.stderr or b"").decode(errors="replace").strip()
|
stderr = (e.stderr or b"").decode(errors="replace").strip()
|
||||||
@@ -540,11 +551,7 @@ def main() -> None:
|
|||||||
logging.basicConfig(stream=sys.stderr, level=logging.INFO)
|
logging.basicConfig(stream=sys.stderr, level=logging.INFO)
|
||||||
try:
|
try:
|
||||||
config.load_config()
|
config.load_config()
|
||||||
logger.warning(
|
logger.info("preview-worker config=%s", config.conffile)
|
||||||
"preview-worker config=%s master_secret=%s",
|
|
||||||
config.conffile,
|
|
||||||
config.config.secret,
|
|
||||||
)
|
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.exception("preview-worker failed to load config at startup")
|
logger.exception("preview-worker failed to load config at startup")
|
||||||
if len(sys.argv) > 1:
|
if len(sys.argv) > 1:
|
||||||
|
|||||||
Reference in New Issue
Block a user