From 0ff26034c0e3b0ddba72b157fd957b1cc3d90833 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Wed, 10 Aug 2022 23:22:41 +0200 Subject: [PATCH 01/10] No end stream on ws connect and flush every message --- .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 4 ++-- .../src/System/Net/WebSockets/ManagedWebSocket.cs | 10 +++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 25c27d7d0fdc52..f52200327e2b10 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1600,7 +1600,7 @@ private async ValueTask SendHeadersAsync(HttpRequestMessage request // Start the write. This serializes access to write to the connection, and ensures that HEADERS // and CONTINUATION frames stay together, as they must do. We use the lock as well to ensure new // streams are created and started in order. - await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null && !http2Stream.ConnectProtocolEstablished), mustFlush), static (s, writeBuffer) => + await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null && !request.IsWebSocketH2Request()), mustFlush ), static (s, writeBuffer) => { if (NetEventSource.Log.IsEnabled()) s.thisRef.Trace(s.http2Stream.StreamId, $"Started writing. Total header bytes={s.headerBytes.Length}"); @@ -1962,7 +1962,7 @@ public async Task SendAsync(HttpRequestMessage request, boo try { // Send request headers - bool shouldExpectContinue = request.Content != null && request.HasHeaders && request.Headers.ExpectContinue == true; + bool shouldExpectContinue = (request.Content != null && request.HasHeaders && request.Headers.ExpectContinue == true) || request.IsWebSocketH2Request(); Http2Stream http2Stream = await SendHeadersAsync(request, cancellationToken, mustFlush: shouldExpectContinue).ConfigureAwait(false); bool duplex = request.Content != null && request.Content.AllowDuplex; 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 4130cff1cc65df..290054038b58b4 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 @@ -411,17 +411,19 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, // If we get here, the cancellation token is not cancelable so we don't have to worry about it, // and we own the semaphore, so we don't need to asynchronously wait for it. ValueTask writeTask = default; + Task flushTask; bool releaseSendBufferAndSemaphore = true; try { // Write the payload synchronously to the buffer, then write that buffer out to the network. int sendBytes = WriteFrameToSendBuffer(opcode, endOfMessage, disableCompression, payloadBuffer.Span); writeTask = _stream.WriteAsync(new ReadOnlyMemory(_sendBuffer, 0, sendBytes)); + flushTask = _stream.FlushAsync(); // If the operation happens to complete synchronously (or, more specifically, by // the time we get from the previous line to here), release the semaphore, return // the task, and we're done. - if (writeTask.IsCompleted) + if (writeTask.IsCompleted && flushTask.IsCompleted) { return writeTask; } @@ -447,14 +449,15 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, } } - return WaitForWriteTaskAsync(writeTask); + return WaitForWriteTaskAsync(writeTask, flushTask); } - private async ValueTask WaitForWriteTaskAsync(ValueTask writeTask) + private async ValueTask WaitForWriteTaskAsync(ValueTask writeTask, Task flushTask) { try { await writeTask.ConfigureAwait(false); + await flushTask.ConfigureAwait(false); } catch (Exception exc) when (!(exc is OperationCanceledException)) { @@ -478,6 +481,7 @@ private async ValueTask SendFrameFallbackAsync(MessageOpcode opcode, bool endOfM using (cancellationToken.Register(static s => ((ManagedWebSocket)s!).Abort(), this)) { await _stream.WriteAsync(new ReadOnlyMemory(_sendBuffer, 0, sendBytes), cancellationToken).ConfigureAwait(false); + await _stream.FlushAsync(cancellationToken).ConfigureAwait(false); } } catch (Exception exc) when (!(exc is OperationCanceledException)) From fe2a960940ea794deb889ec9a2f88cf614adf11f Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 11 Aug 2022 13:54:55 +0200 Subject: [PATCH 02/10] Apply suggestions from code review Co-authored-by: Natalia Kondratyeva --- .../System/Net/Http/SocketsHttpHandler/Http2Connection.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index f52200327e2b10..57466c7b0d0e7c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1600,7 +1600,7 @@ private async ValueTask SendHeadersAsync(HttpRequestMessage request // Start the write. This serializes access to write to the connection, and ensures that HEADERS // and CONTINUATION frames stay together, as they must do. We use the lock as well to ensure new // streams are created and started in order. - await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null && !request.IsWebSocketH2Request()), mustFlush ), static (s, writeBuffer) => + await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null && !request.IsWebSocketH2Request()), mustFlush), static (s, writeBuffer) => { if (NetEventSource.Log.IsEnabled()) s.thisRef.Trace(s.http2Stream.StreamId, $"Started writing. Total header bytes={s.headerBytes.Length}"); @@ -1962,8 +1962,8 @@ public async Task SendAsync(HttpRequestMessage request, boo try { // Send request headers - bool shouldExpectContinue = (request.Content != null && request.HasHeaders && request.Headers.ExpectContinue == true) || request.IsWebSocketH2Request(); - Http2Stream http2Stream = await SendHeadersAsync(request, cancellationToken, mustFlush: shouldExpectContinue).ConfigureAwait(false); + bool shouldExpectContinue = (request.Content != null && request.HasHeaders && request.Headers.ExpectContinue == true); + Http2Stream http2Stream = await SendHeadersAsync(request, cancellationToken, mustFlush: shouldExpectContinue || request.IsWebSocketH2Request()).ConfigureAwait(false); bool duplex = request.Content != null && request.Content.AllowDuplex; From f7551b0b76b30c9215fe17dabda3e83b0f4594c4 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 11 Aug 2022 13:57:19 +0200 Subject: [PATCH 03/10] Await flush in web socket after write --- .../src/System/Net/WebSockets/ManagedWebSocket.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) 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 290054038b58b4..fa1491da9eb7ad 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 @@ -418,14 +418,17 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, // Write the payload synchronously to the buffer, then write that buffer out to the network. int sendBytes = WriteFrameToSendBuffer(opcode, endOfMessage, disableCompression, payloadBuffer.Span); writeTask = _stream.WriteAsync(new ReadOnlyMemory(_sendBuffer, 0, sendBytes)); - flushTask = _stream.FlushAsync(); // If the operation happens to complete synchronously (or, more specifically, by // the time we get from the previous line to here), release the semaphore, return // the task, and we're done. - if (writeTask.IsCompleted && flushTask.IsCompleted) + if (writeTask.IsCompleted) { - return writeTask; + flushTask = _stream.FlushAsync(); + if (flushTask.IsCompleted) + { + return writeTask; + } } // Up until this point, if an exception occurred (such as when accessing _stream or when @@ -449,15 +452,15 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, } } - return WaitForWriteTaskAsync(writeTask, flushTask); + return WaitForWriteTaskAsync(writeTask); } - private async ValueTask WaitForWriteTaskAsync(ValueTask writeTask, Task flushTask) + private async ValueTask WaitForWriteTaskAsync(ValueTask writeTask) { try { await writeTask.ConfigureAwait(false); - await flushTask.ConfigureAwait(false); + await _stream.FlushAsync().ConfigureAwait(false); } catch (Exception exc) when (!(exc is OperationCanceledException)) { From 54542ac24bf724aa9f0d19c64d4f32788bda2ba3 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 11 Aug 2022 14:08:02 +0200 Subject: [PATCH 04/10] Flush for web socket tests should be supported --- .../System.Net.WebSockets/tests/WebSocketTestStream.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.WebSockets/tests/WebSocketTestStream.cs b/src/libraries/System.Net.WebSockets/tests/WebSocketTestStream.cs index b7dfb3ea7f26da..1e416f4dc49ccb 100644 --- a/src/libraries/System.Net.WebSockets/tests/WebSocketTestStream.cs +++ b/src/libraries/System.Net.WebSockets/tests/WebSocketTestStream.cs @@ -206,7 +206,7 @@ public override async ValueTask WriteAsync(ReadOnlyMemory buffer, Cancella Write(buffer.Span); } - public override void Flush() => throw new NotSupportedException(); + public override void Flush() { } public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException(); From daaaa5d0b43e3038310d091c24f404f830349fd5 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 11 Aug 2022 17:28:32 +0200 Subject: [PATCH 05/10] feedback --- .../src/System/Net/Http/HttpRequestMessage.cs | 2 ++ .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 4 ++-- .../tests/SendReceiveTest.Http2.cs | 6 ++++++ .../src/System/Net/WebSockets/ManagedWebSocket.cs | 9 +++------ 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs index f258c5ca8ae800..1083b164bc4170 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs @@ -170,6 +170,8 @@ public override string ToString() internal bool IsWebSocketH2Request() => _version.Major == 2 && Method == HttpMethod.Connect && HasHeaders && string.Equals(Headers.Protocol, "websocket", StringComparison.OrdinalIgnoreCase); + internal bool IsExtendedConnectRequest => Method == HttpMethod.Connect && _headers?.Protocol != null; + #region IDisposable Members protected virtual void Dispose(bool disposing) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 57466c7b0d0e7c..3b2019f5af657e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1600,7 +1600,7 @@ private async ValueTask SendHeadersAsync(HttpRequestMessage request // Start the write. This serializes access to write to the connection, and ensures that HEADERS // and CONTINUATION frames stay together, as they must do. We use the lock as well to ensure new // streams are created and started in order. - await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null && !request.IsWebSocketH2Request()), mustFlush), static (s, writeBuffer) => + await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null && !request.IsExtendedConnectRequest), mustFlush), static (s, writeBuffer) => { if (NetEventSource.Log.IsEnabled()) s.thisRef.Trace(s.http2Stream.StreamId, $"Started writing. Total header bytes={s.headerBytes.Length}"); @@ -1963,7 +1963,7 @@ public async Task SendAsync(HttpRequestMessage request, boo { // Send request headers bool shouldExpectContinue = (request.Content != null && request.HasHeaders && request.Headers.ExpectContinue == true); - Http2Stream http2Stream = await SendHeadersAsync(request, cancellationToken, mustFlush: shouldExpectContinue || request.IsWebSocketH2Request()).ConfigureAwait(false); + Http2Stream http2Stream = await SendHeadersAsync(request, cancellationToken, mustFlush: shouldExpectContinue || request.IsExtendedConnectRequest).ConfigureAwait(false); bool duplex = request.Content != null && request.Content.AllowDuplex; diff --git a/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.Http2.cs b/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.Http2.cs index 5f3be83d5bfb79..b229bcb4a5924c 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.Http2.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.Http2.cs @@ -57,6 +57,9 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async uri => // send status 200 OK to establish websocket await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false); + WebSocket websocket = WebSocket.CreateFromStream(connection.Stream, isServer: true, null, Timeout.InfiniteTimeSpan); + Assert.Equal(WebSocketState.Open, websocket.State); + // send reply byte binaryMessageType = 2; var prefix = new byte[] { binaryMessageType, (byte)serverMessage.Length }; @@ -97,6 +100,9 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async uri => // send status 200 OK to establish websocket await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false); + WebSocket websocket = WebSocket.CreateFromStream(connection.Stream, isServer: true, null, Timeout.InfiniteTimeSpan); + Assert.Equal(WebSocketState.Open, websocket.State); + // send reply byte binaryMessageType = 2; var prefix = new byte[] { binaryMessageType, (byte)serverMessage.Length }; 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 fa1491da9eb7ad..961343b1825f96 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 @@ -411,7 +411,6 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, // If we get here, the cancellation token is not cancelable so we don't have to worry about it, // and we own the semaphore, so we don't need to asynchronously wait for it. ValueTask writeTask = default; - Task flushTask; bool releaseSendBufferAndSemaphore = true; try { @@ -424,11 +423,9 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, // the task, and we're done. if (writeTask.IsCompleted) { - flushTask = _stream.FlushAsync(); - if (flushTask.IsCompleted) - { - return writeTask; - } + writeTask.GetAwaiter().GetResult(); + writeTask = new ValueTask(_stream.FlushAsync()); + return writeTask; } // Up until this point, if an exception occurred (such as when accessing _stream or when From 411d928ab9f11b98a75e86a07ab35a1d5aa04206 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 11 Aug 2022 19:16:00 +0200 Subject: [PATCH 06/10] Refactoring write and flush tasks in WebSocket --- .../System/Net/WebSockets/ManagedWebSocket.cs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) 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 961343b1825f96..d86f22f7dff04b 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 @@ -411,6 +411,7 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, // If we get here, the cancellation token is not cancelable so we don't have to worry about it, // and we own the semaphore, so we don't need to asynchronously wait for it. ValueTask writeTask = default; + ValueTask flushTask; bool releaseSendBufferAndSemaphore = true; try { @@ -424,8 +425,15 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, if (writeTask.IsCompleted) { writeTask.GetAwaiter().GetResult(); - writeTask = new ValueTask(_stream.FlushAsync()); - return writeTask; + flushTask = new ValueTask(_stream.FlushAsync()); + if (flushTask.IsCompleted) + { + return flushTask; + } + else + { + return WaitForWriteTaskAsync(flushTask, false); + } } // Up until this point, if an exception occurred (such as when accessing _stream or when @@ -449,15 +457,18 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, } } - return WaitForWriteTaskAsync(writeTask); + return WaitForWriteTaskAsync(writeTask, true); } - private async ValueTask WaitForWriteTaskAsync(ValueTask writeTask) + private async ValueTask WaitForWriteTaskAsync(ValueTask writeTask, bool shouldFlush) { try { await writeTask.ConfigureAwait(false); - await _stream.FlushAsync().ConfigureAwait(false); + if (shouldFlush) + { + await _stream.FlushAsync().ConfigureAwait(false); + } } catch (Exception exc) when (!(exc is OperationCanceledException)) { From 3de8b77b8d58349a15e607a444bc23ac2b51291c Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 11 Aug 2022 20:14:09 +0200 Subject: [PATCH 07/10] feedback --- .../src/System/Net/WebSockets/ManagedWebSocket.cs | 1 + 1 file changed, 1 insertion(+) 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 d86f22f7dff04b..6f4a2f6a06e02a 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 @@ -432,6 +432,7 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, } else { + releaseSendBufferAndSemaphore = false; return WaitForWriteTaskAsync(flushTask, false); } } From fa0f6de9dbb5eaf13f7d69e94b39b04e59252aed Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 12 Aug 2022 16:07:39 +0200 Subject: [PATCH 08/10] Replace check for more generic extended connect --- .../src/System/Net/Http/HttpRequestMessage.cs | 2 -- .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 6 +++--- .../tests/SendReceiveTest.Http2.cs | 6 ------ 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs index 1083b164bc4170..40884f72085e9e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs @@ -168,8 +168,6 @@ public override string ToString() internal bool WasRedirected() => (_sendStatus & MessageIsRedirect) != 0; - internal bool IsWebSocketH2Request() => _version.Major == 2 && Method == HttpMethod.Connect && HasHeaders && string.Equals(Headers.Protocol, "websocket", StringComparison.OrdinalIgnoreCase); - internal bool IsExtendedConnectRequest => Method == HttpMethod.Connect && _headers?.Protocol != null; #region IDisposable Members diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index e560915288e56a..7e6f148c72b9f8 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs @@ -1018,7 +1018,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn // Use HTTP/3 if possible. if (IsHttp3Supported() && // guard to enable trimming HTTP/3 support _http3Enabled && - !request.IsWebSocketH2Request() && + !request.IsExtendedConnectRequest && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) { Debug.Assert(async); @@ -1047,7 +1047,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn Debug.Assert(connection is not null || !_http2Enabled); if (connection is not null) { - if (request.IsWebSocketH2Request()) + if (request.IsExtendedConnectRequest) { await connection.InitialSettingsReceived.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false); if (!connection.IsConnectEnabled) @@ -1120,7 +1120,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn if (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { HttpRequestException exception = new HttpRequestException(SR.Format(SR.net_http_requested_version_server_refused, request.Version, request.VersionPolicy), e); - if (request.IsWebSocketH2Request()) + if (request.IsExtendedConnectRequest) { exception.Data["HTTP2_ENABLED"] = false; } diff --git a/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.Http2.cs b/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.Http2.cs index b229bcb4a5924c..5f3be83d5bfb79 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.Http2.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.Http2.cs @@ -57,9 +57,6 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async uri => // send status 200 OK to establish websocket await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false); - WebSocket websocket = WebSocket.CreateFromStream(connection.Stream, isServer: true, null, Timeout.InfiniteTimeSpan); - Assert.Equal(WebSocketState.Open, websocket.State); - // send reply byte binaryMessageType = 2; var prefix = new byte[] { binaryMessageType, (byte)serverMessage.Length }; @@ -100,9 +97,6 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async uri => // send status 200 OK to establish websocket await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false); - WebSocket websocket = WebSocket.CreateFromStream(connection.Stream, isServer: true, null, Timeout.InfiniteTimeSpan); - Assert.Equal(WebSocketState.Open, websocket.State); - // send reply byte binaryMessageType = 2; var prefix = new byte[] { binaryMessageType, (byte)serverMessage.Length }; From b621b042d871958f26fff03cf5d1dc16e8bb0ffa Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 12 Aug 2022 16:47:39 +0200 Subject: [PATCH 09/10] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../src/System/Net/WebSockets/ManagedWebSocket.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 6f4a2f6a06e02a..4a6058dc4879d6 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 @@ -433,7 +433,7 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, else { releaseSendBufferAndSemaphore = false; - return WaitForWriteTaskAsync(flushTask, false); + return WaitForWriteTaskAsync(flushTask, shouldFlush: false); } } @@ -458,7 +458,7 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, } } - return WaitForWriteTaskAsync(writeTask, true); + return WaitForWriteTaskAsync(writeTask, shouldFlush: true); } private async ValueTask WaitForWriteTaskAsync(ValueTask writeTask, bool shouldFlush) From 12ff77f026bfb5fe96acb45e5f377e97f601f090 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 12 Aug 2022 16:48:43 +0200 Subject: [PATCH 10/10] feedback --- .../src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs | 4 ++-- .../src/System/Net/WebSockets/ManagedWebSocket.cs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 1ecb968fa322a4..927a8e74b18a13 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -108,7 +108,7 @@ public Http2Stream(HttpRequestMessage request, Http2Connection connection) if (_request.Content == null) { _requestCompletionState = StreamCompletionState.Completed; - if (_request.IsWebSocketH2Request()) + if (_request.IsExtendedConnectRequest) { _requestBodyCancellationSource = new CancellationTokenSource(); } @@ -637,7 +637,7 @@ private void OnStatus(int statusCode) } else { - if (statusCode == 200 && _response.RequestMessage!.IsWebSocketH2Request()) + if (statusCode == 200 && _response.RequestMessage!.IsExtendedConnectRequest) { ConnectProtocolEstablished = true; } 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 4a6058dc4879d6..4c0c4e8d693b3f 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 @@ -411,7 +411,6 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, // If we get here, the cancellation token is not cancelable so we don't have to worry about it, // and we own the semaphore, so we don't need to asynchronously wait for it. ValueTask writeTask = default; - ValueTask flushTask; bool releaseSendBufferAndSemaphore = true; try { @@ -425,7 +424,7 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode, if (writeTask.IsCompleted) { writeTask.GetAwaiter().GetResult(); - flushTask = new ValueTask(_stream.FlushAsync()); + ValueTask flushTask = new ValueTask(_stream.FlushAsync()); if (flushTask.IsCompleted) { return flushTask;