Fix buffered ConnectAsync completion race on Unix

[steveisok]

Summary

Fixes #133012 — the intermittent failure of System.Net.Sockets.Tests.SocketBlockingModeTransitionTests.ConnectAsync_WithBuffer_Succeeds on Linux across CoreCLR, Mono, and NativeAOT.

Root cause

In SocketAsyncContext.Unix.cs, ConnectOperation.DoTryComplete would complete the underlying connect() and, if it succeeded with buffered data still to send, kick off the buffered send via a separately-spawned operation — but it always reported the connect itself as "complete" regardless of whether that follow-up send was actually still pending (IOPending). This caused SocketAsyncContext.ConnectAsync to return SocketError.Success instead of IOPending, so SocketAsyncEventArgs.DoOperationConnectEx treated the whole connect+buffered-send as a synchronous completion, while a genuinely asynchronous completion of the same callback was still coming later (when the spawned send operation finished). This double-completion race violated the SocketAsyncEventArgs sync/async completion contract and is what the test observed intermittently (the socket ended up still non-blocking on the branch that assumed a synchronous completion always means blocking mode was restored).

There was also a related gap: when the initial connect() completed synchronously but the trailing buffered send needed to complete asynchronously, native blocking mode was never restored once that send eventually finished.

Fix

• ConnectOperation now tracks whether connect() has completed and, once it has, continues sending any buffered data as part of the same operation (reusing the inherited Offset/Count/BytesTransferred fields and calling SocketPal.TryCompleteSendTo directly, mirroring the pattern the base BufferMemorySendOperation.DoTryComplete already uses) instead of spawning a separate operation. DoTryComplete now only reports completion once the entire connect+send unit is actually done (or has failed).
• ConnectOperation.InvokeCallback now unconditionally restores native blocking mode before invoking the user callback, since completion is now guaranteed to mean "fully done."
• In ConnectAsync's synchronous-TryStartConnect branch, the callback passed to the follow-up buffered SendToAsync call is wrapped to restore blocking mode immediately before invoking the real callback if that send completes asynchronously.
• The ConnectOperation-enqueued branch of ConnectAsync now restores blocking mode unconditionally once StartAsyncOperation reports synchronous completion, since that's now guaranteed to mean the whole connect+send is finished.

Tests

• ConnectAsync_WithBuffer_Succeeds no longer special-cases Apple/Android for the blocking-mode assertion, since blocking restoration is now deterministic on all platforms/paths.
• Removed the [ActiveIssue(".../128141", TestPlatforms.Android)] skip (duplicate of this same bug) since the fix makes this test reliable there too.
• Added a new deterministic regression test, ConnectAsync_WithLargeBuffer_PendingSendCompletesBeforeBlockingIsRestored, which forces the buffered send into the genuinely-asynchronous completion path via small send/receive buffer sizes plus a multi-megabyte payload, and asserts blocking mode is only restored once that pending send has actually finished. Verified this test fails reliably (5/5 runs) without the fix and passes reliably (15-20/20 runs) with it.

Validation

• Baseline ./build.sh clr+libs -rc release succeeded before any edits.
• System.Net.Sockets builds with 0 warnings/errors.
• Full SocketBlockingModeTransitionTests suite: 11/11 pass.
• All Connect-related tests (511 total, 505 run / 6 pre-existing skips): 0 failures.
• Full System.Net.Sockets.Tests suite: only pre-existing, unrelated SendReceive*.TcpReceiveSendGetsCanceledByDispose/multicast-option flakiness remains, confirmed to reproduce identically on the unmodified baseline.

Note

This PR description and the associated code changes were produced with the assistance of GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @karelz, @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[Copilot]

Copilot review overview

🟡 Changes recommended

The new regression test can hang the test run due to blocking Accept()/Receive() usage without timeouts, which risks CI deadlocks on regressions or stressed environments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	src/​libraries/​System.Net.Sockets/​tests/​FunctionalTests/​Connect.Unix.cs — The new regression test uses a synchronous Accept() followed by a potentially long-running…

What changed in this PR

This PR updates the Unix ConnectAsync implementation in System.Net.Sockets to ensure that a ConnectAsync with a buffered payload is only reported as “complete” once the entire connect + buffered-send sequence has actually finished, and that native blocking mode is restored deterministically at that true completion point.

Changes:

• Refactors SocketAsyncContext.Unix.ConnectOperation to continue any buffered send within the same operation (instead of spawning a separate send operation), preventing sync/async double-completion races.
• Ensures native blocking mode restoration occurs at the correct point for both the “TryStartConnect sync” path and the queued ConnectOperation path.
• Updates/extends the Unix blocking-mode transition tests, including adding a large-buffer regression test.

File	Description
src/​libraries/​System.Net.Sockets/​src/​System/​Net/​Sockets/​SocketAsyncContext.Unix.cs	Makes connect completion atomic with any buffered send and restores native blocking mode at true completion.
src/​libraries/​System.Net.Sockets/​tests/​FunctionalTests/​Connect.Unix.cs	Updates the existing buffered-connect test expectation and adds a large-buffer regression scenario.

[Copilot]

Copilot review overview

🔵 Needs a closer look

It changes low-level async connect/send completion semantics and blocking-mode transitions on Unix, which has broad behavioral impact and warrants final human review.

Review tier: Lite
Findings: None

Issues resolved since last review (1)

Severity	Finding
	src/​libraries/​System.Net.Sockets/​tests/​FunctionalTests/​Connect.Unix.cs — The new regression test uses a synchronous Accept() followed by a potentially long-running… View resolved comment

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.Unix.cs:231

• The new test name suggests it validates the relative ordering of buffered-send completion vs. blocking-mode restoration, but the assertions only verify that the operation completed asynchronously and that the socket is in blocking mode at the end. Renaming the test to match what it actually asserts will make the intent clearer and reduce the risk of future misinterpretation.

[steveisok]

@dotnet/ncl can I get a review on this? Addresses one of the top known issues that continue to pop.

[rzikm]

@dotnet/ncl can I get a review on this? Addresses one of the top known issues that continue to pop.

I will take a look

[rzikm]

LGTM modulo existing copilot comment

[rzikm]

@steveisok gentle ping, are you going to continue with this PR or do you want us to take over?

[Copilot]

Copilot review overview

🟡 Changes recommended

A critical unresolved race can allow queued I/O to run before native blocking mode is safely restored.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 1

Open (1)

• Restore native mode before dispatching queued I/O · New

[steveisok]

@steveisok gentle ping, are you going to continue with this PR or do you want us to take over?

@rzikm feel free to push commits if this needs more.