Skip to content

Commit 9f88ea7

Browse files
Add regression tests for stdio cleanup hanging
**NOTE: These tests FAIL without the changes introduced in #559** - test_stdio_client_universal_timeout - test_stdio_client_immediate_completion await read_stream.aclose() await write_stream.aclose() await read_stream_writer.aclose() await write_stream_reader.aclose() These tests verify that stdio_client completes cleanup within reasonable time for both slow-terminating and fast-exiting processes, preventing the hanging issues reported in #559. **NOTE: This test FAILS without the changes introduced in #555** - test_stdio_client_sigint_only_process try: process.terminate() with anyio.fail_after(2.0): await process.wait() except TimeoutError: # If process doesn't terminate in time, force kill it process.kill() This test verifies that on UNIX systems MCP servers that don't respect SIGTERM but e.g. SIGINT still get terminated after a grace period.
1 parent 4b4637a commit 9f88ea7

File tree

1 file changed

+164
-0
lines changed

1 file changed

+164
-0
lines changed

tests/client/test_stdio.py

Lines changed: 164 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,163 @@ 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 simple sleep command that's available on all platforms
107+
# This simulates a process that takes time to terminate
108+
if sys.platform == "win32":
109+
# Windows: use ping with timeout to simulate a running process
110+
server_params = StdioServerParameters(
111+
command="ping",
112+
args=["127.0.0.1", "-n", "10"], # Ping 10 times, takes ~10 seconds
113+
)
114+
else:
115+
# Unix: use sleep command
116+
server_params = StdioServerParameters(
117+
command="sleep",
118+
args=["10"], # Sleep for 10 seconds
119+
)
120+
121+
start_time = time.time()
122+
123+
# Use move_on_after which is more reliable for cleanup scenarios
124+
with anyio.move_on_after(6.0) as cancel_scope:
125+
async with stdio_client(server_params) as (read_stream, write_stream):
126+
# Immediately exit - this triggers cleanup while process is still running
127+
pass
128+
129+
end_time = time.time()
130+
elapsed = end_time - start_time
131+
132+
# Key assertion: Should complete quickly due to timeout mechanism
133+
assert elapsed < 5.0, (
134+
f"stdio_client cleanup took {elapsed:.1f} seconds, expected < 5.0 seconds. "
135+
f"This suggests the timeout mechanism may not be working properly."
136+
)
137+
138+
# Check if we timed out
139+
if cancel_scope.cancelled_caught:
140+
pytest.fail(
141+
"stdio_client cleanup timed out after 6.0 seconds. "
142+
"This indicates the cleanup mechanism is hanging and needs fixing."
143+
)
144+
145+
146+
@pytest.mark.anyio
147+
async def test_stdio_client_immediate_completion():
148+
"""
149+
Test that stdio_client doesn't introduce unnecessary delays
150+
when processes exit normally and quickly.
151+
"""
152+
153+
# Use a command that exits immediately
154+
if sys.platform == "win32":
155+
server_params = StdioServerParameters(
156+
command="cmd",
157+
args=["/c", "echo", "hello"], # Windows: echo and exit
158+
)
159+
else:
160+
server_params = StdioServerParameters(
161+
command="echo",
162+
args=["hello"], # Unix: echo and exit
163+
)
164+
165+
start_time = time.time()
166+
167+
# Use move_on_after which is more reliable for cleanup scenarios
168+
with anyio.move_on_after(3.0) as cancel_scope:
169+
async with stdio_client(server_params) as (read_stream, write_stream):
170+
pass
171+
172+
end_time = time.time()
173+
elapsed = end_time - start_time
174+
175+
# Should complete very quickly when process exits normally
176+
assert elapsed < 2.0, (
177+
f"stdio_client took {elapsed:.1f} seconds for fast-exiting process, "
178+
f"expected < 2.0 seconds. Timeout mechanism may be introducing delays."
179+
)
180+
181+
# Check if we timed out
182+
if cancel_scope.cancelled_caught:
183+
pytest.fail(
184+
"stdio_client timed out after 3.0 seconds for fast-exiting process. "
185+
"This indicates a serious hang in the cleanup mechanism."
186+
)
187+
188+
189+
@pytest.mark.anyio
190+
@pytest.mark.skipif(sys.platform == "win32", reason="Windows signal handling is different")
191+
async def test_stdio_client_sigint_only_process():
192+
"""
193+
Test cleanup with a process that ignores SIGTERM but responds to SIGINT.
194+
195+
This tests Cristipufu's concern: processes that expect SIGINT (like many
196+
Node.js servers or interactive tools) may not respond to SIGTERM, causing
197+
hanging during cleanup.
198+
"""
199+
# Create a Python script that ignores SIGTERM but handles SIGINT
200+
script_content = textwrap.dedent(
201+
"""
202+
import signal
203+
import sys
204+
import time
205+
206+
# Ignore SIGTERM (what process.terminate() sends)
207+
signal.signal(signal.SIGTERM, signal.SIG_IGN)
208+
209+
# Handle SIGINT (Ctrl+C signal) by exiting cleanly
210+
def sigint_handler(signum, frame):
211+
sys.exit(0)
212+
213+
signal.signal(signal.SIGINT, sigint_handler)
214+
215+
# Keep running until SIGINT received
216+
while True:
217+
time.sleep(0.1)
218+
"""
219+
)
220+
221+
server_params = StdioServerParameters(
222+
command="python",
223+
args=["-c", script_content],
224+
)
225+
226+
start_time = time.time()
227+
228+
try:
229+
# Use anyio timeout to prevent test from hanging forever
230+
with anyio.move_on_after(5.0) as cancel_scope:
231+
async with stdio_client(server_params) as (read_stream, write_stream):
232+
# Let the process start and begin ignoring SIGTERM
233+
await anyio.sleep(0.5)
234+
# Exit context triggers cleanup - this should not hang
235+
pass
236+
237+
if cancel_scope.cancelled_caught:
238+
raise TimeoutError("Test timed out")
239+
240+
end_time = time.time()
241+
elapsed = end_time - start_time
242+
243+
# Should complete quickly even with SIGTERM-ignoring process
244+
# This will fail if cleanup only uses process.terminate() without fallback
245+
assert elapsed < 5.0, (
246+
f"stdio_client cleanup took {elapsed:.1f} seconds with SIGTERM-ignoring process. "
247+
f"Expected < 5.0 seconds. This suggests the cleanup needs SIGINT/SIGKILL fallback."
248+
)
249+
except (TimeoutError, Exception) as e:
250+
if isinstance(e, TimeoutError) or "timed out" in str(e):
251+
pytest.fail(
252+
"stdio_client cleanup timed out after 5.0 seconds with SIGTERM-ignoring process. "
253+
"This confirms the cleanup needs SIGINT/SIGKILL fallback for processes that ignore SIGTERM."
254+
)
255+
else:
256+
raise

0 commit comments

Comments
 (0)