-
Notifications
You must be signed in to change notification settings - Fork 2
(do not merge) fix: implement proper async signal handling for graceful server shutdown #73
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,8 +5,10 @@ | |||||
""" | ||||||
|
||||||
import asyncio | ||||||
import signal | ||||||
import sys | ||||||
|
||||||
import anyio | ||||||
from fastmcp import FastMCP | ||||||
|
||||||
from connector_builder_mcp._util import initialize_logging | ||||||
|
@@ -22,13 +24,35 @@ | |||||
def main() -> None: | ||||||
"""Main entry point for the Builder MCP server.""" | ||||||
print("Starting Builder MCP server.", file=sys.stderr) | ||||||
|
||||||
async def run_server(): | ||||||
"""Run the server with proper signal handling.""" | ||||||
shutdown_event = asyncio.Event() | ||||||
|
||||||
def signal_handler(): | ||||||
print("Builder MCP server interrupted by user.", file=sys.stderr) | ||||||
shutdown_event.set() | ||||||
|
||||||
if sys.platform != "win32": | ||||||
loop = asyncio.get_running_loop() | ||||||
for sig in (signal.SIGTERM, signal.SIGINT): | ||||||
loop.add_signal_handler(sig, signal_handler) | ||||||
|
||||||
try: | ||||||
async with anyio.create_task_group() as tg: | ||||||
tg.start_soon(app.run_stdio_async) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a potential race condition where
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
await shutdown_event.wait() | ||||||
tg.cancel_scope.cancel() | ||||||
except anyio.get_cancelled_exc_class(): | ||||||
pass | ||||||
except Exception as ex: | ||||||
print(f"Error running Builder MCP server: {ex}", file=sys.stderr) | ||||||
sys.exit(1) | ||||||
|
||||||
try: | ||||||
asyncio.run(app.run_stdio_async()) | ||||||
asyncio.run(run_server()) | ||||||
except KeyboardInterrupt: | ||||||
print("Builder MCP server interrupted by user.", file=sys.stderr) | ||||||
except Exception as ex: | ||||||
print(f"Error running Builder MCP server: {ex}", file=sys.stderr) | ||||||
sys.exit(1) | ||||||
|
||||||
print("Builder MCP server stopped.", file=sys.stderr) | ||||||
sys.exit(0) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signal handlers are registered but never cleaned up. If the function is called multiple times or the event loop is reused, this could lead to duplicate signal handlers. Consider adding signal handler cleanup in a finally block or using a context manager approach.
Copilot uses AI. Check for mistakes.