fix: add atexit and signal handlers for ZMQ cleanup#206
fix: add atexit and signal handlers for ZMQ cleanup#206Sahil-u07 wants to merge 2 commits intoControlCore-Project:devfrom
Conversation
Sahil-u07
commented
Feb 4, 2026
Sahil-u07
commented
- Register terminate_zmq() with atexit to ensure cleanup on normal exit
- Add signal handlers for SIGINT (Ctrl+C) and SIGTERM
- Improve terminate_zmq() with better logging and error handling
- Clear zmq_ports dict after cleanup to prevent double-cleanup
There was a problem hiding this comment.
Pull request overview
This PR enhances ZMQ resource cleanup by adding exit and signal handlers to ensure proper cleanup when the program terminates.
Changes:
- Added atexit handler to automatically clean up ZMQ resources on normal program exit
- Added signal handlers for SIGINT (Ctrl+C) and SIGTERM to enable graceful shutdown on signals
- Improved terminate_zmq() function with guard checks, better logging including port names, and clearing of zmq_ports dictionary after cleanup
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def signal_handler(sig, frame): | ||
| """Handle interrupt signals gracefully.""" | ||
| print(f"\nReceived signal {sig}, shutting down gracefully...") |
There was a problem hiding this comment.
The signal_handler calls sys.exit(0) which will trigger the atexit handler, resulting in terminate_zmq() being called twice. While the code guards against double cleanup with the 'if not zmq_ports' check, this creates unnecessary overhead and potential confusion. Consider either: (1) not calling sys.exit(0) in the signal handler and instead just returning, allowing the program to exit naturally and trigger atexit, or (2) unregistering the atexit handler before calling sys.exit in the signal handler using atexit.unregister(terminate_zmq).
| print(f"\nReceived signal {sig}, shutting down gracefully...") | |
| print(f"\nReceived signal {sig}, shutting down gracefully...") | |
| # Prevent terminate_zmq from being called twice: once here and once via atexit | |
| try: | |
| atexit.unregister(terminate_zmq) | |
| except Exception: | |
| # If unregister fails for any reason, proceed with explicit cleanup anyway | |
| pass |
| def terminate_zmq(): | ||
| for port in zmq_ports.values(): | ||
| """Clean up all ZMQ sockets and contexts before exit.""" | ||
| if not zmq_ports: | ||
| return # No ports to clean up | ||
|
|
||
| print("\nCleaning up ZMQ resources...") | ||
| for port_name, port in zmq_ports.items(): | ||
| try: | ||
| port.socket.close() | ||
| port.context.term() | ||
| print(f"Closed ZMQ port: {port_name}") | ||
| except Exception as e: | ||
| print(f"Error while terminating ZMQ port {port.address}: {e}") | ||
| print(f"Error while terminating ZMQ port {port_name} ({port.address}): {e}") | ||
| zmq_ports.clear() |
There was a problem hiding this comment.
If a signal arrives while terminate_zmq() is already executing (from normal program flow or atexit), the signal_handler will call terminate_zmq() again recursively. While the 'if not zmq_ports' guard at line 98 provides some protection, it won't help if the signal arrives after the check but during the iteration. Consider adding a boolean flag (e.g., '_cleanup_in_progress') to prevent reentrant calls to terminate_zmq().
concore.py
Outdated
| # Register cleanup handlers | ||
| atexit.register(terminate_zmq) | ||
| signal.signal(signal.SIGINT, signal_handler) # Handle Ctrl+C | ||
| signal.signal(signal.SIGTERM, signal_handler) # Handle termination |
There was a problem hiding this comment.
SIGTERM signal handling is not properly supported on Windows. While the signal constant exists, Windows doesn't support custom handlers for SIGTERM in the same way Unix systems do. This line should be conditionally registered only on non-Windows platforms to avoid potential issues. Consider wrapping this in a platform check similar to line 14's check for Windows.
| signal.signal(signal.SIGTERM, signal_handler) # Handle termination | |
| if not hasattr(sys, 'getwindowsversion'): | |
| signal.signal(signal.SIGTERM, signal_handler) # Handle termination |
- Register terminate_zmq() with atexit to ensure cleanup on normal exit - Add signal handlers for SIGINT (Ctrl+C) and SIGTERM - Improve terminate_zmq() with better logging and error handling - Clear zmq_ports dict after cleanup to prevent double-cleanup
Sahil-u07
force-pushed
the
fix/zmq-cleanup-on-exit
branch
from
c51c041 to
ff3206a
Compare
1 participant
