Skip to content

Commit 954020b

Browse files
Add regression tests for child process termination
- test_stdio_client_child_process_cleanup: Verifies that child processes spawned by the parent (like npx spawning node) are properly terminated - test_stdio_client_nested_process_tree: Tests that deeply nested process trees (parent -> child -> grandchild) are all terminated - test_stdio_client_early_parent_exit: Tests the race condition where parent exits during cleanup but children are still terminated via process group These tests verify the fix works correctly across different subprocess spawning scenarios without requiring external dependencies like psutil.
1 parent 0ac8b69 commit 954020b

File tree

1 file changed

+277
-0
lines changed

1 file changed

+277
-0
lines changed

tests/client/test_stdio.py

Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import os
12
import shutil
23
import sys
4+
import tempfile
35
import textwrap
46
import time
57

@@ -323,3 +325,278 @@ def sigterm_handler(signum, frame):
323325
f"stdio_client cleanup took {elapsed:.1f} seconds for stdin-ignoring process. "
324326
f"Expected between 2-4 seconds (2s stdin timeout + termination time)."
325327
)
328+
329+
330+
@pytest.mark.anyio
331+
async def test_stdio_client_child_process_cleanup():
332+
"""
333+
Test that child processes are properly terminated when the parent is killed.
334+
This addresses the issue where processes like npx spawn child processes
335+
that need to be cleaned up.
336+
"""
337+
338+
# Create a marker file for the child process to write to
339+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
340+
marker_file = f.name
341+
342+
# Also create a file to verify parent started
343+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
344+
parent_marker = f.name
345+
346+
try:
347+
# Parent script that spawns a child process
348+
parent_script = textwrap.dedent(
349+
f"""
350+
import subprocess
351+
import sys
352+
import time
353+
import os
354+
355+
# Mark that parent started
356+
with open({repr(parent_marker)}, 'w') as f:
357+
f.write('parent started\\n')
358+
359+
# Child script that writes continuously
360+
child_script = '''
361+
import time
362+
with open({repr(marker_file)}, 'a') as f:
363+
while True:
364+
f.write(f"{{time.time()}}\\\\n")
365+
f.flush()
366+
time.sleep(0.1)
367+
'''
368+
369+
# Start the child process
370+
child = subprocess.Popen([sys.executable, '-c', child_script])
371+
372+
# Parent just sleeps
373+
while True:
374+
time.sleep(0.1)
375+
"""
376+
)
377+
378+
print("\nStarting child process termination test...")
379+
380+
# Start the parent process directly with process group
381+
proc = await anyio.open_process([sys.executable, "-c", parent_script], start_new_session=True)
382+
383+
# Wait for processes to start
384+
await anyio.sleep(0.5)
385+
386+
# Verify parent started
387+
assert os.path.exists(parent_marker), "Parent process didn't start"
388+
389+
# Verify child is writing
390+
if os.path.exists(marker_file):
391+
initial_size = os.path.getsize(marker_file)
392+
await anyio.sleep(0.3)
393+
size_after_wait = os.path.getsize(marker_file)
394+
assert size_after_wait > initial_size, "Child process should be writing"
395+
print(f"Child is writing (file grew from {initial_size} to {size_after_wait} bytes)")
396+
397+
# Terminate using our function
398+
print("Terminating process and children...")
399+
from mcp.client.stdio import _terminate_process_with_children
400+
401+
await _terminate_process_with_children(proc)
402+
403+
# Verify processes stopped
404+
await anyio.sleep(0.5)
405+
if os.path.exists(marker_file):
406+
size_after_cleanup = os.path.getsize(marker_file)
407+
await anyio.sleep(0.5)
408+
final_size = os.path.getsize(marker_file)
409+
410+
print(f"After cleanup: file size {size_after_cleanup} -> {final_size}")
411+
assert (
412+
final_size == size_after_cleanup
413+
), f"Child process still running! File grew by {final_size - size_after_cleanup} bytes"
414+
415+
print("SUCCESS: Child process was properly terminated")
416+
417+
finally:
418+
# Clean up files
419+
for f in [marker_file, parent_marker]:
420+
try:
421+
os.unlink(f)
422+
except OSError:
423+
pass
424+
425+
426+
@pytest.mark.anyio
427+
async def test_stdio_client_nested_process_tree():
428+
"""
429+
Test that a nested process tree (parent → child → grandchild) is properly cleaned up.
430+
Each level writes to a different file to verify all processes are terminated.
431+
"""
432+
433+
# Create temporary files for each process level
434+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f1:
435+
parent_file = f1.name
436+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f2:
437+
child_file = f2.name
438+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f3:
439+
grandchild_file = f3.name
440+
441+
try:
442+
# Simple nested process tree test
443+
# We create parent -> child -> grandchild, each writing to a file
444+
parent_script = textwrap.dedent(
445+
f"""
446+
import subprocess
447+
import sys
448+
import time
449+
import os
450+
451+
# Child will spawn grandchild and write to child file
452+
child_script = '''import subprocess
453+
import sys
454+
import time
455+
456+
# Grandchild just writes to file
457+
grandchild_script = \\"\\"\\"import time
458+
with open({repr(grandchild_file)}, 'a') as f:
459+
while True:
460+
f.write(f"gc {{time.time()}}\\\\\\\\n")
461+
f.flush()
462+
time.sleep(0.1)\\"\\"\\"
463+
464+
# Spawn grandchild
465+
subprocess.Popen([sys.executable, '-c', grandchild_script])
466+
467+
# Child writes to its file
468+
with open({repr(child_file)}, 'a') as f:
469+
while True:
470+
f.write(f"c {{time.time()}}\\\\\\\\n")
471+
f.flush()
472+
time.sleep(0.1)'''
473+
474+
# Spawn child process
475+
subprocess.Popen([sys.executable, '-c', child_script])
476+
477+
# Parent writes to its file
478+
with open({repr(parent_file)}, 'a') as f:
479+
while True:
480+
f.write(f"p {{time.time()}}\\\\n")
481+
f.flush()
482+
time.sleep(0.1)
483+
"""
484+
)
485+
486+
# Start parent process directly
487+
proc = await anyio.open_process([sys.executable, "-c", parent_script], start_new_session=True)
488+
489+
# Let all processes start
490+
await anyio.sleep(1.0)
491+
492+
# Verify all are writing
493+
for file_path, name in [(parent_file, "parent"), (child_file, "child"), (grandchild_file, "grandchild")]:
494+
if os.path.exists(file_path):
495+
initial_size = os.path.getsize(file_path)
496+
await anyio.sleep(0.3)
497+
new_size = os.path.getsize(file_path)
498+
assert new_size > initial_size, f"{name} process should be writing"
499+
500+
# Terminate the whole tree
501+
from mcp.client.stdio import _terminate_process_with_children
502+
503+
await _terminate_process_with_children(proc)
504+
505+
# Verify all stopped
506+
await anyio.sleep(0.5)
507+
for file_path, name in [(parent_file, "parent"), (child_file, "child"), (grandchild_file, "grandchild")]:
508+
if os.path.exists(file_path):
509+
size1 = os.path.getsize(file_path)
510+
await anyio.sleep(0.3)
511+
size2 = os.path.getsize(file_path)
512+
assert size1 == size2, f"{name} still writing after cleanup!"
513+
514+
print("SUCCESS: All processes in tree terminated")
515+
516+
finally:
517+
# Clean up all marker files
518+
for f in [parent_file, child_file, grandchild_file]:
519+
try:
520+
os.unlink(f)
521+
except OSError:
522+
pass
523+
524+
525+
@pytest.mark.anyio
526+
async def test_stdio_client_early_parent_exit():
527+
"""
528+
Test that child processes are cleaned up when parent exits during cleanup.
529+
This tests the race condition where parent might die during our termination
530+
sequence but we can still clean up the children via the process group.
531+
"""
532+
533+
# Create a temporary file for the child
534+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
535+
marker_file = f.name
536+
537+
try:
538+
# Parent that spawns child and waits briefly
539+
parent_script = textwrap.dedent(
540+
f"""
541+
import subprocess
542+
import sys
543+
import time
544+
import signal
545+
546+
# Child that continues running
547+
child_script = '''import time
548+
with open({repr(marker_file)}, 'a') as f:
549+
while True:
550+
f.write(f"child {{time.time()}}\\\\n")
551+
f.flush()
552+
time.sleep(0.1)'''
553+
554+
# Start child in same process group
555+
subprocess.Popen([sys.executable, '-c', child_script])
556+
557+
# Parent waits a bit then exits on SIGTERM
558+
def handle_term(sig, frame):
559+
sys.exit(0)
560+
561+
signal.signal(signal.SIGTERM, handle_term)
562+
563+
# Wait
564+
while True:
565+
time.sleep(0.1)
566+
"""
567+
)
568+
569+
# Start the parent process
570+
proc = await anyio.open_process([sys.executable, "-c", parent_script], start_new_session=True)
571+
572+
# Let child start writing
573+
await anyio.sleep(0.5)
574+
575+
# Verify child is writing
576+
if os.path.exists(marker_file):
577+
size1 = os.path.getsize(marker_file)
578+
await anyio.sleep(0.3)
579+
size2 = os.path.getsize(marker_file)
580+
assert size2 > size1, "Child should be writing"
581+
582+
# Terminate - this will kill the process group even if parent exits first
583+
from mcp.client.stdio import _terminate_process_with_children
584+
585+
await _terminate_process_with_children(proc)
586+
587+
# Verify child stopped
588+
await anyio.sleep(0.5)
589+
if os.path.exists(marker_file):
590+
size3 = os.path.getsize(marker_file)
591+
await anyio.sleep(0.3)
592+
size4 = os.path.getsize(marker_file)
593+
assert size3 == size4, "Child should be terminated"
594+
595+
print("SUCCESS: Child terminated even with parent exit during cleanup")
596+
597+
finally:
598+
# Clean up marker file
599+
try:
600+
os.unlink(marker_file)
601+
except OSError:
602+
pass

0 commit comments

Comments
 (0)