Replace ffi-rzmq heartbeat device with poll loop#390
Open
kojix2 wants to merge 1 commit into
Open
Conversation
- Replace ZMQ::Device with an explicit blocking poll/echo loop - Treat ETERM and ENOTSOCK as normal heartbeat shutdown conditions - Add a heartbeat shutdown hook so sessions can stop the loop before closing sockets - Use zmq_ctx_shutdown for ffi-rzmq heartbeat cleanup when available - Set zero linger on the ffi-rzmq heartbeat socket to avoid delaying shutdown This addresses SciRuby#357, where Ruby 3.4 test runs could finish successfully but hang at process exit when using the ffi-rzmq session adapter. The likely cause was the heartbeat thread staying blocked inside ZMQ::Device / zmq_proxy without an explicit cleanup path.
There was a problem hiding this comment.
Pull request overview
Replaces the ZMQ::Device-based heartbeat with an explicit poll/echo loop in the ffi-rzmq adapter and adds a dedicated shutdown_heartbeat hook so Session#close can unblock the heartbeat thread before tearing down sockets. This targets the Ruby 3.4 process-exit hang reported in #357 where zmq_proxy left the heartbeat thread blocked with no clean exit path.
Changes:
- Replace
ZMQ::Devicewith aZMQ::Poller-based echo loop inFfirzmqAdapter#heartbeat_loop, treatingETERM/ENOTSOCK/EINTR/EAGAINas expected conditions. - Add
shutdown_heartbeathook onBaseAdapter, ffi-rzmq (usingzmq_ctx_shutdownwhen available) and test adapter; restructureSession#closeto invoke it first and set@closed = truebefore stopping the heartbeat thread. - Set
ZMQ::LINGER=0on the REP heartbeat socket and dedupeclosed_socketsentries in the test adapter so the close-lifecycle test still asserts a single REP entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/iruby/session.rb | Reorders close to mark closed, shutdown heartbeat, stop thread, then close sockets and adapter. |
| lib/iruby/session_adapter.rb | Adds default shutdown_heartbeat that just closes the socket. |
| lib/iruby/session_adapter/ffirzmq_adapter.rb | New poll/echo heartbeat loop, zmq_ctx_shutdown based shutdown hook, LINGER=0 on REP, renames type to type_symbol to avoid shadowing. |
| lib/iruby/session_adapter/test_adapter.rb | Dedupes closed_sockets and implements shutdown_heartbeat that flips @closed and closes the socket. |
💡 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, where Ruby 3.4 test runs could finish successfully but hang at process exit when using the ffi-rzmq session adapter. The likely cause was the heartbeat thread staying blocked inside ZMQ::Device / zmq_proxy without an explicit cleanup path.
This pull request was prepared with assistance from Codex.