Title: [v1.x] Deflake the child process cleanup tests by maxisbey · Pull Request #2839 · modelcontextprotocol/python-sdk · GitHub
Open Graph Title: [v1.x] Deflake the child process cleanup tests by maxisbey · Pull Request #2839 · modelcontextprotocol/python-sdk
X Title: [v1.x] Deflake the child process cleanup tests by maxisbey · Pull Request #2839 · modelcontextprotocol/python-sdk
Description: Deflakes tests/client/test_stdio.py::TestChildProcessCleanup, the most frequent flake on v1.x CI (Windows legs). Tests-only; nothing under src/ is touched. Motivation and Context These three tests have two independent failure modes, both confirmed from CI failure logs and reproduced under load on 2-core Windows runners: Startup race (the dominant mode): the tests gave the spawned parent 0.5s to boot and the child a 0.3s window to demonstrate writing, using fixed anyio.sleep() calls. Two cold Python interpreter starts on a loaded 2-core runner routinely exceed that, and the test fails with assert 0 > 0 before the cleanup logic under test ever runs. Cross-test GC collateral: the tests kill the process tree but never reap or close the process object. The subprocess transports survive past the per-test event loop, become garbage, and get collected during whatever test runs next on that worker. On Windows the "unclosed transport" warning then dies inside the transport's __repr__ (ValueError: I/O operation on closed pipe), and filterwarnings = ["error"] fails that innocent test. This also explains failures previously observed in unrelated tests that happened to share a worker with these. Measured on 2-core Windows runners with three CPU-load processes (100 repetitions per arm per Python version, deliberately harsher than real CI): Arm 3.10 3.13 Failure modes stock tests 11/100 2/100 11 startup race, 2 GC collateral polling fix only 4/100 2/100 0 startup race, 6 GC collateral polling + disposal (this PR) 0/100 0/100 none What changed Polling instead of fixed sleeps (commit 1): startup and writing are verified by polling under anyio.fail_after(15); the post-kill "writing stopped" check polls until the file size is stable. Net faster than the old fixed sleeps on the green path. Stop detection hardening (commit 2): "stopped" requires three consecutive stable samples, so a starved-but-alive child cannot slip through a single quiet window; a tree_killed flag prevents the cleanup path from re-terminating an already-killed tree (the second Windows call would operate on a closed job handle). Reap and dispose (commit 3): after the tree is confirmed dead, the tests wait() the process and drain stdout to EOF before closing it, which is what allows asyncio to close the subprocess transport inside the test. No transports survive to be collected during a later test. The first two commits also reached the maxisbey/v1x-interaction-backport branch via #2837; the hunks are identical, so merging both into v1.x converges cleanly, with the third commit new here. How Has This Been Tested? Full v1.x gate (pytest + 100% line and branch coverage) green. Locally: 70+ repetitions of the class including runs under full-core CPU load, cross-test GC probes (-n 0 with a follow-on test file) clean, and zero warning output in any run. The load-rig confirmation run for the final arm completed: 0 failures in 100 repetitions on each of Python 3.10 and 3.13. Breaking Changes None. Tests-only. Types of changes Bug fix (non-breaking change which fixes an issue) New feature (non-breaking change which adds functionality) Breaking change (fix or feature that would cause existing functionality to change) Documentation update Checklist I have read the MCP Documentation My code follows the repository's style guidelines New and existing tests pass locally I have added appropriate error handling I have added or updated documentation as needed AI Disclaimer
Open Graph Description: Deflakes tests/client/test_stdio.py::TestChildProcessCleanup, the most frequent flake on v1.x CI (Windows legs). Tests-only; nothing under src/ is touched. Motivation and Context These three tests ...
X Description: Deflakes tests/client/test_stdio.py::TestChildProcessCleanup, the most frequent flake on v1.x CI (Windows legs). Tests-only; nothing under src/ is touched. Motivation and Context These three tests ...
Opengraph URL: https://github.com/modelcontextprotocol/python-sdk/pull/2839
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:855ec588-9ae2-54e5-2a5d-abf8da8037d2 |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | 82BE:214886:134094:1A41F3:6A5A5955 |
| html-safe-nonce | debca35faf4383f9da198354c0f7ca8b05d8b3f0366544978a07a2d977b773a7 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI4MkJFOjIxNDg4NjoxMzQwOTQ6MUE0MUYzOjZBNUE1OTU1IiwidmlzaXRvcl9pZCI6IjM0Njc1Nzg1MzIwNzgyMTY1IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | ccb2c49a5bfced5585f40ff3c0d7b5089a7dd5a5055c5f441e9ed440d2091773 |
| hovercard-subject-tag | pull_request:3848893953 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/modelcontextprotocol/python-sdk/pull/2839/files |
| twitter:image | https://avatars.githubusercontent.com/u/224885523?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/224885523?s=400&v=4 |
| og:image:alt | Deflakes tests/client/test_stdio.py::TestChildProcessCleanup, the most frequent flake on v1.x CI (Windows legs). Tests-only; nothing under src/ is touched. Motivation and Context These three tests ... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 05b9ddf6a47d2dbe13944873a99f5fb4b83ba4871f9cb8a8e256793a63ca9687 |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/modelcontextprotocol/python-sdk git https://github.com/modelcontextprotocol/python-sdk.git |
| octolytics-dimension-user_id | 182288589 |
| octolytics-dimension-user_login | modelcontextprotocol |
| octolytics-dimension-repository_id | 862584018 |
| octolytics-dimension-repository_nwo | modelcontextprotocol/python-sdk |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 862584018 |
| octolytics-dimension-repository_network_root_nwo | modelcontextprotocol/python-sdk |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | f8d29d1bd03dda2dd14b3f80b8bc27e1111f43bd |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width