Skip to content

Commit 359de6b

Browse files
Copilotdrewnoakes
andcommitted
Fix infinite loop in RequestSocket SendMultipartMessage
Update Mailbox.TryRecv to only catch SocketException instead of all exceptions. This allows ObjectDisposedException to propagate and break out of the TrySend loop when a socket is disposed during a send operation. Add test to validate that using a disposed socket doesn't cause an infinite loop. Co-authored-by: drewnoakes <350947+drewnoakes@users.noreply.github.com>
1 parent fa99177 commit 359de6b

File tree

2 files changed

+59
-2
lines changed

2 files changed

+59
-2
lines changed

src/NetMQ.Tests/ReqRepTests.cs

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
using NetMQ.Sockets;
1+
using System;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
using NetMQ.Sockets;
25
using Xunit;
36

47
namespace NetMQ.Tests
@@ -282,5 +285,59 @@ public void SendMultipartMessageSucceeds()
282285
Assert.Equal(new[] { "Hello", "Back" }, req.ReceiveMultipartStrings());
283286
}
284287
}
288+
289+
[Fact]
290+
public async Task DisposedSocketDoesNotCauseInfiniteLoop()
291+
{
292+
// This test validates that when a socket is disposed while a send operation is in progress,
293+
// it doesn't cause an infinite loop in TrySend. The fix ensures that Mailbox.TryRecv only
294+
// catches SocketException and not ObjectDisposedException, allowing the exception to propagate
295+
// and break out of the sending loop.
296+
297+
var req = new RequestSocket();
298+
req.Options.Linger = TimeSpan.Zero;
299+
300+
// Bind to an endpoint that has no peer - this will cause sends to block
301+
req.Connect("tcp://localhost:15555");
302+
303+
// Dispose the socket to trigger ObjectDisposedException
304+
req.Dispose();
305+
306+
// Create a task that will attempt to send on the disposed socket
307+
// This should complete quickly by throwing an exception rather than hanging
308+
var sendTask = Task.Run(() =>
309+
{
310+
try
311+
{
312+
var msg = new NetMQMessage();
313+
msg.Append("test");
314+
req.SendMultipartMessage(msg);
315+
}
316+
catch (ObjectDisposedException)
317+
{
318+
// Expected exception - this is good, it means we broke out of the loop
319+
return true;
320+
}
321+
catch (Exception)
322+
{
323+
// Any other exception is also fine - the key is that we don't hang
324+
return true;
325+
}
326+
return false;
327+
});
328+
329+
// If the fix is working, this should complete quickly (within 5 seconds)
330+
// If there's an infinite loop, it will timeout
331+
var timeoutTask = Task.Delay(TimeSpan.FromSeconds(5));
332+
var completedTask = await Task.WhenAny(sendTask, timeoutTask);
333+
334+
Assert.True(completedTask == sendTask, "Send operation should complete quickly after disposal, not hang in infinite loop");
335+
336+
if (completedTask == sendTask)
337+
{
338+
var result = await sendTask;
339+
Assert.True(result, "Send operation should have thrown an exception");
340+
}
341+
}
285342
}
286343
}

src/NetMQ/Core/Mailbox.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ public bool TryRecv(int timeout, out Command command)
213213
m_active = false;
214214
m_signaler.Recv();
215215
}
216-
catch
216+
catch (SocketException)
217217
{
218218
m_active = true;
219219
command = default(Command);

0 commit comments

Comments
 (0)