Skip to content

prevent redundant zeroMQ thread spawning by sharing a single context#473

Merged
pradeeban merged 5 commits intoControlCore-Project:devfrom
GREENRAT-K405:fix/redundant-context-instances
Mar 1, 2026
Merged

prevent redundant zeroMQ thread spawning by sharing a single context#473
pradeeban merged 5 commits intoControlCore-Project:devfrom
GREENRAT-K405:fix/redundant-context-instances

Conversation

@GREENRAT-K405
Copy link

Previously, zeroMQPort instantiated a new zmq.Context() for every port, which unintentionally spawned unnecessary I/O threads and increased resource overhead.

  • created a single shared _zmq_context = zmq.Context() at the module level.
  • updated the ZeroMQPort constructor to accept the shared context via dependency injection.
  • changed terminate_zmq() to ensure all port sockets are safely closed before calling _zmq_context.term() exactly once.

What this does:
reduces memory footprint and eliminated redundant background thread creation.

This fixes #472 .

Copilot AI review requested due to automatic review settings March 1, 2026 17:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Python ZeroMQ integration to follow the “single context per process” ZeroMQ pattern, reducing redundant ZMQ I/O thread creation and per-port overhead (fixes #472).

Changes:

  • Introduces a module-level shared _zmq_context for all ZMQ ports.
  • Updates ZeroMQPort creation to use an injected shared context rather than creating a new context per port.
  • Updates terminate_zmq() to close sockets first and then terminate the shared context once.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +19 to +24
def _get_zmq_context():
"""Return the process-level shared ZMQ context, creating it on first call."""
global _zmq_context
if _zmq_context is None or _zmq_context.closed:
_zmq_context = zmq.Context()
return _zmq_context
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The new shared-context lifecycle (_get_zmq_context() lazy init + terminate_zmq() single term) isn’t covered by tests. Adding a unit test that asserts only one zmq.Context is created across multiple init_zmq_port calls, and that terminate_zmq resets/terminates the context (including the failure path where port creation raises and no port is registered) would help prevent regressions.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

Will write tests once this PR is discussed with mentors!

GREENRAT-K405 and others added 2 commits March 1, 2026 23:30
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@GREENRAT-K405
Copy link
Author

@pradeeban @Rahuljagwani please have a look.

@pradeeban pradeeban merged commit d302863 into ControlCore-Project:dev Mar 1, 2026
6 checks passed
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.

3 participants