Fix stdio_client kill process after timeout #555
+7
−27
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Apr 21, 2025
This was referenced May 7, 2025
9 tasks
ihrpr
requested changes
May 28, 2025
There was a problem hiding this comment.
Thank you @cristipufu for working on this!
Do you have steps to reproduce? How this changes were tested?
Kludex
reviewed
May 28, 2025
| await process.wait() | ||
| except TimeoutError: | ||
| # Force kill if it doesn't terminate | ||
| process.kill() |
There was a problem hiding this comment.
I think the same TerminateProcess is called on Windows... Did this worked locally?
https://anyio.readthedocs.io/en/stable/api.html#anyio.abc.Process.kill
|
Slightly orthogonal but this does not handle child process spawns which can cause the parent to leave ghost processes which are being re-parented to the root process. Addressed it here: #850 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Be consistent across platforms (Windows, Linux, etc). Some MCP Servers don't react to SIGTERM, so
stdio_clientwill hang indefinitely when exiting the context manager.Reference: #372
How Has This Been Tested?
Yes
Breaking Changes
No
Types of changes
Checklist