fix(mcp): self-exit on stdin EOF/broken pipe so orphaned stdio sessions release locks
A stdio mcp_server orphaned by a dropped SSH session used to swallow pipe errors in the generic exception handler and sleep forever on the dead channel, holding the mine_palace flock (2026-07-10 write-path outage, PID 23392). The loop now logs and shuts down on stdin EOF, OSError from stdin reads (EIO/EBADF from orphaned ptys), and BrokenPipeError/OSError on stdout writes. Shutdown returns from the loop for a normal interpreter exit 0 — no os._exit — so held flocks release via process teardown; a broken stdout is re-pointed at devnull first so the shutdown flush cannot turn the exit into status 120. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DpEdb9SNWF42BpupHWBhC3
This commit is contained in:
parent
aa89bd8227
commit
b11995d81e
|
|
@ -5442,6 +5442,24 @@ def _startup_preflight() -> None:
|
|||
logger.exception("startup preflight failed")
|
||||
|
||||
|
||||
def _drop_broken_stdout() -> None:
|
||||
"""Point fd 1 at devnull after a stdout write failed with a pipe error.
|
||||
|
||||
The response line that failed mid-write can leave bytes buffered in
|
||||
``sys.stdout``; the interpreter's shutdown flush would then re-raise
|
||||
``BrokenPipeError`` and turn a clean exit into status 120. With fd 1
|
||||
on devnull that final flush drains harmlessly, so the process exits 0
|
||||
and any held flocks (e.g. ``mine_palace``) release via normal
|
||||
teardown.
|
||||
"""
|
||||
try:
|
||||
devnull = os.open(os.devnull, os.O_WRONLY)
|
||||
os.dup2(devnull, sys.stdout.fileno())
|
||||
os.close(devnull)
|
||||
except (OSError, ValueError, AttributeError):
|
||||
pass
|
||||
|
||||
|
||||
def _run_stdio_loop() -> None:
|
||||
_restore_stdout()
|
||||
|
||||
|
|
@ -5484,23 +5502,49 @@ def _run_stdio_loop() -> None:
|
|||
while True:
|
||||
try:
|
||||
line = sys.stdin.readline()
|
||||
if not line:
|
||||
break
|
||||
except KeyboardInterrupt:
|
||||
break
|
||||
except OSError as exc:
|
||||
# An orphaned pty/pipe surfaces as EIO/EBADF here instead of a
|
||||
# clean EOF — same meaning: the client is gone. Never loop on
|
||||
# it: an orphaned stdio server holding the mine_palace flock
|
||||
# blocked all palace writes for hours (2026-07-10 outage).
|
||||
logger.info("stdin read failed (%s) — client disconnected, shutting down", exc)
|
||||
break
|
||||
if not line:
|
||||
logger.info("stdin EOF — client disconnected, shutting down")
|
||||
break
|
||||
|
||||
line = line.strip()
|
||||
if not line:
|
||||
continue
|
||||
line = line.strip()
|
||||
if not line:
|
||||
continue
|
||||
|
||||
payload = None
|
||||
try:
|
||||
request = json.loads(line)
|
||||
response = handle_request(request)
|
||||
|
||||
if response is not None:
|
||||
sys.stdout.write(json.dumps(response, ensure_ascii=False) + "\n")
|
||||
sys.stdout.flush()
|
||||
payload = json.dumps(response, ensure_ascii=False)
|
||||
except KeyboardInterrupt:
|
||||
break
|
||||
except Exception as e:
|
||||
logger.error(f"Server error: {e}")
|
||||
continue
|
||||
|
||||
if payload is None:
|
||||
continue
|
||||
try:
|
||||
sys.stdout.write(payload + "\n")
|
||||
sys.stdout.flush()
|
||||
except KeyboardInterrupt:
|
||||
break
|
||||
except (BrokenPipeError, OSError) as exc:
|
||||
# The client's read end is gone; every future response write
|
||||
# would fail the same way, so treat it like stdin EOF and
|
||||
# shut down instead of swallowing it in the generic handler.
|
||||
logger.info("stdout write failed (%s) — client disconnected, shutting down", exc)
|
||||
_drop_broken_stdout()
|
||||
break
|
||||
|
||||
|
||||
def _run_http_loop() -> None:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,86 @@
|
|||
"""Self-exit on stdin EOF / broken stdout pipe — orphaned stdio servers.
|
||||
|
||||
A stdio MCP server launched over SSH (``ssh <host> python3 -m
|
||||
mempalace.mcp_server``) is orphaned when the SSH session drops. If it
|
||||
does not exit when its stdio channel dies, it sleeps forever on the dead
|
||||
pipe and keeps holding whatever palace locks it acquired — the
|
||||
``mine_palace`` flock held idle for ~4 h caused the 2026-07-10 palace
|
||||
write-path outage.
|
||||
|
||||
These tests spawn the real server as a subprocess and kill its stdio
|
||||
channel from the outside. Palace isolation comes from conftest.py, which
|
||||
redirects HOME to a throwaway temp dir before any test runs; the
|
||||
subprocess inherits that environment, so ``~/.mempalace`` resolves inside
|
||||
the temp dir (same mechanism the test_mcp_stdio_protection.py subprocess
|
||||
tests rely on).
|
||||
"""
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
# Server startup imports chromadb — slow on a cold cache — so the exit
|
||||
# deadline is generous. The requirement is "exits at all", not "exits fast".
|
||||
_EXIT_TIMEOUT = 60
|
||||
|
||||
|
||||
def _spawn_server() -> subprocess.Popen:
|
||||
return subprocess.Popen(
|
||||
[sys.executable, "-m", "mempalace.mcp_server"],
|
||||
stdin=subprocess.PIPE,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
)
|
||||
|
||||
|
||||
def _wait_or_fail(proc: subprocess.Popen, why: str) -> int:
|
||||
try:
|
||||
return proc.wait(timeout=_EXIT_TIMEOUT)
|
||||
except subprocess.TimeoutExpired:
|
||||
proc.kill()
|
||||
proc.wait()
|
||||
pytest.fail(f"server did not exit within {_EXIT_TIMEOUT}s after {why}")
|
||||
|
||||
|
||||
def test_stdin_eof_exits_zero_and_logs():
|
||||
"""Closing stdin must make the server log to stderr and exit 0."""
|
||||
proc = _spawn_server()
|
||||
proc.stdin.close()
|
||||
returncode = _wait_or_fail(proc, "stdin EOF")
|
||||
stderr = proc.stderr.read().decode("utf-8", errors="replace")
|
||||
assert returncode == 0, f"expected exit 0, got {returncode}\nstderr: {stderr}"
|
||||
assert "stdin EOF" in stderr, f"missing EOF shutdown log line\nstderr: {stderr}"
|
||||
assert proc.stdout.read() == b"", "stdout must stay a clean JSON-RPC channel"
|
||||
|
||||
|
||||
def test_stdout_broken_pipe_exits_zero():
|
||||
"""A dead stdout reader must terminate the server, not be swallowed.
|
||||
|
||||
Deterministic: once the parent closes the pipe's only read end, the
|
||||
server's next stdout write fails with EPIPE immediately (Python
|
||||
ignores SIGPIPE, so it surfaces as BrokenPipeError, never a signal
|
||||
death). stdin stays open the whole time, so an exit can only come
|
||||
from the write path.
|
||||
"""
|
||||
proc = _spawn_server()
|
||||
request = json.dumps({"jsonrpc": "2.0", "id": 1, "method": "tools/list"}).encode() + b"\n"
|
||||
|
||||
# Prove the loop is up: one request, one response line.
|
||||
proc.stdin.write(request)
|
||||
proc.stdin.flush()
|
||||
first_response = proc.stdout.readline()
|
||||
assert first_response.startswith(b"{"), f"no JSON-RPC response: {first_response!r}"
|
||||
|
||||
# Kill the read end, then force another response write.
|
||||
proc.stdout.close()
|
||||
proc.stdin.write(request)
|
||||
proc.stdin.flush()
|
||||
|
||||
returncode = _wait_or_fail(proc, "stdout broken pipe")
|
||||
stderr = proc.stderr.read().decode("utf-8", errors="replace")
|
||||
assert returncode == 0, f"expected exit 0, got {returncode}\nstderr: {stderr}"
|
||||
assert "client disconnected" in stderr, (
|
||||
f"missing broken-pipe shutdown log line\nstderr: {stderr}"
|
||||
)
|
||||
Loading…
Reference in New Issue