Use raw zmq_proxy for ffi-rzmq heartbeat - #391
Merged
Merged
Conversation
- Replace ZMQ::Device with LibZMQ.zmq_proxy - Stop heartbeat with zmq_ctx_shutdown before closing sockets - Treat ETERM and EINTR from zmq_proxy as normal shutdown - Set zero linger on the ffi-rzmq heartbeat socket - Add a Session close test for the live heartbeat path This addresses SciRuby#357. The ffi-rzmq heartbeat still uses libzmq's proxy behavior, but avoids the ZMQ::Device wrapper and gives Session#close an explicit path to stop the heartbeat thread.
There was a problem hiding this comment.
Pull request overview
This PR updates IRuby’s session shutdown path to explicitly stop the ffi-rzmq heartbeat thread (addressing the Ruby 3.4 test-suite hang in #357) by avoiding the ZMQ::Device wrapper and adding an adapter hook to shut down the underlying libzmq proxy loop cleanly.
Changes:
- Replace
ZMQ::Devicewith a directLibZMQ.zmq_proxycall in theffi-rzmqheartbeat loop and treatETERM/EINTRas normal shutdown. - Introduce
SessionAdapter#shutdown_heartbeatand invoke it fromSession#closeprior to stopping the heartbeat thread. - Set
LINGER=0for theffi-rzmqheartbeat (:REP) socket and add a test ensuringSession#closeterminates the heartbeat thread.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
lib/iruby/session.rb |
Adjusts shutdown ordering to request heartbeat shutdown before stopping thread and closing sockets. |
lib/iruby/session_adapter.rb |
Adds shutdown_heartbeat hook to the adapter base API. |
lib/iruby/session_adapter/ffirzmq_adapter.rb |
Uses raw zmq_proxy, adds context shutdown logic, and sets zero linger for heartbeat socket. |
lib/iruby/session_adapter/test_adapter.rb |
Updates test adapter shutdown/close tracking to support the new heartbeat shutdown path. |
test/iruby/session_test.rb |
Adds coverage ensuring the live ffi-rzmq heartbeat thread is not left running after Session#close. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This addresses #357. The ffi-rzmq heartbeat still uses libzmq's proxy behavior, but avoids the ZMQ::Device wrapper and gives Session#close an explicit path to stop the heartbeat thread.