From b11995d81e0cb54f2ca3e6274bce5f79f4ee3bdf Mon Sep 17 00:00:00 2001 From: xcarbo Date: Sun, 12 Jul 2026 11:49:48 +0400 Subject: [PATCH] fix(mcp): self-exit on stdin EOF/broken pipe so orphaned stdio sessions release locks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01DpEdb9SNWF42BpupHWBhC3 --- mempalace/mcp_server.py | 60 +++++++++++++++++++++---- tests/test_mcp_server_eof.py | 86 ++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 tests/test_mcp_server_eof.py diff --git a/mempalace/mcp_server.py b/mempalace/mcp_server.py index 91d7df0f7f..abb6a50469 100644 --- a/mempalace/mcp_server.py +++ b/mempalace/mcp_server.py @@ -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: diff --git a/tests/test_mcp_server_eof.py b/tests/test_mcp_server_eof.py new file mode 100644 index 0000000000..81d35e5909 --- /dev/null +++ b/tests/test_mcp_server_eof.py @@ -0,0 +1,86 @@ +"""Self-exit on stdin EOF / broken stdout pipe — orphaned stdio servers. + +A stdio MCP server launched over SSH (``ssh 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}" + )