diff --git a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs index 9a8f33299de93e..dbcc997e932e36 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs @@ -422,9 +422,13 @@ protected async Task RunClient_CloseAsync_DuringConcurrentReceiveAsync_ExpectedS await cws.CloseAsync(WebSocketCloseStatus.NormalClosure, "", CancellationToken.None); - // There is a race condition in the above. If the ReceiveAsync receives the sent close message from the server, - // then it will complete successfully and the socket will close successfully. If the CloseAsync receive the sent - // close message from the server, then the receive async will end up getting aborted along with the socket. + // The outcome depends on timing. Two outcomes are acceptable: + // 1. The pending ReceiveAsync picks up the server's close frame cleanly: it completes with a Close + // message and the socket transitions to Closed. + // 2. The close handshake triggers an internal Abort (e.g. WaitForServerToCloseConnectionAsync + // times out because the server doesn't close TCP within its 1s window): state becomes Aborted, + // the parked receive's stream read fails, and ReceiveAsyncPrivate translates the exception to + // OperationCanceledException because of the Aborted state. try { await t; diff --git a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs index a90ed4ccf38498..69ca835f149287 100644 --- a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs +++ b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs @@ -82,6 +82,8 @@ internal sealed partial class ManagedWebSocket : WebSocket private bool _sentCloseFrame; /// Whether we've ever received a close frame. private bool _receivedCloseFrame; + /// 0 if has not been invoked yet; 1 otherwise. + private int _waitedForServerClose; /// The reason for the close, as sent by the server, or null if not yet closed. private WebSocketCloseStatus? _closeStatus; /// A description of the close reason as sent by the server, or null if not yet closed. @@ -1123,9 +1125,18 @@ private async ValueTask HandleReceivedCloseAsync(MessageHeader header, Cancellat } } - /// Issues a read on the stream to wait for EOF. + /// Issues a read on the stream to wait for EOF. Idempotent across call sites. private async ValueTask WaitForServerToCloseConnectionAsync(CancellationToken cancellationToken) { + // Called from both the receive and the send path. In rare concurrent Close + Receive + // scenarios both paths can observe the close handshake as complete and would otherwise + // both issue a stream read (violating most Stream implementations' single-consumer invariant). + // The flag guarantees that only the first invocation performs the wait; the other is a no-op. + if (Interlocked.CompareExchange(ref _waitedForServerClose, 1, 0) != 0) + { + return; + } + if (NetEventSource.Log.IsEnabled()) NetEventSource.Trace(this); // Per RFC 6455 7.1.1, try to let the server close the connection. We give it up to a second.