Skip to content

Replace ffi-rzmq heartbeat device with poll loop#390

Open
kojix2 wants to merge 1 commit into
SciRuby:masterfrom
kojix2:fix-ffirzmq-heartbeat-exit-hang
Open

Replace ffi-rzmq heartbeat device with poll loop#390
kojix2 wants to merge 1 commit into
SciRuby:masterfrom
kojix2:fix-ffirzmq-heartbeat-exit-hang

Conversation

@kojix2
Copy link
Copy Markdown
Member

@kojix2 kojix2 commented May 31, 2026

  • 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 #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.

- 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.
Copy link
Copy Markdown

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

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::Device with a ZMQ::Poller-based echo loop in FfirzmqAdapter#heartbeat_loop, treating ETERM/ENOTSOCK/EINTR/EAGAIN as expected conditions.
  • Add shutdown_heartbeat hook on BaseAdapter, ffi-rzmq (using zmq_ctx_shutdown when available) and test adapter; restructure Session#close to invoke it first and set @closed = true before stopping the heartbeat thread.
  • Set ZMQ::LINGER=0 on the REP heartbeat socket and dedupe closed_sockets entries 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.

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.

2 participants