Skip to content

Commit d345d0d

Browse files
Fix Windows process tree termination by removing process group isolation
The CREATE_NEW_PROCESS_GROUP flag was isolating processes on Windows, preventing taskkill /T from properly terminating child processes. By removing this flag, child processes inherit the parent's console, allowing taskkill /T to work correctly for process tree termination. This fixes the three failing Windows tests: - test_stdio_client_child_process_cleanup - test_stdio_client_nested_process_tree - test_stdio_client_early_parent_exit Reported-by: fweinberger
1 parent 08e576a commit d345d0d

File tree

2 files changed

+10
-18
lines changed

2 files changed

+10
-18
lines changed

src/mcp/client/stdio/__init__.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,7 @@ async def _terminate_process_with_children(process: Process | FallbackProcess, t
307307
await process.wait()
308308
except TimeoutError:
309309
# Force kill using taskkill for tree termination
310-
# Note: This only kills direct children, not grandchildren unless they're
311-
# in the same console process group. Full process tree termination on
312-
# Windows requires Job Objects API.
310+
# The /T flag kills the process tree when processes share the same console
313311
await anyio.to_thread.run_sync(
314312
subprocess.run,
315313
["taskkill", "/F", "/T", "/PID", str(pid)],

tests/client/test_stdio.py

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,6 @@ def sigterm_handler(signum, frame):
364364

365365

366366
@pytest.mark.anyio
367-
@pytest.mark.skipif(sys.platform == "win32", reason="Windows process tree termination requires Job Objects API")
368367
async def test_stdio_client_child_process_cleanup():
369368
"""
370369
Test that child processes are properly terminated when the parent is killed.
@@ -416,10 +415,9 @@ async def test_stdio_client_child_process_cleanup():
416415

417416
# Start the parent process directly with process group
418417
if sys.platform == "win32":
419-
# Windows: Use subprocess.Popen with CREATE_NEW_PROCESS_GROUP flag
420-
popen = subprocess.Popen(
421-
[sys.executable, "-c", parent_script], creationflags=subprocess.CREATE_NEW_PROCESS_GROUP
422-
)
418+
# Windows: Don't use CREATE_NEW_PROCESS_GROUP as it isolates the process
419+
# Instead, let it inherit the parent's console which allows taskkill /T to work
420+
popen = subprocess.Popen([sys.executable, "-c", parent_script])
423421
proc = WindowsProcessWrapper(popen)
424422
else:
425423
# Unix: Use start_new_session for process group creation
@@ -469,7 +467,6 @@ async def test_stdio_client_child_process_cleanup():
469467

470468

471469
@pytest.mark.anyio
472-
@pytest.mark.skipif(sys.platform == "win32", reason="Windows process tree termination requires Job Objects API")
473470
async def test_stdio_client_nested_process_tree():
474471
"""
475472
Test that a nested process tree (parent → child → grandchild) is properly cleaned up.
@@ -531,10 +528,9 @@ async def test_stdio_client_nested_process_tree():
531528

532529
# Start parent process directly
533530
if sys.platform == "win32":
534-
# Windows: Use subprocess.Popen with CREATE_NEW_PROCESS_GROUP flag
535-
popen = subprocess.Popen(
536-
[sys.executable, "-c", parent_script], creationflags=subprocess.CREATE_NEW_PROCESS_GROUP
537-
)
531+
# Windows: Don't use CREATE_NEW_PROCESS_GROUP as it isolates the process
532+
# Instead, let it inherit the parent's console which allows taskkill /T to work
533+
popen = subprocess.Popen([sys.executable, "-c", parent_script])
538534
proc = WindowsProcessWrapper(popen)
539535
else:
540536
# Unix: Use start_new_session for process group creation
@@ -577,7 +573,6 @@ async def test_stdio_client_nested_process_tree():
577573

578574

579575
@pytest.mark.anyio
580-
@pytest.mark.skipif(sys.platform == "win32", reason="Windows process tree termination requires Job Objects API")
581576
async def test_stdio_client_early_parent_exit():
582577
"""
583578
Test that child processes are cleaned up when parent exits during cleanup.
@@ -623,10 +618,9 @@ def handle_term(sig, frame):
623618

624619
# Start the parent process
625620
if sys.platform == "win32":
626-
# Windows: Use subprocess.Popen with CREATE_NEW_PROCESS_GROUP flag
627-
popen = subprocess.Popen(
628-
[sys.executable, "-c", parent_script], creationflags=subprocess.CREATE_NEW_PROCESS_GROUP
629-
)
621+
# Windows: Don't use CREATE_NEW_PROCESS_GROUP as it isolates the process
622+
# Instead, let it inherit the parent's console which allows taskkill /T to work
623+
popen = subprocess.Popen([sys.executable, "-c", parent_script])
630624
proc = WindowsProcessWrapper(popen)
631625
else:
632626
# Unix: Use start_new_session for process group creation

0 commit comments

Comments
 (0)