[RF] Reimplement RooFit::MultiProcess IPC without ZeroMQ - #23343
Merged
Conversation
The worker-side update wrote the received offsets into the vector with reserve() plus std::copy to begin(), which is undefined behavior whenever the vector is shorter than the message: the copy writes into raw reserved storage past end() and the vector size stays stale, so the received offsets are silently invisible afterwards. The mainstream fit flows are only safe because LikelihoodJob::update_state (which uses resize(), correctly) happens to run first and size the shared vector. Use resize() here too, like in LikelihoodJob. 🤖 Done with the help of AI
Replace the ZeroMQ-based interprocess communication in RooFit::MultiProcess with a direct implementation on top of plain socketpair() pipes, and remove the ZeroMQ and cppzmq dependencies together with the RooFitZMQ wrapper library. The new transport lives in three small classes in the multiprocess package: - Message: a byte buffer replacing zmq::message_t, including in the Job::receive_task_result_on_master() interface. - Channel: one end of an AF_UNIX socketpair with framed whole-message send/receive on top of the byte stream (8-byte header carrying the payload size and a "more" bit for multipart messages). Sends never block: bytes the kernel buffer does not accept are kept in a per-channel pending buffer that is flushed whenever any channel in the process waits for input, mimicking the previous unlimited high-water-mark ZeroMQ setup and making send-send deadlocks between processes impossible. Multipart messages are flushed on their final frame only, so a k-frame state update costs one send() system call per receiver instead of k. - Poller: an index-stable replacement for the ZeroMQPoller. The socketpairs are created in the ProcessManager before forking, so every process inherits its ends of the already-connected channels; the IPC socket files in /tmp and the PUB-SUB subscriber handshake are gone. All descriptors are opened close-on-exec so they cannot leak into programs executed by user code. One full-duplex channel per link replaces the previous socket pairs: master-queue, queue-worker, and master-worker, where the latter carries both the state updates previously published over PUB-SUB and the task results previously pushed to the master's PULL socket. The master receives results round-robin over the ready worker channels, like the fair queuing of a ZeroMQ PULL socket, and keeps multipart messages together. SIGTERM handling no longer needs the sigprocmask/ppoll dance (whose zmq_ppoll needed the ZeroMQ draft API, and which plain ppoll would not cover on macOS): the signal handler now writes to a self-pipe that every poll watches, which closes the same check-then-block race. Benign signal interruptions (profilers, SIGCHLD, debuggers) are retried inside the wait primitive instead of surfacing to the event loops, so they can no longer desynchronize multi-frame message sequences; only a SIGTERM leaves a blocking call exceptionally, which also made the old EINTR-retry ladders at the call sites collapse. During JobManager shutdown the ProcessManager now terminates the child processes before the Messenger closes the channels, so no process sees a closed connection during a normal shutdown. An unexpectedly closed connection fails fast instead of hanging in a poll that can never return, and exceptions on the forked child processes are caught in JobManager::activate() so they exit in an orderly way rather than unwinding into the master-side stack inherited through fork. Since the feature no longer needs external dependencies, it is now always built on non-Windows platforms. The roofit_multiprocess build option therefore has no effect anymore: it is deprecated with a warning and will be removed one release cycle later. 🤖 Done with the help of AI
guitargeek
requested review from
bellenot,
dpiparo and
hageboeck
as code owners
September 11, 2026 13:26
Test Results 23 files 23 suites 3d 19h 39m 50s ⏱️ Results for commit 02f098f. |
dpiparo
approved these changes
Sep 12, 2026
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.
Replace the ZeroMQ-based interprocess communication in RooFit::MultiProcess with a direct implementation on top of plain socketpair() pipes, and remove the ZeroMQ and cppzmq dependencies together with the RooFitZMQ wrapper library.
The new transport lives in three small classes in the multiprocess package:
The socketpairs are created in the ProcessManager before forking, so every process inherits its ends of the already-connected channels; the IPC socket files in /tmp and the PUB-SUB subscriber handshake are gone. All descriptors are opened close-on-exec so they cannot leak into programs executed by user code. One full-duplex channel per link replaces the previous socket pairs: master-queue, queue-worker, and master-worker, where the latter carries both the state updates previously published over PUB-SUB and the task results previously pushed to the master's PULL socket. The master receives results round-robin over the ready worker channels, like the fair queuing of a ZeroMQ PULL socket, and keeps multipart messages together.
SIGTERM handling no longer needs the sigprocmask/ppoll dance (whose zmq_ppoll needed the ZeroMQ draft API, and which plain ppoll would not cover on macOS): the signal handler now writes to a self-pipe that every poll watches, which closes the same check-then-block race. Benign signal interruptions (profilers, SIGCHLD, debuggers) are retried inside the wait primitive instead of surfacing to the event loops, so they can no longer desynchronize multi-frame message sequences; only a SIGTERM leaves a blocking call exceptionally, which also made the old EINTR-retry ladders at the call sites collapse.
During JobManager shutdown the ProcessManager now terminates the child processes before the Messenger closes the channels, so no process sees a closed connection during a normal shutdown. An unexpectedly closed connection fails fast instead of hanging in a poll that can never return, and exceptions on the forked child processes are caught in JobManager::activate() so they exit in an orderly way rather than unwinding into the master-side stack inherited through fork.
Since the feature no longer needs external dependencies, it is now always built on non-Windows platforms. The roofit_multiprocess build option therefore has no effect anymore: it is deprecated with a warning and will be removed one release cycle later.
🤖 Done with the help of AI