Skip to content

Commit b7e8a56

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 a60d852 commit b7e8a56

File tree

1 file changed

+161
-0
lines changed

1 file changed

+161
-0
lines changed

tests/client/test_stdio.py

Lines changed: 161 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,160 @@ 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 Python script that prints and exits immediately
154+
# This is more consistent across platforms than echo
155+
script_content = "print('hello')"
156+
157+
server_params = StdioServerParameters(
158+
command=sys.executable,
159+
args=["-c", script_content],
160+
)
161+
162+
start_time = time.time()
163+
164+
# Use move_on_after which is more reliable for cleanup scenarios
165+
with anyio.move_on_after(3.0) as cancel_scope:
166+
async with stdio_client(server_params) as (read_stream, write_stream):
167+
pass
168+
169+
end_time = time.time()
170+
elapsed = end_time - start_time
171+
172+
# Should complete very quickly when process exits normally
173+
assert elapsed < 2.0, (
174+
f"stdio_client took {elapsed:.1f} seconds for fast-exiting process, "
175+
f"expected < 2.0 seconds. Timeout mechanism may be introducing delays."
176+
)
177+
178+
# Check if we timed out
179+
if cancel_scope.cancelled_caught:
180+
pytest.fail(
181+
"stdio_client timed out after 3.0 seconds for fast-exiting process. "
182+
"This indicates a serious hang in the cleanup mechanism."
183+
)
184+
185+
186+
@pytest.mark.anyio
187+
@pytest.mark.skipif(sys.platform == "win32", reason="Windows signal handling is different")
188+
async def test_stdio_client_sigint_only_process():
189+
"""
190+
Test cleanup with a process that ignores SIGTERM but responds to SIGINT.
191+
192+
This tests Cristipufu's concern: processes that expect SIGINT (like many
193+
Node.js servers or interactive tools) may not respond to SIGTERM, causing
194+
hanging during cleanup.
195+
"""
196+
# Create a Python script that ignores SIGTERM but handles SIGINT
197+
script_content = textwrap.dedent(
198+
"""
199+
import signal
200+
import sys
201+
import time
202+
203+
# Ignore SIGTERM (what process.terminate() sends)
204+
signal.signal(signal.SIGTERM, signal.SIG_IGN)
205+
206+
# Handle SIGINT (Ctrl+C signal) by exiting cleanly
207+
def sigint_handler(signum, frame):
208+
sys.exit(0)
209+
210+
signal.signal(signal.SIGINT, sigint_handler)
211+
212+
# Keep running until SIGINT received
213+
while True:
214+
time.sleep(0.1)
215+
"""
216+
)
217+
218+
server_params = StdioServerParameters(
219+
command="python",
220+
args=["-c", script_content],
221+
)
222+
223+
start_time = time.time()
224+
225+
try:
226+
# Use anyio timeout to prevent test from hanging forever
227+
with anyio.move_on_after(5.0) as cancel_scope:
228+
async with stdio_client(server_params) as (read_stream, write_stream):
229+
# Let the process start and begin ignoring SIGTERM
230+
await anyio.sleep(0.5)
231+
# Exit context triggers cleanup - this should not hang
232+
pass
233+
234+
if cancel_scope.cancelled_caught:
235+
raise TimeoutError("Test timed out")
236+
237+
end_time = time.time()
238+
elapsed = end_time - start_time
239+
240+
# Should complete quickly even with SIGTERM-ignoring process
241+
# This will fail if cleanup only uses process.terminate() without fallback
242+
assert elapsed < 5.0, (
243+
f"stdio_client cleanup took {elapsed:.1f} seconds with SIGTERM-ignoring process. "
244+
f"Expected < 5.0 seconds. This suggests the cleanup needs SIGINT/SIGKILL fallback."
245+
)
246+
except (TimeoutError, Exception) as e:
247+
if isinstance(e, TimeoutError) or "timed out" in str(e):
248+
pytest.fail(
249+
"stdio_client cleanup timed out after 5.0 seconds with SIGTERM-ignoring process. "
250+
"This confirms the cleanup needs SIGINT/SIGKILL fallback for processes that ignore SIGTERM."
251+
)
252+
else:
253+
raise

0 commit comments

Comments
 (0)