Skip to content

Problem: multipart send drops unsent frames - #689

Merged
gummif merged 2 commits into
zeromq:masterfrom
fhgffy:fix/659-preserve-unsent-parts
Oct 5, 2026
Merged

gummif merged 2 commits into
zeromq:masterfrom
fhgffy:fix/659-preserve-unsent-parts

Conversation

@fhgffy

@fhgffy fhgffy commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #659.

Problem

multipart_t::send() removes a frame before attempting to send it. If the first send returns EAGAIN, the multipart loses that frame and a retry sends an incomplete message. A thrown zmq::error_t also destroys the removed frame.

Solution

Send the front frame in place and remove it only after the send succeeds. The failed frame and remaining unsent parts stay available. There is no copying, catch-all handler, or API change.

The follow-up adds coverage for empty multipart messages and caller-supplied SNDMORE through both flag overloads, and formats the existing static_assert block that stopped CI. The production send loop remains unchanged from the initial PR.

I evaluated delegating to send_multipart, but did not keep that change: with libzmq 4.3.5, closing an inproc PULL peer during a large multipart PUSH send caused the helper's assert(it == begin(msgs)) to abort. In this environment, the current loop instead returned false. The sockets were each used only by their owning thread; no send result or errno was injected. This is an observed disconnect case, not a claim about every libzmq version or transport. After partial progress, the original full message is not automatically reconstructible; retaining the unsent suffix does not promise a transparent full-message retry.

Validation

Freshly tested on Linux x86_64, GCC 14.2, libzmq 4.3.5 (DRAFTS enabled, CURVE disabled), and Catch2 3.5.3:

  • The three original regressions fail with the unmodified upstream production header 7f0530688804c2b5b6b0d985773405593fd25ca8 and pass with both the current PR header and the follow-up: EAGAIN/repeated retry, high-water-mark drain/retry, and EFSM recovery
  • Related multipart/send_multipart/recv_multipart tests: 174 assertions in 9 cases pass in C++14 and C++20
  • The two added boundary cases pass on upstream, the current PR, and the follow-up: 19 assertions; these add coverage without changing the behavior
  • A real C++11 standalone harness covers both flag overloads, nonblocking/timed EAGAIN, repeated attempts, one/three frames, an empty first frame, exact contents/order, HWM recovery, EFSM recovery, SNDMORE masking, and empty success: 202 assertions pass; 100 consecutive runs pass
  • The same harness passes an explicitly selected legacy-addon branch and the partial-C++11 template branch. These are GCC branch-coverage checks, not execution on old MSVC
  • AddressSanitizer and UndefinedBehaviorSanitizer pass the C++11 harness and related multipart suite with both the header and libzmq instrumented; detect_leaks=0 is required because the default LeakSanitizer run fails in this container
  • Full tracked .cpp/.hpp formatting passes clang-format 20.1.8; git diff --check passes. CI's exact Ubuntu clang-format 20.1.2 build was not available locally

Full-suite limitations were compared with the same final test source on all three production headers. In C++14, current PR and follow-up have 159/179 passing tests, versus upstream 156/179. In C++20, current PR and follow-up have 160/182, versus upstream 157/182. The only removed failures are the three regressions. The same remaining 20/22 tests abort at libzmq's getifaddrs call with EPERM. No full-suite green result, remote CI success, old-MSVC validation, other operating-system coverage, or leak-check success is claimed.

Solution: remove each part only after a successful send and test retry behavior after EAGAIN, backpressure and EFSM.

Fixes zeromq#659.
@fhgffy
fhgffy marked this pull request as ready for review October 2, 2026 17:06
@gummif

gummif commented Oct 5, 2026

Copy link
Copy Markdown
Member

Note: There is also zmq::send_multipart free function which is superior. That could probably replace the implementation of multipart_t::send, the just clear at the end.

Solution: format the existing static_assert and cover empty sends and caller SNDMORE through both flag overloads. Keep the corrected send loop after peer-close testing exposed an abort in direct helper delegation.
@fhgffy

fhgffy commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the suggestion. I tried it, but with libzmq 4.3.5 an inproc PULL peer disconnecting during a large multipart send triggered send_multipart's assertion. The current loop returned false in the same test, so I've kept it for now. I've also added empty-message/SNDMORE coverage and fixed the formatting check.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 37292403362

Warning

No base build found for commit 7f05306 on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 86.116%

Details

  • Patch coverage: 3 of 3 lines across 1 file are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 1102
Covered Lines: 949
Line Coverage: 86.12%
Coverage Strength: 27.46 hits per line

💛 - Coveralls

@gummif
gummif merged commit c5b2f52 into zeromq:master Oct 5, 2026
11 checks passed
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.

Dropped first frame on failed send

3 participants