René's URL Explorer Experiment

Title: Rewrite TestChildProcessCleanup with socket-based deterministic liveness probe by maxisbey · Pull Request #2265 · modelcontextprotocol/python-sdk · GitHub

Open Graph Title: Rewrite TestChildProcessCleanup with socket-based deterministic liveness probe by maxisbey · Pull Request #2265 · modelcontextprotocol/python-sdk

X Title: Rewrite TestChildProcessCleanup with socket-based deterministic liveness probe by maxisbey · Pull Request #2265 · modelcontextprotocol/python-sdk

Description: Summary Closes #1775. Supersedes #2071. The three TestChildProcessCleanup tests in tests/client/test_stdio.py failed intermittently on Windows/macOS CI. The root cause was a timing-dependent test design: watching a file grow on disk as a proxy for "is the child process alive?", with fixed anyio.sleep() durations to "wait long enough" for nested Python interpreters to start. On loaded CI, "long enough" wasn't. A secondary bug compounded this: when the flaky assertion failed, proc was never terminated — the finally blocks only cleaned up tempfiles. The leaked subprocess was GC'd during a later test, triggering PytestUnraisableExceptionWarning there (observed: test_call_tool — an in-memory test that never touches subprocesses — failing with ResourceWarning: subprocess 5592 is still running). New design — socket-based, zero sleeps, zero polling Each subprocess in the tree connects a TCP socket back to a listener owned by the test. Two kernel-guaranteed blocking-I/O signals then tell us everything: await listener.accept() blocks until the subprocess connects → proves it started. No polling for "did the file grow yet?". await stream.receive(1) after _terminate_process_tree() raises anyio.EndOfStream (or anyio.BrokenResourceError on Windows, where TerminateJobObject causes an abrupt RST rather than clean FIN) because the kernel closes all file descriptors — including sockets — when a process terminates. This is a direct, OS-level proof that the child is dead. No polling for "did the file stop growing?". Every synchronization point is a blocking read that unblocks on a kernel event. The anyio.fail_after() wrappers are upper bounds for catastrophic failure (subprocess never started / survived termination), not timing assumptions. # Old: fixed sleep, indirect proxy, races against CI load await anyio.sleep(0.5) initial_size = os.path.getsize(marker_file) await anyio.sleep(0.3) assert os.path.getsize(marker_file) > initial_size # fails: assert 0 > 0 # New: blocking accept, kernel-level EOF signal with anyio.fail_after(10): stream = await sock.accept() # blocks until child connects assert await stream.receive(5) == b"alive" await _terminate_process_tree(proc) with pytest.raises((anyio.EndOfStream, anyio.BrokenResourceError)): await stream.receive(1) # child dead → kernel closed its socket Process + transport cleanup _terminate_process_tree kills the OS process group / Job Object but doesn't call process.wait() or close the anyio stdin/stdout pipe wrappers. On Windows, the resulting _WindowsSubprocessTransport / PipeHandle objects were GC'd later, triggering ResourceWarning in whatever test ran next. The SDK's production code at stdio.py:180 avoids this via async with process: — these tests call _terminate_process_tree directly, so they need equivalent cleanup. Fixed with _terminate_and_reap(proc) registered via AsyncExitStack.push_async_callback() immediately after process creation: terminates the tree (no-op if already dead) → await proc.wait() → close stdin/stdout pipes. Bounded by move_on_after(5). All three tests use AsyncExitStack for RAII-style resource management — socket listener, process, and accepted streams each have their cleanup callback pushed at acquisition, so there are no if x is not None: guards in finally blocks, and no coverage pragmas. This made the @pytest.mark.filterwarnings("ignore::ResourceWarning" if win32 ...) decorators unnecessary — removed. Also removed tempfile import and all file I/O escape_path_for_python import (scripts are now injected via {code!r} repr() literals, which eliminates all nested-quote escaping) ~30 lines of boilerplate textwrap.dedent blocks with triply-nested f-string scripts The parent_marker check from test 1, which was a no-op: NamedTemporaryFile(delete=False) creates the file, so os.path.exists() was always True Timing Test main PR #2265 Speedup test_basic_child_process_cleanup 1.91s 0.15s 12.7× test_nested_process_tree 3.41s 0.16s 21.3× test_early_parent_exit 1.70s 0.14s 12.1× Total 7.04s 0.46s 15.3× 30 runs × 4 parallel workers (flake-finder): ~2.1s, 30/30 pass. Hygiene Metric Before After anyio.sleep() calls in TestChildProcessCleanup 12 0 Coverage pragmas in TestChildProcessCleanup 5 0 filterwarnings("ignore::ResourceWarning") decorators 3 0 Diff — +180 −256 (net −76 lines) AI Disclaimer

Open Graph Description: Summary Closes #1775. Supersedes #2071. The three TestChildProcessCleanup tests in tests/client/test_stdio.py failed intermittently on Windows/macOS CI. The root cause was a timing-dependent test d...

X Description: Summary Closes #1775. Supersedes #2071. The three TestChildProcessCleanup tests in tests/client/test_stdio.py failed intermittently on Windows/macOS CI. The root cause was a timing-dependent test d...

Opengraph URL: https://github.com/modelcontextprotocol/python-sdk/pull/2265

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width