Fix infinite blocking in TrySendMultipartMessage at low bandwidth by VincentVbs · Pull Request #1157 · zeromq/netmq

VincentVbs · 2026-05-01T23:23:05Z

Apply timeout to all frames in TrySendMultipartMessage, not just the first one. Previously, only the first frame used TrySendFrame with timeout, while subsequent frames used blocking SendMoreFrame/SendFrame methods. This caused infinite hangs when network bandwidth was extremely low (< 10 kbit/s).

The fix ensures all frames respect the timeout parameter, preventing infinite blocking and allowing graceful failure when send operations cannot complete.

Added three unit tests to validate the fix:

TrySendMultipartMessage_AppliesTimeoutToAllFrames
TrySendMultipartMessage_FailsGracefullyOnMiddleFrameTimeout
TrySendMultipartMessage_FailsGracefullyOnLastFrameTimeout

Fixes #1127

Apply timeout to all frames in TrySendMultipartMessage, not just the first one. Previously, only the first frame used TrySendFrame with timeout, while subsequent frames used blocking SendMoreFrame/SendFrame methods. This caused infinite hangs when network bandwidth was extremely low (< 10 kbit/s). The fix ensures all frames respect the timeout parameter, preventing infinite blocking and allowing graceful failure when send operations cannot complete. Added three unit tests to validate the fix: - TrySendMultipartMessage_AppliesTimeoutToAllFrames - TrySendMultipartMessage_FailsGracefullyOnMiddleFrameTimeout - TrySendMultipartMessage_FailsGracefullyOnLastFrameTimeout Fixes zeromq#1127

drewnoakes

Thanks, this looks great too.

drewnoakes · 2026-05-02T13:28:28Z

-
-            return true;
+            // Send the last frame with timeout (no 'more' flag)
+            return TrySendFrame(socket, timeout, message.Last.Buffer, message.Last.MessageSize, false);


Suggested change

return TrySendFrame(socket, timeout, message.Last.Buffer, message.Last.MessageSize, false);

return TrySendFrame(socket, timeout, message.Last.Buffer, message.Last.MessageSize, more: false);

Copilot

Pull request overview

This PR updates TrySendMultipartMessage so timeout-aware sends are used for every frame of a NetMQMessage, addressing the reported hang when multipart sends encounter extreme backpressure. It also adds focused unit tests around timeout handling for successful sends and failures on later frames.

Changes:

Reworked TrySendMultipartMessage(TimeSpan, NetMQMessage) to call TrySendFrame for every frame instead of mixing one timed send with later blocking sends.
Removed the old special-case path that timed only the first frame and blocked on the rest.
Added three tests covering timeout propagation and failure behavior on middle/last frames.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`src/NetMQ/OutgoingSocketExtensions.cs`	Changes multipart send logic so all `NetMQMessage` frames use the timeout-aware send path.
`src/NetMQ.Tests/OutgoingSocketExtensionsTests.cs`	Adds unit tests intended to validate timeout handling across all frames and timeout failures on later frames.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+                bool sentSuccessfully = TrySendFrame(socket, timeout, message[i].Buffer, message[i].MessageSize, true);
                if (!sentSuccessfully)
                    return false;
            }

-            for (int i = 1; i < message.FrameCount - 1; i++)
-            {
-                socket.SendMoreFrame(message[i].Buffer, message[i].MessageSize);
-            }
-
-            socket.SendFrame(message.Last.Buffer, message.Last.MessageSize);
-
-            return true;
+            // Send the last frame with timeout (no 'more' flag)
+            return TrySendFrame(socket, timeout, message.Last.Buffer, message.Last.MessageSize, false);


+                // CRITICAL: ALL frames must respect the timeout parameter
+                Assert.Equal(timeout, actualTimeout);


+                // Verify timeout is applied to all frames
+                Assert.Equal(timeout, actualTimeout);


+
+            var socket = new MockOutgoingSocket((ref Msg msg, TimeSpan actualTimeout, bool more) =>
+            {
+                Assert.Equal(timeout, actualTimeout);


drewnoakes · 2026-05-02T13:31:00Z

Looks like there's a test failure. I like where this is going.

The active test run was aborted. Reason: Test host process crashed : Unhandled exception.
System.Net.Sockets.SocketException (22): Invalid argument
   at System.Net.Sockets.Socket.UpdateStatusAfterSocketErrorAndThrowException(SocketError error, Boolean disconnectOnFailure, String callerName)
   at System.Net.Sockets.Socket.UpdateStatusAfterSocketOptionErrorAndThrowException(SocketError error, String callerName)
   at System.Net.Sockets.Socket.SetSocketOption(SocketOptionLevel optionLevel, SocketOptionName optionName, Int32 optionValue, Boolean silent)
   at NetMQ.Core.Transports.Tcp.TcpConnector.OutCompleted(SocketError socketError, Int32 bytesTransferred) in /_/src/NetMQ/Core/Transports/Tcp/TcpConnector.cs:line 240
   at NetMQ.Core.Utils.Proactor.Loop() in /_/src/NetMQ/Core/Utils/Proactor.cs:line 130
   at System.Threading.Thread.StartCallback()

drewnoakes · 2026-06-22T22:23:39Z

@VincentVbs are you interested in continuing with this PR?

VincentVbs · 2026-06-23T12:57:44Z

Thanks, @drewnoakes, for following up with me and for giving me the opportunity to contribute to this project.
Unfortunately, even though I spent extra time on the PR, I’m not able to do more today.
I did not anticipate running the unit tests on my machine, and the fix I initially proposed, which seemed trivial, turned out to be unfounded after reviewing the failing unit test.
In the end, I wasn’t able to reproduce the bug, but I have the feeling that the issue might no longer exist in the latest sources.
It would be useful to check exactly which version the user experienced the infinite loop with.
I did some tests using a Debian virtual machine and a tool to slow down communication, but I admit I wasn’t systematic enough and ended up getting exhausted.
I’m also not sure whether using a VM is the right approach to recommend.
Sorry for the noise this may have caused in the project.
Maybe I should have asked for guidance before working on the code.
How should I come back to zmq for a future contribution?
Thank you for taking the time to read this.

drewnoakes · 2026-06-25T04:26:39Z

No problem at all @VincentVbs, it's great that you added to the project. Even incomplete PRs can help move things along.

Maybe I should have asked for guidance before working on the code.

Some projects ask contributors to do that, but generally I think the main reason to do that is to protect your own time against working on something that the project won't accept. Sometimes it's hard to know whether it's acceptable before the investigation is finished though. Don't worry about it.

How should I come back to zmq for a future contribution?

Just work on anything that seems interesting to you. You can discuss beforehand if you like. We're happy to have any contribution here on NetMQ. Other projects in the zeromq org may have different requirements.

drewnoakes requested a review from Copilot May 2, 2026 13:25

Copilot started reviewing on behalf of drewnoakes May 2, 2026 13:26

View session

drewnoakes approved these changes May 2, 2026

View reviewed changes

Copilot AI reviewed May 2, 2026

View reviewed changes

Navigation Menu

Repository navigation

Fix infinite blocking in TrySendMultipartMessage at low bandwidth - #1157

Fix infinite blocking in TrySendMultipartMessage at low bandwidth#1157
VincentVbs wants to merge 1 commit into
zeromq:masterfrom
VincentVbs:fix-1127-TrySendMultipartMessage-With-Low-Network-Speeds

VincentVbs commented May 1, 2026

Uh oh!

drewnoakes left a comment

Uh oh!

Uh oh!

drewnoakes May 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

drewnoakes commented May 2, 2026

Uh oh!

drewnoakes commented Jun 22, 2026

Uh oh!

VincentVbs commented Jun 23, 2026

Uh oh!

drewnoakes commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	return TrySendFrame(socket, timeout, message.Last.Buffer, message.Last.MessageSize, false);
	return TrySendFrame(socket, timeout, message.Last.Buffer, message.Last.MessageSize, more: false);

		// CRITICAL: ALL frames must respect the timeout parameter
		Assert.Equal(timeout, actualTimeout);

		// Verify timeout is applied to all frames
		Assert.Equal(timeout, actualTimeout);

Conversation

VincentVbs commented May 1, 2026

Uh oh!

drewnoakes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

drewnoakes May 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

drewnoakes commented May 2, 2026

Uh oh!

drewnoakes commented Jun 22, 2026

Uh oh!

VincentVbs commented Jun 23, 2026

Uh oh!

drewnoakes commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants