Skip to content

Fix buffered ConnectAsync completion race on Unix #133201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rzikm merged 3 commits into from
Sep 16, 2026
Merged

Fix buffered ConnectAsync completion race on Unix #133201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56e1bb6
Fix buffered ConnectAsync completion race on Unix (#133012)
steveisok Sep 3, 2026
9f5c4d0
Harden buffered ConnectAsync regression test
steveisok Sep 3, 2026
77e4515
Address buffered ConnectAsync review feedback
steveisok Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 44 additions & 26 deletions src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,22 +650,38 @@ public override void InvokeCallback(bool allowPooling)

private sealed class ConnectOperation : BufferMemorySendOperation
{
// Set once the underlying connect() has completed successfully and we have
// started sending any buffered data, potentially across partial sends.
private bool _connected;

public ConnectOperation(SocketAsyncContext context) : base(context) { }

protected override bool DoTryComplete(SocketAsyncContext context)
{
bool result = SocketPal.TryCompleteConnect(context._socket, out ErrorCode);
context._socket.RegisterConnectResult(ErrorCode);

if (result && ErrorCode == SocketError.Success && Buffer.Length > 0)
if (!_connected)
{
SocketError error = context.SendToAsync(Buffer, 0, Buffer.Length, SocketFlags.None, Memory<byte>.Empty, ref BytesTransferred, Callback!, default);
if (error != SocketError.Success && error != SocketError.IOPending)
bool result = SocketPal.TryCompleteConnect(context._socket, out ErrorCode);
context._socket.RegisterConnectResult(ErrorCode);

if (!result || ErrorCode != SocketError.Success || Buffer.Length == 0)
{
context._socket.RegisterConnectResult(ErrorCode);
return result;
}

// Connect completed successfully and there is buffered data to send. Continue
// sending it as part of this same operation (retrying as needed) so that this
// operation -- and therefore the overall ConnectAsync call -- isn't reported as
// complete until the buffered data has actually finished sending (or failed).
_connected = true;
Offset = 0;
Count = Buffer.Length;
}
return result;

// Note: a failure here is a failure of the follow-up send, not of connect() itself
// (which already succeeded above), so it must not be reported via RegisterConnectResult
// -- doing so would incorrectly mark the connect itself as failed (LastConnectFailed),
// which affects unrelated behavior such as multi-connect handle replacement.
return SocketPal.TryCompleteSendTo(context._socket, Buffer.Span, ref Offset, ref Count, SocketFlags.None, default, ref BytesTransferred, out ErrorCode);
}

public override void InvokeCallback(bool allowPooling)
Expand All @@ -674,17 +690,12 @@ public override void InvokeCallback(bool allowPooling)
int bt = BytesTransferred;
Memory<byte> sa = SocketAddress;
SocketError ec = ErrorCode;
Memory<byte> buffer = Buffer;

if (buffer.Length == 0 || ec != SocketError.Success)
{
AssociatedContext._socket.SetBlocking();
// DoTryComplete only reports this operation as complete once any buffered data has
// been fully sent (or has failed), so it is now safe to restore native blocking mode.
AssociatedContext._socket.SetBlocking();
Comment thread
rzikm marked this conversation as resolved.

// Invoke callback only when we are completely done.
// In case data were provided for Connect we may or may not send them all.
// If we did not we will need follow-up with Send operation
cb(bt, sa, SocketFlags.None, ec);
}
cb(bt, sa, SocketFlags.None, ec);
}
}

Expand Down Expand Up @@ -1582,7 +1593,17 @@ public SocketError ConnectAsync(Memory<byte> socketAddress, Action<int, Memory<b

if (errorCode == SocketError.Success && remains > 0)
{
errorCode = SendToAsync(buffer.Slice(sentBytes), 0, remains, SocketFlags.None, Memory<byte>.Empty, ref sentBytes, callback!, default);
// If the buffered send doesn't complete synchronously, its own completion
// (whenever that happens) is what determines when the whole connect+send
// operation is done, so blocking mode must only be restored at that point.
SafeSocketHandle socket = _socket;
Action<int, Memory<byte>, SocketFlags, SocketError> connectCallback = callback;
errorCode = SendToAsync(buffer.Slice(sentBytes), 0, remains, SocketFlags.None, Memory<byte>.Empty, ref sentBytes,
(bytesTransferred, sendSocketAddress, flags, sendErrorCode) =>
{
socket.SetBlocking();
connectCallback(bytesTransferred, sendSocketAddress, flags, sendErrorCode);
}, default);
Comment thread
rzikm marked this conversation as resolved.
}

if (remains == 0 || errorCode != SocketError.IOPending)
Expand All @@ -1602,15 +1623,12 @@ public SocketError ConnectAsync(Memory<byte> socketAddress, Action<int, Memory<b

if (!_sendQueue.StartAsyncOperation(this, operation, observedSequenceNumber, cancellationToken))
{
if (operation.ErrorCode == SocketError.Success)
{
sentBytes += operation.BytesTransferred;
}
sentBytes = operation.BytesTransferred;

if (buffer.Length == 0 || operation.ErrorCode != SocketError.Success)
{
_socket.SetBlocking();
}
// ConnectOperation.DoTryComplete only reports synchronous completion once the
// entire connect, including any buffered send, has finished, so it's safe to
// restore native blocking mode here unconditionally.
_socket.SetBlocking();

return operation.ErrorCode;
}
Expand Down
68 changes: 57 additions & 11 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Net.Sockets;
using System.Threading;
using System.Threading.Tasks;
using Xunit;

Expand Down Expand Up @@ -193,7 +194,6 @@ public async Task AcceptAsync_ConcurrentAccepts_DoNotCorruptListenerState()
Assert.False(IsSocketNonBlocking(accepted2));
}

[ActiveIssue("https://github.com/dotnet/runtime/issues/128141", TestPlatforms.Android)]
[Fact]
public async Task ConnectAsync_WithBuffer_Succeeds()
{
Expand All @@ -219,20 +219,66 @@ public async Task ConnectAsync_WithBuffer_Succeeds()
await tcs.Task;

Assert.Equal(SocketError.Success, saea.SocketError);
Assert.Equal(3, saea.BytesTransferred);
Assert.True(client.Blocking);

// On Apple and Android platforms, TFO (connectx/sendto) may complete the connect+send
// in a single syscall, so the socket can end up blocking even on the async path.
// On Linux, async connect always leaves the socket non-blocking when
// buffer > 0 because SendToAsync is pending.
if (!completedAsync || PlatformDetection.IsApplePlatform || PlatformDetection.IsAndroid)
{
Assert.False(IsSocketNonBlocking(client));
}
else
// Native blocking mode is only restored once the entire connect, including any
// buffered send, has fully completed -- regardless of platform, and regardless of
// whether that completion happened synchronously or asynchronously.
Assert.False(IsSocketNonBlocking(client));
}

[Fact]
public async Task ConnectAsync_WithLargeBuffer_PendingSendCompletesBeforeBlockingIsRestored()
{
using Socket listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
listener.ReceiveBufferSize = 8 * 1024;
listener.Bind(new IPEndPoint(IPAddress.Loopback, 0));
listener.Listen(1);

using Socket client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

// Force a small send buffer so a multi-megabyte payload can't fit in a single non-blocking
// send(), guaranteeing (rather than merely hoping) that the buffered send started as part
// of ConnectAsync goes through the asynchronous (IOPending) completion path.
client.SendBufferSize = 8 * 1024;

byte[] data = new byte[4 * 1024 * 1024];

using var saea = new SocketAsyncEventArgs();
saea.RemoteEndPoint = (IPEndPoint)listener.LocalEndPoint!;
saea.SetBuffer(data, 0, data.Length);

var tcs = new TaskCompletionSource();
saea.Completed += (_, _) => tcs.SetResult();

bool completedAsync = client.ConnectAsync(saea);
Assert.True(completedAsync);
Assert.False(tcs.Task.IsCompleted);
Assert.True(IsSocketNonBlocking(client));

using var cts = new CancellationTokenSource(TestSettings.PassingTestTimeout);
using Socket accepted = await listener.AcceptAsync(cts.Token);

byte[] readBuffer = new byte[8 * 1024];
int totalRead = 0;
while (totalRead < data.Length)
{
Assert.True(IsSocketNonBlocking(client));
int n = await accepted.ReceiveAsync(readBuffer, SocketFlags.None, cts.Token);
Assert.NotEqual(0, n);
totalRead += n;
}

await tcs.Task.WaitAsync(cts.Token);

Assert.Equal(SocketError.Success, saea.SocketError);
Assert.Equal(data.Length, saea.BytesTransferred);
Assert.True(client.Blocking);

// Native blocking mode must not be restored until the pending buffered send has actually
// completed -- restoring it prematurely (e.g. right after connect() succeeds, before the
// follow-up send finishes) would fail intermittently depending on scheduling.
Assert.False(IsSocketNonBlocking(client));
Comment thread
rzikm marked this conversation as resolved.
}
}
}
Loading