FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix stdio cleanup hanging on slow-terminating processes · modelcontextprotocol/python-sdk@2be6f09 · GitHub

/ python-sdk Public

Commit 2be6f09

Browse files
felixweinberger
and
cristipufu
committed
Fix stdio cleanup hanging on slow-terminating processes
The stdio cleanup was hanging indefinitely when processes ignored termination signals or took too long to exit. This caused the MCP client to freeze during shutdown, especially with servers that don't handle SIGTERM properly. This was already being handled on Windows, but not Unix systems. This Commit unifies the two approaches, removing special logic for windows process termination. The fix introduces a 2-second timeout for process termination. If a process doesn't exit gracefully within this window, it's forcefully killed. This ensures the client always completes cleanup in bounded time while still giving well-behaved servers a chance to exit cleanly. This resolves hanging issues reported when MCP servers ignore standard termination signals. resolves #555 Also adds regression tests for #559. Co-authored-by: Cristian Pufu <cristian.pufu@uipath.com>
1 parent adf3271 commit 2be6f09

3 files changed

Lines changed: 128 additions & 28 deletions

File tree

‎src/mcp/client/stdio/__init__.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
from .win32 import (
1717
create_windows_process,
1818
get_windows_executable_command,
19-
terminate_windows_process,
2019
)
2120

2221
# Environment variables to inherit by default
@@ -180,10 +179,12 @@ async def stdin_writer():
180179
finally:
181180
# Clean up process to prevent any dangling orphaned processes
182181
try:
183-
if sys.platform == "win32":
184-
await terminate_windows_process(process)
185-
else:
186-
process.terminate()
182+
process.terminate()
183+
with anyio.fail_after(2.0):
184+
await process.wait()
185+
except TimeoutError:
186+
# If process doesn't terminate in time, force kill it
187+
process.kill()
187188
except ProcessLookupError:
188189
# Process already exited, which is fine
189190
pass

‎src/mcp/client/stdio/win32.py‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88
from pathlib import Path
99
from typing import BinaryIO, TextIO, cast
1010

11-
import anyio
1211
from anyio import to_thread
13-
from anyio.abc import Process
1412
from anyio.streams.file import FileReadStream, FileWriteStream
1513

1614

@@ -159,24 +157,3 @@ async def create_windows_process(
159157
bufsize=0,
160158
)
161159
return FallbackProcess(popen_obj)
162-
163-
164-
async def terminate_windows_process(process: Process | FallbackProcess):
165-
"""
166-
Terminate a Windows process.
167-
168-
Note: On Windows, terminating a process with process.terminate() doesn't
169-
always guarantee immediate process termination.
170-
So we give it 2s to exit, or we call process.kill()
171-
which sends a SIGKILL equivalent signal.
172-
173-
Args:
174-
process: The process to terminate
175-
"""
176-
try:
177-
process.terminate()
178-
with anyio.fail_after(2.0):
179-
await process.wait()
180-
except TimeoutError:
181-
# Force kill if it doesn't terminate
182-
process.kill()

‎tests/client/test_stdio.py‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import shutil
2+
import sys
3+
import textwrap
4+
import time
25

6+
import anyio
37
import pytest
48

59
from mcp.client.session import ClientSession
@@ -90,3 +94,121 @@ async def test_stdio_client_nonexistent_command():
9094
or "not found" in error_message.lower()
9195
or "cannot find the file" in error_message.lower() # Windows error message
9296
)
97+
98+
99+
@pytest.mark.anyio
100+
async def test_stdio_client_universal_cleanup():
101+
"""
102+
Test that stdio_client completes cleanup within reasonable time
103+
even when connected to processes that exit slowly.
104+
"""
105+
106+
# Use a Python script that simulates a long-running process
107+
# This ensures consistent behavior across platforms
108+
long_running_script = textwrap.dedent(
109+
"""
110+
import time
111+
import sys
112+
113+
# Simulate a long-running process
114+
for i in range(100):
115+
time.sleep(0.1)
116+
# Flush to ensure output is visible
117+
sys.stdout.flush()
118+
sys.stderr.flush()
119+
"""
120+
)
121+
122+
server_params = StdioServerParameters(
123+
command=sys.executable,
124+
args=["-c", long_running_script],
125+
)
126+
127+
start_time = time.time()
128+
129+
with anyio.move_on_after(8.0) as cancel_scope:
130+
async with stdio_client(server_params) as (read_stream, write_stream):
131+
# Immediately exit - this triggers cleanup while process is still running
132+
pass
133+
134+
end_time = time.time()
135+
elapsed = end_time - start_time
136+
137+
# On Windows: 2s (stdin wait) + 2s (terminate wait) + overhead = ~5s expected
138+
assert elapsed < 6.0, (
139+
f"stdio_client cleanup took {elapsed:.1f} seconds, expected < 6.0 seconds. "
140+
f"This suggests the timeout mechanism may not be working properly."
141+
)
142+
143+
# Check if we timed out
144+
if cancel_scope.cancelled_caught:
145+
pytest.fail(
146+
"stdio_client cleanup timed out after 8.0 seconds. "
147+
"This indicates the cleanup mechanism is hanging and needs fixing."
148+
)
149+
150+
151+
@pytest.mark.anyio
152+
@pytest.mark.skipif(sys.platform == "win32", reason="Windows signal handling is different")
153+
async def test_stdio_client_sigint_only_process():
154+
"""
155+
Test cleanup with a process that ignores SIGTERM but responds to SIGINT.
156+
"""
157+
# Create a Python script that ignores SIGTERM but handles SIGINT
158+
script_content = textwrap.dedent(
159+
"""
160+
import signal
161+
import sys
162+
import time
163+
164+
# Ignore SIGTERM (what process.terminate() sends)
165+
signal.signal(signal.SIGTERM, signal.SIG_IGN)
166+
167+
# Handle SIGINT (Ctrl+C signal) by exiting cleanly
168+
def sigint_handler(signum, frame):
169+
sys.exit(0)
170+
171+
signal.signal(signal.SIGINT, sigint_handler)
172+
173+
# Keep running until SIGINT received
174+
while True:
175+
time.sleep(0.1)
176+
"""
177+
)
178+
179+
server_params = StdioServerParameters(
180+
command=sys.executable,
181+
args=["-c", script_content],
182+
)
183+
184+
start_time = time.time()
185+
186+
try:
187+
# Use anyio timeout to prevent test from hanging forever
188+
with anyio.move_on_after(5.0) as cancel_scope:
189+
async with stdio_client(server_params) as (read_stream, write_stream):
190+
# Let the process start and begin ignoring SIGTERM
191+
await anyio.sleep(0.5)
192+
# Exit context triggers cleanup - this should not hang
193+
pass
194+
195+
if cancel_scope.cancelled_caught:
196+
raise TimeoutError("Test timed out")
197+
198+
end_time = time.time()
199+
elapsed = end_time - start_time
200+
201+
# Should complete quickly even with SIGTERM-ignoring process
202+
# This will fail if cleanup only uses process.terminate() without fallback
203+
assert elapsed < 5.0, (
204+
f"stdio_client cleanup took {elapsed:.1f} seconds with SIGTERM-ignoring process. "
205+
f"Expected < 5.0 seconds. This suggests the cleanup needs SIGINT/SIGKILL fallback."
206+
)
207+
except (TimeoutError, Exception) as e:
208+
if isinstance(e, TimeoutError) or "timed out" in str(e):
209+
pytest.fail(
210+
"stdio_client cleanup timed out after 5.0 seconds with SIGTERM-ignoring process. "
211+
"This confirms the cleanup needs SIGINT/SIGKILL fallback for processes that ignore SIGTERM."
212+
)
213+
else:
214+
raise

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL