Address @ihrpr review feedback

- Add SIGTERM_IGNORING_PROCESS_TIMEOUT constant in tests to document timeout behavior
- Add PROCESS_TERMINATION_TIMEOUT constant to replace magic number in stdio client
- Restore deprecated terminate_windows_process function with original functionality
  to maintain backward compatibility for external users

The deprecated function is marked using @deprecated decorator following the
codebase convention, while preserving its original terminate-wait-kill behavior.

Author: felixweinberger

src/mcp/client/stdio/__init__.py

@@ -37,6 +37,9 @@
     else ["HOME", "LOGNAME", "PATH", "SHELL", "TERM", "USER"]
 )
 
+# Timeout for process termination before falling back to force kill
+PROCESS_TERMINATION_TIMEOUT = 2.0
+
 
 def get_default_environment() -> dict[str, str]:
     """
@@ -180,7 +183,7 @@ async def stdin_writer():
             # Clean up process to prevent any dangling orphaned processes
             try:
                 process.terminate()
-                with anyio.fail_after(2.0):
+                with anyio.fail_after(PROCESS_TERMINATION_TIMEOUT):
                     await process.wait()
             except TimeoutError:
                 # If process doesn't terminate in time, force kill it

src/mcp/client/stdio/win32.py

@@ -12,6 +12,7 @@
 from anyio import to_thread
 from anyio.abc import Process
 from anyio.streams.file import FileReadStream, FileWriteStream
+from typing_extensions import deprecated
 
 
 def get_windows_executable_command(command: str) -> str:
@@ -199,3 +200,28 @@ async def _create_windows_fallback_process(
             bufsize=0,
         )
         return FallbackProcess(popen_obj)
+
+
+@deprecated(
+    "terminate_windows_process is deprecated and will be removed in a future version. "
+    "Process termination is now handled internally by the stdio_client context manager."
+)
+async def terminate_windows_process(process: Process | FallbackProcess):
+    """
+    Terminate a Windows process.
+
+    Note: On Windows, terminating a process with process.terminate() doesn't
+    always guarantee immediate process termination.
+    So we give it 2s to exit, or we call process.kill()
+    which sends a SIGKILL equivalent signal.
+
+    Args:
+        process: The process to terminate
+    """
+    try:
+        process.terminate()
+        with anyio.fail_after(2.0):
+            await process.wait()
+    except TimeoutError:
+        # Force kill if it doesn't terminate
+        process.kill()

tests/client/test_stdio.py

@@ -15,6 +15,11 @@
 from mcp.shared.message import SessionMessage
 from mcp.types import CONNECTION_CLOSED, JSONRPCMessage, JSONRPCRequest, JSONRPCResponse
 
+# Timeout for cleanup of processes that ignore SIGTERM
+# This timeout ensures the test fails quickly if the cleanup logic doesn't have
+# proper fallback mechanisms (SIGINT/SIGKILL) for processes that ignore SIGTERM
+SIGTERM_IGNORING_PROCESS_TIMEOUT = 5.0
+
 tee: str = shutil.which("tee")  # type: ignore
 python: str = shutil.which("python")  # type: ignore
 
@@ -200,14 +205,16 @@ def sigint_handler(signum, frame):
 
         # Should complete quickly even with SIGTERM-ignoring process
         # This will fail if cleanup only uses process.terminate() without fallback
-        assert elapsed < 5.0, (
+        assert elapsed < SIGTERM_IGNORING_PROCESS_TIMEOUT, (
             f"stdio_client cleanup took {elapsed:.1f} seconds with SIGTERM-ignoring process. "
-            f"Expected < 5.0 seconds. This suggests the cleanup needs SIGINT/SIGKILL fallback."
+            f"Expected < {SIGTERM_IGNORING_PROCESS_TIMEOUT} seconds. "
+            "This suggests the cleanup needs SIGINT/SIGKILL fallback."
         )
     except (TimeoutError, Exception) as e:
         if isinstance(e, TimeoutError) or "timed out" in str(e):
             pytest.fail(
-                "stdio_client cleanup timed out after 5.0 seconds with SIGTERM-ignoring process. "
+                f"stdio_client cleanup timed out after {SIGTERM_IGNORING_PROCESS_TIMEOUT} seconds "
+                "with SIGTERM-ignoring process. "
                 "This confirms the cleanup needs SIGINT/SIGKILL fallback for processes that ignore SIGTERM."
             )
         else: