Skip to content

Fix Windows graceful process shutdown with CTRL_C_EVENT #1039

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

Closed

Conversation

felixweinberger
Copy link
Contributor

@felixweinberger felixweinberger commented Jun 26, 2025

Summary

Problem

On Windows, the cleanup procedure after yield in the lifespan context manager was not executed because process.terminate() forcefully killed processes without allowing cleanup code to run. This prevented proper resource cleanup and graceful shutdown of MCP servers on Windows.

Solution

Modified the Windows process termination logic to:

  1. First attempt graceful shutdown using signal.CTRL_C_EVENT (Windows-specific signal)
  2. Wait up to 2 seconds for the process to exit gracefully
  3. Fall back to terminate() if graceful shutdown fails
  4. Finally use kill() as a last resort

The fix is applied to both:

  • FallbackProcess.__aexit__: Handles process cleanup when exiting context manager
  • terminate_windows_process(): Utility function for terminating Windows processes

Technical Details

  • Uses os.kill(pid, signal.CTRL_C_EVENT) to send interrupt signal on Windows
  • Uses getattr(signal, "CTRL_C_EVENT") to avoid type errors on non-Windows platforms
  • Maintains backward compatibility with existing behavior
  • Includes proper exception handling for ProcessLookupError and OSError

Test Plan

  • All existing stdio tests pass
  • Code passes ruff formatting and linting
  • Code passes pyright type checking
  • Manual testing confirms cleanup code executes properly
  • Windows-specific testing to verify CTRL_C_EVENT behavior
  • Verify no regression in process termination reliability

Related Issues

@felixweinberger felixweinberger marked this pull request as draft June 26, 2025 12:35
@felixweinberger felixweinberger marked this pull request as draft June 26, 2025 12:35
Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixweinberger is there a way to add a test for this?

@felixweinberger
Copy link
Contributor Author

felixweinberger commented Jun 26, 2025

@felixweinberger is there a way to add a test for this?

Yes accidentally pushed as non-draft and planning to test on a Windows VM - working on this atm.

@felixweinberger felixweinberger force-pushed the fweinberger/fix-windows-graceful-shutdown branch 5 times, most recently from 30344e3 to 5e07c8e Compare June 26, 2025 14:17
Add comprehensive test suite for Windows-specific FallbackProcess to verify
that CTRL_C_EVENT signal properly triggers cleanup code in lifespan context
managers. These tests will fail until issue #1027 is fixed.

Includes detailed documentation explaining why the metaprogramming approach
is necessary for testing OS-level signal handling between processes.

Tests include:
- Graceful shutdown with CTRL_C_EVENT signal
- Timeout fallback to terminate() when signal is ignored
- CTRL_C_EVENT availability verification
- Async stdio stream functionality

Uses @pytest.mark.skipif pattern consistent with codebase conventions.

Github-Issue:#1027
Implement proper graceful shutdown for Windows subprocesses by sending
CTRL_C_EVENT signal before termination. This allows cleanup code in
lifespan context managers to execute properly.

The fix addresses issue #1027 where cleanup code after yield statements
was not being executed on Windows due to forceful process termination.

Changes:
- Send CTRL_C_EVENT signal for graceful shutdown on Windows
- Wait up to 2 seconds for process to exit gracefully
- Fall back to terminate() if graceful shutdown fails
- Add terminate_windows_process() helper function

Github-Issue:#1027
Reported-by:Felix Weinberger
@felixweinberger felixweinberger force-pushed the fweinberger/fix-windows-graceful-shutdown branch from 5e07c8e to 7b5ddf4 Compare June 26, 2025 14:48
@felixweinberger felixweinberger deleted the fweinberger/fix-windows-graceful-shutdown branch June 26, 2025 19:13
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.

The cleanup procedure after "yield" in lifespan is unreachable on Windows
2 participants