Isolate the stdio server's stdin and stdout from handler subprocesses (#3117)
This commit is contained in:
parent
00a70148bc
commit
629ca297d2
2 changed files with 811 additions and 169 deletions
|
|
@ -15,12 +15,15 @@ import sys
|
|||
import threading
|
||||
from contextlib import AsyncExitStack
|
||||
from pathlib import Path
|
||||
from textwrap import dedent
|
||||
|
||||
import anyio
|
||||
import anyio.abc
|
||||
import pytest
|
||||
from mcp_types import TextContent
|
||||
|
||||
from mcp.client import stdio
|
||||
from mcp.client.client import Client
|
||||
from mcp.client.stdio import StdioServerParameters, stdio_client
|
||||
from mcp.os.win32.utilities import FallbackProcess
|
||||
from tests.transports.stdio._liveness import (
|
||||
|
|
@ -274,3 +277,42 @@ async def test_fallback_process_wait_is_cancellable_while_the_child_lives() -> N
|
|||
popen.wait()
|
||||
popen.stdin.close()
|
||||
popen.stdout.close()
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_a_tool_spawned_childs_stdout_writes_never_reach_the_wire(tmp_path: Path) -> None:
|
||||
"""A child writing to its inherited stdout pollutes the server's stderr, never the protocol.
|
||||
|
||||
Pre-isolation the junk line landed in the JSON-RPC stream (fails on base);
|
||||
fd 1 has exactly one target, so stderr delivery proves the wire never saw it.
|
||||
"""
|
||||
server = dedent(
|
||||
"""
|
||||
import subprocess, sys
|
||||
from mcp.server import MCPServer
|
||||
|
||||
mcp = MCPServer("noisy-spawner")
|
||||
|
||||
@mcp.tool()
|
||||
def run_noisy_child() -> str:
|
||||
# No redirection: the child inherits the server's stdout.
|
||||
proc = subprocess.run([sys.executable, "-c", "print('this is not json')"], timeout=20)
|
||||
return str(proc.returncode)
|
||||
|
||||
mcp.run()
|
||||
"""
|
||||
)
|
||||
|
||||
with (tmp_path / "server-stderr.txt").open("w+", encoding="utf-8") as errlog:
|
||||
transport = stdio_client(StdioServerParameters(command=sys.executable, args=["-c", server]), errlog=errlog)
|
||||
# Bound covers three interpreter cold starts; a regressed Windows leg hangs rather than corrupts.
|
||||
with anyio.fail_after(40):
|
||||
async with Client(transport) as client:
|
||||
result = await client.call_tool("run_noisy_child")
|
||||
errlog.seek(0)
|
||||
server_stderr = errlog.read()
|
||||
|
||||
content = result.content[0]
|
||||
assert isinstance(content, TextContent)
|
||||
assert content.text == "0"
|
||||
assert "this is not json" in server_stderr
|
||||
|
|
|
|||
|
|
@ -15,12 +15,14 @@ import asyncio
|
|||
import sys
|
||||
from contextlib import AsyncExitStack
|
||||
from pathlib import Path
|
||||
from textwrap import dedent
|
||||
|
||||
import anyio
|
||||
import anyio.abc
|
||||
import pytest
|
||||
from mcp_types import JSONRPCRequest, JSONRPCResponse
|
||||
from mcp_types import JSONRPCRequest, JSONRPCResponse, TextContent
|
||||
|
||||
from mcp.client.client import Client
|
||||
from mcp.client.stdio import StdioServerParameters, stdio_client
|
||||
from mcp.os.win32.utilities import FallbackProcess
|
||||
from mcp.shared.message import SessionMessage
|
||||
|
|
@ -238,3 +240,46 @@ async def test_a_native_server_emitting_crlf_line_endings_round_trips_messages()
|
|||
# here instead of a parsed message.
|
||||
assert isinstance(received, SessionMessage)
|
||||
assert received.message == JSONRPCResponse(jsonrpc="2.0", id=1, result={})
|
||||
|
||||
|
||||
async def test_a_tool_spawned_python_child_with_default_stdin_completes_promptly() -> None: # pragma: no cover
|
||||
"""A tool that runs a Python subprocess without redirecting stdin returns promptly.
|
||||
|
||||
Regression for #671: pre-isolation the child inherited the protocol stdin pipe
|
||||
and hung in interpreter startup (CPython gh-78961) until the next inbound message.
|
||||
"""
|
||||
server = dedent(
|
||||
"""
|
||||
import subprocess, sys
|
||||
from mcp.server import MCPServer
|
||||
|
||||
mcp = MCPServer("spawner")
|
||||
|
||||
@mcp.tool()
|
||||
def run_child() -> str:
|
||||
proc = subprocess.run([sys.executable, "-c", "print('ok')"], capture_output=True, timeout=20)
|
||||
return proc.stdout.decode().strip()
|
||||
|
||||
@mcp.tool()
|
||||
def run_child_bare() -> str:
|
||||
# Even without redirection the console subsystem hands the child the standard handles.
|
||||
proc = subprocess.run([sys.executable, "-c", "pass"], timeout=20)
|
||||
return str(proc.returncode)
|
||||
|
||||
mcp.run()
|
||||
"""
|
||||
)
|
||||
transport = stdio_client(StdioServerParameters(command=sys.executable, args=["-c", server]))
|
||||
|
||||
# A regression hangs forever, so the bound only has to beat "never".
|
||||
with anyio.fail_after(40.0):
|
||||
async with Client(transport) as client:
|
||||
result = await client.call_tool("run_child")
|
||||
bare = await client.call_tool("run_child_bare")
|
||||
|
||||
content = result.content[0]
|
||||
assert isinstance(content, TextContent)
|
||||
assert content.text == "ok"
|
||||
bare_content = bare.content[0]
|
||||
assert isinstance(bare_content, TextContent)
|
||||
assert bare_content.text == "0"
|
||||
|
|
|
|||
Loading…
Reference in a new issue