Skip to content

Commit 160999f

Browse files
committed
Reap process and close pipe transports after _terminate_process_tree
_terminate_process_tree kills the OS process group / Job Object but does not call process.wait() or close the anyio stdin/stdout pipe wrappers. On Windows, the resulting _WindowsSubprocessTransport / PipeHandle objects are GC'd after the test's filterwarnings scope has exited, triggering PytestUnraisableExceptionWarning in whatever test runs next on the same worker. The old file-watching tests masked this because their ~1.8s of sleep() calls per test gave the asyncio event loop's child watcher time to fire _process_exited() callbacks (which close the transport) during the test. The new socket-based tests are ~0.15s each — too fast for that callback to fire before GC. The SDK's production path (stdio.py:180) avoids this via 'async with process:', whose __aexit__ handles reaping + stream closure. These tests call _terminate_process_tree directly, so they must do the same cleanup explicitly. Added _terminate_and_reap() helper that runs unconditionally in each test's finally: terminate tree (no-op if already dead), await wait() to reap, close stdin/stdout pipe wrappers. Bounded by move_on_after(5). Github-Issue: #1775
1 parent 324bbc2 commit 160999f

File tree

1 file changed

+33
-12
lines changed

1 file changed

+33
-12
lines changed

tests/client/test_stdio.py

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
_terminate_process_tree,
1616
stdio_client,
1717
)
18+
from mcp.os.win32.utilities import FallbackProcess
1819
from mcp.shared.exceptions import MCPError
1920
from mcp.shared.message import SessionMessage
2021
from mcp.types import CONNECTION_CLOSED, JSONRPCMessage, JSONRPCRequest, JSONRPCResponse
@@ -311,6 +312,32 @@ async def _assert_stream_closed(stream: anyio.abc.SocketStream) -> None:
311312
)
312313

313314

315+
async def _terminate_and_reap(proc: anyio.abc.Process | FallbackProcess) -> None:
316+
"""Terminate the process tree, then reap and close pipe transports.
317+
318+
``_terminate_process_tree`` kills the OS process group / Job Object but does
319+
not call ``process.wait()`` or close stdin/stdout pipe wrappers. On Windows,
320+
the resulting ``_WindowsSubprocessTransport`` / ``PipeHandle`` objects are
321+
then GC'd after the test's ``filterwarnings`` scope has exited, triggering
322+
``PytestUnraisableExceptionWarning`` in whatever test happens to run next.
323+
324+
The SDK's production path (``stdio.py``) avoids this via ``async with process:``
325+
which wraps termination in a context manager whose ``__aexit__`` handles
326+
reaping + stream closure. These tests call ``_terminate_process_tree``
327+
directly, so they must do the same cleanup explicitly.
328+
329+
Safe to call on an already-terminated process (termination no-ops, wait
330+
returns immediately). Bounded by ``move_on_after`` to prevent hangs.
331+
"""
332+
with anyio.move_on_after(5.0):
333+
await _terminate_process_tree(proc)
334+
await proc.wait()
335+
if proc.stdin is not None: # pragma: no branch
336+
await proc.stdin.aclose()
337+
if proc.stdout is not None: # pragma: no branch
338+
await proc.stdout.aclose()
339+
340+
314341
class TestChildProcessCleanup:
315342
"""Integration tests for ``_terminate_process_tree`` covering basic,
316343
nested, and early-parent-exit process tree scenarios. See module-level
@@ -335,15 +362,13 @@ async def test_basic_child_process_cleanup(self):
335362

336363
# Terminate the process tree (the behavior under test).
337364
await _terminate_process_tree(proc)
338-
proc = None
339365

340366
# Deterministic: kernel closed child's socket when it died.
341367
await _assert_stream_closed(stream)
342368

343369
finally:
344-
if proc is not None: # pragma: no cover
345-
with anyio.move_on_after(5.0):
346-
await _terminate_process_tree(proc)
370+
if proc is not None: # pragma: no branch
371+
await _terminate_and_reap(proc)
347372
if stream is not None: # pragma: no branch
348373
await stream.aclose()
349374
await sock.aclose()
@@ -378,16 +403,14 @@ async def test_nested_process_tree(self):
378403

379404
# Terminate the entire tree.
380405
await _terminate_process_tree(proc)
381-
proc = None
382406

383407
# Every level of the tree must be dead: three kernel-level EOFs.
384408
for stream in streams:
385409
await _assert_stream_closed(stream)
386410

387411
finally:
388-
if proc is not None: # pragma: no cover
389-
with anyio.move_on_after(5.0):
390-
await _terminate_process_tree(proc)
412+
if proc is not None: # pragma: no branch
413+
await _terminate_and_reap(proc)
391414
for stream in streams:
392415
await stream.aclose()
393416
await sock.aclose()
@@ -420,15 +443,13 @@ async def test_early_parent_exit(self):
420443
# Parent will sys.exit(0) on SIGTERM, but the process-group kill
421444
# (POSIX killpg / Windows Job Object) must still terminate the child.
422445
await _terminate_process_tree(proc)
423-
proc = None
424446

425447
# Child must be dead despite parent's early exit.
426448
await _assert_stream_closed(stream)
427449

428450
finally:
429-
if proc is not None: # pragma: no cover
430-
with anyio.move_on_after(5.0):
431-
await _terminate_process_tree(proc)
451+
if proc is not None: # pragma: no branch
452+
await _terminate_and_reap(proc)
432453
if stream is not None: # pragma: no branch
433454
await stream.aclose()
434455
await sock.aclose()

0 commit comments

Comments
 (0)