Skip to content

Commit 56241e3

Browse files
committed
fix(mcp): drain subprocess stderr so a verbose server can't block itself
stderr=PIPE without a consumer let a spammy MCP server fill the ~64KB pipe buffer + StreamReader limit, pause the transport, and force the child to block on stderr.flush() — no more stdin reads, no more stdout writes, every request timed out. The drain task reads chunks as they arrive so the child stays responsive; lines are logged at debug so operators can still recover them by turning logging up.
1 parent 96f3df3 commit 56241e3

1 file changed

Lines changed: 37 additions & 0 deletions

File tree

zhub/mcp.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ def __init__(
6161
self._next_id = 0
6262
self._pending: dict[int, asyncio.Future] = {}
6363
self._reader_task: asyncio.Task | None = None
64+
self._stderr_task: asyncio.Task | None = None
6465
self._lock = asyncio.Lock()
6566
self._initialized = False
6667

@@ -74,6 +75,12 @@ async def start(self, init_timeout: float = 10.0) -> None:
7475
env=self.env,
7576
)
7677
self._reader_task = asyncio.create_task(self._reader_loop())
78+
# Drain the subprocess's stderr. If we leave stderr=PIPE without a
79+
# consumer, an MCP server that logs verbosely fills the ~64KB pipe
80+
# buffer, blocks on its next stderr write, and stops answering any
81+
# request — every subsequent call then times out. Logging lines at
82+
# debug keeps them recoverable when the operator turns logging up.
83+
self._stderr_task = asyncio.create_task(self._stderr_drain())
7784
try:
7885
await asyncio.wait_for(
7986
self._request("initialize", {
@@ -115,6 +122,13 @@ async def close(self) -> None:
115122
except (asyncio.CancelledError, Exception):
116123
pass
117124
self._reader_task = None
125+
if self._stderr_task:
126+
self._stderr_task.cancel()
127+
try:
128+
await self._stderr_task
129+
except (asyncio.CancelledError, Exception):
130+
pass
131+
self._stderr_task = None
118132
if self.process and self.process.returncode is None:
119133
try:
120134
self.process.terminate()
@@ -207,3 +221,26 @@ async def _reader_loop(self) -> None:
207221
return
208222
# Clean EOF (the `break` above): subprocess closed its output stream.
209223
self._fail_pending(MCPError("MCP subprocess closed its output stream"))
224+
225+
async def _stderr_drain(self) -> None:
226+
"""Discard-with-debug-log drain for the subprocess's stderr.
227+
228+
Runs alongside `_reader_loop`; consumes chunks so the pipe buffer
229+
never fills. Uses `read(N)` rather than `readline()` — a spammy MCP
230+
server that writes megabytes without a newline (progress bars, JSON
231+
blobs on one line) would otherwise accumulate past StreamReader's
232+
limit, pause the transport, and refill the pipe anyway. On EOF the
233+
drain returns cleanly — stdout EOF terminates the client, stderr EOF
234+
alone is expected on shutdown and carries no signal we act on.
235+
"""
236+
assert self.process and self.process.stderr
237+
try:
238+
while True:
239+
chunk = await self.process.stderr.read(4096)
240+
if not chunk:
241+
return
242+
log.debug("mcp stderr: %s", chunk.decode("utf-8", "replace").rstrip("\n"))
243+
except asyncio.CancelledError:
244+
raise
245+
except Exception as e:
246+
log.debug("mcp stderr drain stopped: %s", e)

0 commit comments

Comments
 (0)