communication: Separate _reset_socket() function#792
Conversation
WalkthroughAdds a private _reset_socket() helper in SocketInterface to close the ZeroMQ socket and terminate its context, resetting internal state. Updates shutdown(wait=True) to call _reset_socket() after spawner shutdown and return the prior result in that path. Removes an unintended return from _reset_socket(). del unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant SocketInterface
participant Spawner
participant ZMQContext as ZMQ Context
participant ZMQSocket as ZMQ Socket
Client->>SocketInterface: shutdown(wait=True)
SocketInterface->>Spawner: shutdown(wait=True)
Spawner-->>SocketInterface: result
alt post-shutdown cleanup
SocketInterface->>ZMQSocket: close()
SocketInterface->>ZMQContext: term()
note right of SocketInterface: _process/_socket/_context = None
SocketInterface-->>Client: result
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #792 +/- ##
=======================================
Coverage 97.75% 97.75%
=======================================
Files 33 33
Lines 1471 1473 +2
=======================================
+ Hits 1438 1440 +2
Misses 33 33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
executorlib/standalone/interactive/communication.py (2)
136-139: Harden _reset_socket(): unregister from poller and avoid close/term hangs.Unregister the socket from the poller and set LINGER=0 before closing to prevent potential blocking; reinit or clear the poller to drop stale registrations.
Apply:
def _reset_socket(self): """ - Reset the socket and context of the SocketInterface instance. + Reset the socket and context of the SocketInterface instance. """ - if self._socket is not None: - self._socket.close() - if self._context is not None: - self._context.term() - self._process = None - self._socket = None - self._context = None + if self._socket is not None: + # Ensure poller no longer references the socket + try: + self._poller.unregister(self._socket) + except KeyError: + pass + # Avoid blocking on close + try: + self._socket.close(linger=0) + finally: + self._socket = None + if self._context is not None: + self._context.term() + self._context = None + # Drop/refresh poller to clear any stale registrations + self._poller = zmq.Poller() + self._process = NoneOptional: consider renaming to _close_socket() to better reflect that this is a teardown, not a reinitializing “reset.”
133-134: executorlib/standalone/interactive/communication.py: make shutdown() return type explicit
Annotateshutdown()with-> Optional[Any]and document its return value so callers know it may return the client’s shutdown result (orNone):- def shutdown(self, wait: bool = True): + def shutdown(self, wait: bool = True) -> Optional[Any]: """ Shutdown the SocketInterface and the connected client process. Args: wait (bool): Whether to wait for the client process to finish before returning. Default is True. + + Returns: + Optional[Any]: Result returned by the client during shutdown, or None if no result was received. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
executorlib/standalone/interactive/communication.py(1 hunks)
1 participant
Summary by CodeRabbit
Bug Fixes
Refactor