-
Notifications
You must be signed in to change notification settings - Fork 3
Add signal handling for graceful Docker shutdown #2
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
Changes from all commits
38ff613
1833123
96c24e9
e208015
26b2712
1c93bc0
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,9 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import argparse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import asyncio | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import functools | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import signal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from meshcore_proxy.proxy import EventLogLevel, MeshCoreProxy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -104,6 +106,48 @@ def parse_args() -> argparse.Namespace: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return parser.parse_args() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def run_with_shutdown(proxy: MeshCoreProxy) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Run the proxy with proper signal handling for graceful shutdown.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loop = asyncio.get_running_loop() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shutdown_event = asyncio.Event() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def signal_handler(sig): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Handle shutdown signals.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signame = signal.Signals(sig).name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logging.info(f"Received {signame}, shutting down gracefully...") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shutdown_event.set() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Register signal handlers for graceful shutdown | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for sig in (signal.SIGTERM, signal.SIGINT): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loop.add_signal_handler(sig, functools.partial(signal_handler, sig)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Create the proxy task | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| proxy_task = asyncio.create_task(proxy.run()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Wait for either the proxy to complete or a shutdown signal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shutdown_task = asyncio.create_task(shutdown_event.wait()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done, pending = await asyncio.wait( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [proxy_task, shutdown_task], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return_when=asyncio.FIRST_COMPLETED, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # If shutdown was signaled, cancel the proxy task | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if shutdown_task in done: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| proxy_task.cancel() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await proxy_task | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except asyncio.CancelledError: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Cancel any remaining tasks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for task in pending: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| task.cancel() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await task | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except asyncio.CancelledError: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+121
to
+148
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for sig in (signal.SIGTERM, signal.SIGINT): | |
| loop.add_signal_handler(sig, functools.partial(signal_handler, sig)) | |
| # Create the proxy task | |
| proxy_task = asyncio.create_task(proxy.run()) | |
| # Wait for either the proxy to complete or a shutdown signal | |
| shutdown_task = asyncio.create_task(shutdown_event.wait()) | |
| done, pending = await asyncio.wait( | |
| [proxy_task, shutdown_task], | |
| return_when=asyncio.FIRST_COMPLETED, | |
| ) | |
| # If shutdown was signaled, cancel the proxy task | |
| if shutdown_task in done: | |
| proxy_task.cancel() | |
| try: | |
| await proxy_task | |
| except asyncio.CancelledError: | |
| pass | |
| # Cancel any remaining tasks | |
| for task in pending: | |
| task.cancel() | |
| try: | |
| await task | |
| except asyncio.CancelledError: | |
| pass | |
| signals = (signal.SIGTERM, signal.SIGINT) | |
| for sig in signals: | |
| loop.add_signal_handler(sig, functools.partial(signal_handler, sig)) | |
| try: | |
| # Create the proxy task | |
| proxy_task = asyncio.create_task(proxy.run()) | |
| # Wait for either the proxy to complete or a shutdown signal | |
| shutdown_task = asyncio.create_task(shutdown_event.wait()) | |
| done, pending = await asyncio.wait( | |
| [proxy_task, shutdown_task], | |
| return_when=asyncio.FIRST_COMPLETED, | |
| ) | |
| # If shutdown was signaled, cancel the proxy task | |
| if shutdown_task in done: | |
| proxy_task.cancel() | |
| try: | |
| await proxy_task | |
| except asyncio.CancelledError: | |
| pass | |
| # Cancel any remaining tasks | |
| for task in pending: | |
| task.cancel() | |
| try: | |
| await task | |
| except asyncio.CancelledError: | |
| pass | |
| finally: | |
| # Ensure signal handlers are removed even if an error occurs | |
| for sig in signals: | |
| try: | |
| loop.remove_signal_handler(sig) | |
| except RuntimeError: | |
| # Event loop may be closing; ignore cleanup errors | |
| pass |
Copilot
AI
Jan 16, 2026
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.
The code at lines 142-148 that cancels pending tasks is unnecessary because when shutdown_task is done, the only pending task is shutdown_task itself (since proxy_task is already being awaited), and when proxy_task completes naturally, shutdown_task would be in pending and will never complete. This creates confusing logic. The pending task cleanup can be removed or simplified.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| """Tests for CLI signal handling.""" | ||
|
|
||
| import asyncio | ||
| import os | ||
| import signal | ||
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
|
|
||
| from meshcore_proxy.cli import run_with_shutdown | ||
| from meshcore_proxy.proxy import EventLogLevel, MeshCoreProxy | ||
|
|
||
| # Test timing constants | ||
| STARTUP_DELAY = 0.2 # Time to wait for proxy to start before sending signal | ||
| SHUTDOWN_TIMEOUT = 5 # Maximum time to wait for graceful shutdown | ||
|
|
||
|
|
||
| class MockRadio: | ||
| """Mock radio connection for testing.""" | ||
|
|
||
| def __init__(self): | ||
| self.is_connected = False | ||
| self.on_disconnect = None | ||
| self.on_receive = None | ||
|
|
||
| async def connect(self): | ||
| self.is_connected = True | ||
| return "mock-radio" | ||
|
|
||
| async def disconnect(self): | ||
| self.is_connected = False | ||
| if self.on_disconnect: | ||
| result = self.on_disconnect() | ||
| if asyncio.iscoroutine(result): | ||
| await result | ||
|
|
||
| async def send(self, data): | ||
| pass | ||
|
|
||
| def set_disconnect_handler(self, handler): | ||
| self.on_disconnect = handler | ||
|
|
||
| def set_reader(self, reader): | ||
| self.on_receive = reader.handle_rx | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| @patch("meshcore_proxy.proxy.SerialConnection") | ||
| async def test_sigterm_triggers_graceful_shutdown(mock_serial_connection): | ||
| """Test that SIGTERM signal triggers graceful shutdown.""" | ||
| mock_radio = MockRadio() | ||
| mock_serial_connection.return_value = mock_radio | ||
|
|
||
| proxy = MeshCoreProxy( | ||
| serial_port="/dev/ttyUSB0", | ||
| event_log_level=EventLogLevel.OFF, | ||
| tcp_port=5010, | ||
| ) | ||
|
|
||
| # Start the proxy with signal handling | ||
| async def run_and_signal(): | ||
| """Run proxy and send SIGTERM after a short delay.""" | ||
| # Give the proxy time to start | ||
| await asyncio.sleep(STARTUP_DELAY) | ||
| # Send SIGTERM to trigger shutdown | ||
| os.kill(os.getpid(), signal.SIGTERM) | ||
|
|
||
| # Run both tasks | ||
| signal_task = asyncio.create_task(run_and_signal()) | ||
| shutdown_task = asyncio.create_task(run_with_shutdown(proxy)) | ||
|
Comment on lines
+69
to
+70
|
||
|
|
||
| # Wait for shutdown with a timeout | ||
| try: | ||
| await asyncio.wait_for(shutdown_task, timeout=SHUTDOWN_TIMEOUT) | ||
| except asyncio.TimeoutError: | ||
| pytest.fail("Shutdown did not complete within timeout") | ||
|
|
||
| await signal_task | ||
|
|
||
| # Verify proxy stopped cleanly | ||
| assert not proxy._is_running | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| @patch("meshcore_proxy.proxy.SerialConnection") | ||
| async def test_sigint_triggers_graceful_shutdown(mock_serial_connection): | ||
| """Test that SIGINT signal (Ctrl+C) triggers graceful shutdown.""" | ||
| mock_radio = MockRadio() | ||
| mock_serial_connection.return_value = mock_radio | ||
|
|
||
| proxy = MeshCoreProxy( | ||
| serial_port="/dev/ttyUSB0", | ||
| event_log_level=EventLogLevel.OFF, | ||
| tcp_port=5011, | ||
| ) | ||
|
|
||
| # Start the proxy with signal handling | ||
| async def run_and_signal(): | ||
| """Run proxy and send SIGINT after a short delay.""" | ||
| await asyncio.sleep(STARTUP_DELAY) | ||
| os.kill(os.getpid(), signal.SIGINT) | ||
|
|
||
| # Run both tasks | ||
| signal_task = asyncio.create_task(run_and_signal()) | ||
| shutdown_task = asyncio.create_task(run_with_shutdown(proxy)) | ||
|
Comment on lines
+104
to
+105
|
||
|
|
||
| # Wait for shutdown with a timeout | ||
| try: | ||
| await asyncio.wait_for(shutdown_task, timeout=SHUTDOWN_TIMEOUT) | ||
| except asyncio.TimeoutError: | ||
| pytest.fail("Shutdown did not complete within timeout") | ||
|
|
||
| await signal_task | ||
|
|
||
| # Verify proxy stopped cleanly | ||
| assert not proxy._is_running | ||
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.
'except' clause does nothing but pass and there is no explanatory comment.