Skip to content

fweinberger/align shutdown with spec #1091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

felixweinberger
Copy link
Contributor

  • Fix stdio cleanup hanging on slow-terminating processes
  • Implement MCP spec-compliant stdio shutdown sequence
  • Restore anyio.open_process as primary Windows process creation method

Motivation and Context

How Has This Been Tested?

Breaking Changes

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

Additional context

felixweinberger and others added 3 commits July 4, 2025 17:44
The stdio cleanup was hanging indefinitely when processes ignored
termination signals or took too long to exit. This caused the MCP
client to freeze during shutdown, especially with servers that don't
handle SIGTERM properly.

The fix introduces a 2-second timeout for process termination. If a
process doesn't exit gracefully within this window, it's forcefully
killed. This ensures the client always completes cleanup in bounded
time while still giving well-behaved servers a chance to exit cleanly.

This resolves hanging issues reported when MCP servers ignore standard
termination signals or perform lengthy cleanup operations.

Also adds regression tests for #559.

resolves #555

Co-authored-by: Cristian Pufu <[email protected]>
The MCP specification recommends closing stdin first to allow servers
to exit gracefully before resorting to signals. This approach gives
well-behaved servers the opportunity to detect stdin closure and
perform clean shutdown without forceful termination.

The shutdown sequence now follows a graceful escalation path: first
closing stdin and waiting 2 seconds for voluntary exit, then sending
SIGTERM if needed, and finally using SIGKILL as a last resort. This
minimizes the risk of data loss or corruption while ensuring cleanup
always completes.

This unified approach works consistently across all platforms and
improves compatibility with MCP servers that monitor stdin for
lifecycle management.

resolves #765

Co-authored-by: davenpi <[email protected]>
This restores the pre-#596 behavior where anyio.open_process is tried
first, falling back to subprocess.Popen only when necessary (e.g., when
using SelectorEventLoop on Windows).

The approach:
1. Try anyio.open_process with CREATE_NO_WINDOW flag
2. If NotImplementedError, fall back to subprocess.Popen wrapper
3. If other exception, retry anyio.open_process without flags

Also refactored to extract the fallback logic into a separate
_create_windows_fallback_process function for better code organization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant