From 96028726900a5bc2a4aafd5612ead6220ad40661 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 11 Apr 2024 14:16:14 +0200 Subject: [PATCH 01/28] QuicConnection StreamsAvailable properties and event handler --- .../System.Net.Quic/ref/System.Net.Quic.cs | 10 +++ .../src/Resources/Strings.resx | 5 +- .../Net/Quic/Internal/MsQuicExtensions.cs | 2 - .../System/Net/Quic/Internal/ThrowHelper.cs | 2 +- .../src/System/Net/Quic/QuicConnection.cs | 82 +++++++++++++++++++ .../FunctionalTests/QuicConnectionTests.cs | 43 ++++++++++ 6 files changed, 140 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs index 96a53e2bceb15..5bd411ea17f59 100644 --- a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs +++ b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs @@ -23,12 +23,15 @@ public QuicClientConnectionOptions() { } public sealed partial class QuicConnection : System.IAsyncDisposable { internal QuicConnection() { } + public int AvailableBidirectionalStreamsCount { get { throw null; } } + public int AvailableUnidirectionalStreamsCount { get { throw null; } } public static bool IsSupported { get { throw null; } } public System.Net.IPEndPoint LocalEndPoint { get { throw null; } } public System.Net.Security.SslApplicationProtocol NegotiatedApplicationProtocol { get { throw null; } } public System.Security.Cryptography.X509Certificates.X509Certificate? RemoteCertificate { get { throw null; } } public System.Net.IPEndPoint RemoteEndPoint { get { throw null; } } public string TargetHostName { get { throw null; } } + public event System.Net.Quic.QuicConnectionStreamsAvailableEventHandler? StreamsAvailable { add { } remove { } } public System.Threading.Tasks.ValueTask AcceptInboundStreamAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public System.Threading.Tasks.ValueTask CloseAsync(long errorCode, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public static System.Threading.Tasks.ValueTask ConnectAsync(System.Net.Quic.QuicClientConnectionOptions options, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } @@ -48,6 +51,13 @@ internal QuicConnectionOptions() { } public int MaxInboundBidirectionalStreams { get { throw null; } set { } } public int MaxInboundUnidirectionalStreams { get { throw null; } set { } } } + public partial class QuicConnectionStreamsAvailableEventArgs : System.EventArgs + { + internal QuicConnectionStreamsAvailableEventArgs() { } + public int BidirectionalStreamsCountIncrement { get { throw null; } } + public int UnidirectionalStreamsCountIncrement { get { throw null; } } + } + public delegate void QuicConnectionStreamsAvailableEventHandler(object? sender, System.Net.Quic.QuicConnectionStreamsAvailableEventArgs e); public enum QuicError { Success = 0, diff --git a/src/libraries/System.Net.Quic/src/Resources/Strings.resx b/src/libraries/System.Net.Quic/src/Resources/Strings.resx index 86e319ecaf782..65585ab0e43d5 100644 --- a/src/libraries/System.Net.Quic/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Quic/src/Resources/Strings.resx @@ -202,7 +202,10 @@ The server refused the connection. - A QUIC protocol error was encountered + A QUIC protocol error was encountered. + + + A specified application protocol is already in use. A version negotiation error was encountered. diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicExtensions.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicExtensions.cs index cfc5a09a37171..ba8148784b19a 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicExtensions.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicExtensions.cs @@ -54,8 +54,6 @@ public override string ToString() => $"{{ {nameof(DATAGRAM_RECEIVED.Flags)} = {DATAGRAM_RECEIVED.Flags} }}", QUIC_CONNECTION_EVENT_TYPE.DATAGRAM_SEND_STATE_CHANGED => $"{{ {nameof(DATAGRAM_SEND_STATE_CHANGED.ClientContext)} = 0x{(IntPtr)DATAGRAM_SEND_STATE_CHANGED.ClientContext:X11}, {nameof(DATAGRAM_SEND_STATE_CHANGED.State)} = {DATAGRAM_SEND_STATE_CHANGED.State} }}", - QUIC_CONNECTION_EVENT_TYPE.RESUMED - => $"{{ {nameof(RESUMED.ResumptionStateLength)} = {RESUMED.ResumptionStateLength} }}", QUIC_CONNECTION_EVENT_TYPE.RESUMPTION_TICKET_RECEIVED => $"{{ {nameof(RESUMPTION_TICKET_RECEIVED.ResumptionTicketLength)} = {RESUMPTION_TICKET_RECEIVED.ResumptionTicketLength} }}", QUIC_CONNECTION_EVENT_TYPE.PEER_CERTIFICATE_RECEIVED diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/ThrowHelper.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/ThrowHelper.cs index 85a06e12a0f91..456370e3d15d4 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/ThrowHelper.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/ThrowHelper.cs @@ -89,7 +89,7 @@ static Exception GetExceptionInternal(int status, long? errorCode, string? messa if (status == QUIC_STATUS_VER_NEG_ERROR) return new QuicException(QuicError.VersionNegotiationError, null, errorCode, SR.net_quic_ver_neg_error); if (status == QUIC_STATUS_CONNECTION_IDLE) return new QuicException(QuicError.ConnectionIdle, null, errorCode, SR.net_quic_connection_idle); if (status == QUIC_STATUS_PROTOCOL_ERROR) return new QuicException(QuicError.TransportError, null, errorCode, SR.net_quic_protocol_error); - if (status == QUIC_STATUS_ALPN_IN_USE) return new QuicException(QuicError.AlpnInUse, null, errorCode, SR.net_quic_protocol_error); + if (status == QUIC_STATUS_ALPN_IN_USE) return new QuicException(QuicError.AlpnInUse, null, errorCode, SR.net_quic_alpn_in_use); // // Transport errors will throw SocketException diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index 3316fc8050d33..2c2de01fcf7b9 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -18,6 +18,7 @@ using PEER_ADDRESS_CHANGED_DATA = Microsoft.Quic.QUIC_CONNECTION_EVENT._Anonymous_e__Union._PEER_ADDRESS_CHANGED_e__Struct; using PEER_CERTIFICATE_RECEIVED_DATA = Microsoft.Quic.QUIC_CONNECTION_EVENT._Anonymous_e__Union._PEER_CERTIFICATE_RECEIVED_e__Struct; using PEER_STREAM_STARTED_DATA = Microsoft.Quic.QUIC_CONNECTION_EVENT._Anonymous_e__Union._PEER_STREAM_STARTED_e__Struct; +using STREAMS_AVAILABLE_DATA = Microsoft.Quic.QUIC_CONNECTION_EVENT._Anonymous_e__Union._STREAMS_AVAILABLE_e__Struct; using SHUTDOWN_COMPLETE_DATA = Microsoft.Quic.QUIC_CONNECTION_EVENT._Anonymous_e__Union._SHUTDOWN_COMPLETE_e__Struct; using SHUTDOWN_INITIATED_BY_PEER_DATA = Microsoft.Quic.QUIC_CONNECTION_EVENT._Anonymous_e__Union._SHUTDOWN_INITIATED_BY_PEER_e__Struct; using SHUTDOWN_INITIATED_BY_TRANSPORT_DATA = Microsoft.Quic.QUIC_CONNECTION_EVENT._Anonymous_e__Union._SHUTDOWN_INITIATED_BY_TRANSPORT_e__Struct; @@ -179,6 +180,12 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// private IPEndPoint _localEndPoint = null!; /// + /// + private int _availableBidirectionalStreamsCount; + /// + /// + private int _availableUnidirectionalStreamsCount; + /// /// Keeps track whether has been accessed so that we know whether to dispose the certificate or not. /// private bool _remoteCertificateExposed; @@ -207,6 +214,33 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// public IPEndPoint LocalEndPoint => _localEndPoint; + /// + /// Returns the last known number of bidirectional stream that can be opened on this connection. + /// + public int AvailableBidirectionalStreamsCount => Volatile.Read(ref _availableBidirectionalStreamsCount); + /// + /// Returns the last known number of unidirectional stream that can be opened on this connection. + /// + public int AvailableUnidirectionalStreamsCount => Volatile.Read(ref _availableUnidirectionalStreamsCount); + + /// + /// Occurres when an additional stream capacity has been released by the peer. Corresponds to receiving MAX_STREAMS frame. + /// + public event QuicConnectionStreamsAvailableEventHandler? StreamsAvailable; + private async void OnStreamsAvailable(int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement) + { + // Bail out early to avoid queueing work on the thread pool as well as event args instantiation. + if (StreamsAvailable is null) + { + return; + } + + // Do not invoke user-defined event handler code on MsQuic thread. + await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding); + + StreamsAvailable?.Invoke(this, new QuicConnectionStreamsAvailableEventArgs(bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement)); + } + /// /// Gets the name of the server the client is trying to connect to. That name is used for server certificate validation. It can be a DNS name or an IP address. /// @@ -428,6 +462,14 @@ public async ValueTask OpenOutboundStreamAsync(QuicStreamType type, } await stream.StartAsync(cancellationToken).ConfigureAwait(false); + if (type == QuicStreamType.Unidirectional) + { + Interlocked.Decrement(ref _availableUnidirectionalStreamsCount); + } + else + { + Interlocked.Decrement(ref _availableBidirectionalStreamsCount); + } } catch (Exception ex) { @@ -532,6 +574,9 @@ private unsafe int HandleEventConnected(ref CONNECTED_DATA data) QuicAddr localAddress = MsQuicHelpers.GetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_ADDRESS); _localEndPoint = MsQuicHelpers.QuicAddrToIPEndPoint(&localAddress); + _availableBidirectionalStreamsCount = MsQuicHelpers.GetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_BIDI_STREAM_COUNT); + _availableUnidirectionalStreamsCount = MsQuicHelpers.GetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_UNIDI_STREAM_COUNT); + // Final (1-RTT) secrets have been derived, log them if desired to allow decrypting application traffic. _tlsSecret?.WriteSecret(); @@ -604,6 +649,15 @@ private unsafe int HandleEventPeerStreamStarted(ref PEER_STREAM_STARTED_DATA dat data.Flags |= QUIC_STREAM_OPEN_FLAGS.DELAY_ID_FC_UPDATES; return QUIC_STATUS_SUCCESS; } + private unsafe int HandleEventStreamsAvailable(ref STREAMS_AVAILABLE_DATA data) + { + int bidirectionalStreamsCountIncrement = data.BidirectionalCount - _availableBidirectionalStreamsCount; + int unidirectionalStreamsCountIncrement = data.UnidirectionalCount - _availableUnidirectionalStreamsCount; + Volatile.Write(ref _availableBidirectionalStreamsCount, data.BidirectionalCount); + Volatile.Write(ref _availableUnidirectionalStreamsCount, data.UnidirectionalCount); + OnStreamsAvailable(bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); + return QUIC_STATUS_SUCCESS; + } private unsafe int HandleEventPeerCertificateReceived(ref PEER_CERTIFICATE_RECEIVED_DATA data) { // @@ -635,6 +689,7 @@ private unsafe int HandleConnectionEvent(ref QUIC_CONNECTION_EVENT connectionEve QUIC_CONNECTION_EVENT_TYPE.LOCAL_ADDRESS_CHANGED => HandleEventLocalAddressChanged(ref connectionEvent.LOCAL_ADDRESS_CHANGED), QUIC_CONNECTION_EVENT_TYPE.PEER_ADDRESS_CHANGED => HandleEventPeerAddressChanged(ref connectionEvent.PEER_ADDRESS_CHANGED), QUIC_CONNECTION_EVENT_TYPE.PEER_STREAM_STARTED => HandleEventPeerStreamStarted(ref connectionEvent.PEER_STREAM_STARTED), + QUIC_CONNECTION_EVENT_TYPE.STREAMS_AVAILABLE => HandleEventStreamsAvailable(ref connectionEvent.STREAMS_AVAILABLE), QUIC_CONNECTION_EVENT_TYPE.PEER_CERTIFICATE_RECEIVED => HandleEventPeerCertificateReceived(ref connectionEvent.PEER_CERTIFICATE_RECEIVED), _ => QUIC_STATUS_SUCCESS, }; @@ -735,3 +790,30 @@ public async ValueTask DisposeAsync() } } } + +/// +/// Represents the method that will handle the event of a object. +/// +/// The source of the event. +/// A object that contains the event data. +public delegate void QuicConnectionStreamsAvailableEventHandler(object? sender, QuicConnectionStreamsAvailableEventArgs e); +/// +/// Provides data for the event. +/// +public partial class QuicConnectionStreamsAvailableEventArgs : EventArgs +{ + /// + /// The increment saying how many additional bidirectional streams can be opened on the connection. At the moment of event, corresponds to how much increased via the latest STREAMS_AVAILABLE frame. + /// + public int BidirectionalStreamsCountIncrement { get; } + /// + /// The increment saying how many additional unidirectional streams can be opened on the connection. At the moment of event, corresponds to how much increased via the latest STREAMS_AVAILABLE frame. + /// + public int UnidirectionalStreamsCountIncrement { get; } + + internal QuicConnectionStreamsAvailableEventArgs(int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement) + { + BidirectionalStreamsCountIncrement = bidirectionalStreamsCountIncrement; + UnidirectionalStreamsCountIncrement = unidirectionalStreamsCountIncrement; + } +} diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs index 3526ab513cd14..cc039fb449c6a 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs @@ -188,6 +188,49 @@ await RunClientServer( }); } + [Fact] + public async Task GetAvailableStreamsCount_OpenCloseStream_CountsCorrectly() + { + (QuicConnection clientConnection, QuicConnection serverConnection) = await CreateConnectedQuicConnection(); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, clientConnection.AvailableBidirectionalStreamsCount); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, clientConnection.AvailableUnidirectionalStreamsCount); + Assert.Equal(QuicDefaults.DefaultClientMaxInboundBidirectionalStreams, serverConnection.AvailableBidirectionalStreamsCount); + Assert.Equal(QuicDefaults.DefaultClientMaxInboundUnidirectionalStreams, serverConnection.AvailableUnidirectionalStreamsCount); + + SemaphoreSlim streamsAvailableFired = new SemaphoreSlim(0); + int bidiIncrement = -1, unidiIncrement = -1; + clientConnection.StreamsAvailable += (sender, eventArgs) => + { + bidiIncrement = eventArgs.BidirectionalStreamsCountIncrement; + unidiIncrement = eventArgs.UnidirectionalStreamsCountIncrement; + streamsAvailableFired.Release(); + }; + + var clientStreamBidi = await clientConnection.OpenOutboundStreamAsync(QuicStreamType.Bidirectional); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams - 1, clientConnection.AvailableBidirectionalStreamsCount); + await clientStreamBidi.DisposeAsync(); + var serverStreamBidi = await serverConnection.AcceptInboundStreamAsync(); + await serverStreamBidi.DisposeAsync(); + + // STREAMS_AVAILABLE event comes asynchronously, give it a chance to propagate + await streamsAvailableFired.WaitAsync(); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, clientConnection.AvailableBidirectionalStreamsCount); + Assert.Equal(1, bidiIncrement); + Assert.Equal(0, unidiIncrement); + + var clientStreamUnidi = await clientConnection.OpenOutboundStreamAsync(QuicStreamType.Unidirectional); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams - 1, clientConnection.AvailableUnidirectionalStreamsCount); + await clientStreamUnidi.DisposeAsync(); + var serverStreamUnidi = await serverConnection.AcceptInboundStreamAsync(); + await serverStreamUnidi.DisposeAsync(); + + // STREAMS_AVAILABLE event comes asynchronously, give it a chance to propagate + await streamsAvailableFired.WaitAsync(); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, clientConnection.AvailableUnidirectionalStreamsCount); + Assert.Equal(0, bidiIncrement); + Assert.Equal(1, unidiIncrement); + } + [Fact] public async Task ConnectionClosedByPeer_WithPendingAcceptAndConnect_PendingAndSubsequentThrowConnectionAbortedException() { From 366c740249f33742693a00c7478eb3a6a172340e Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 11 Apr 2024 14:17:13 +0200 Subject: [PATCH 02/28] EnableHttp3MultipleConnections property --- .../System.Net.Http/ref/System.Net.Http.cs | 1 + .../Http/BrowserHttpHandler/SocketsHttpHandler.cs | 6 ++++++ .../SocketsHttpHandler/HttpConnectionSettings.cs | 5 +++++ .../Http/SocketsHttpHandler/SocketsHttpHandler.cs | 15 +++++++++++++++ .../FunctionalTests/SocketsHttpHandlerTest.cs | 1 + .../tests/StressTests/HttpStress/StressClient.cs | 2 ++ 6 files changed, 30 insertions(+) diff --git a/src/libraries/System.Net.Http/ref/System.Net.Http.cs b/src/libraries/System.Net.Http/ref/System.Net.Http.cs index c529bdcda2d49..1261932527cca 100644 --- a/src/libraries/System.Net.Http/ref/System.Net.Http.cs +++ b/src/libraries/System.Net.Http/ref/System.Net.Http.cs @@ -424,6 +424,7 @@ public SocketsHttpHandler() { } public System.Net.ICredentials? Credentials { get { throw null; } set { } } public System.Net.ICredentials? DefaultProxyCredentials { get { throw null; } set { } } public bool EnableMultipleHttp2Connections { get { throw null; } set { } } + public bool EnableMultipleHttp3Connections { get { throw null; } set { } } public System.TimeSpan Expect100ContinueTimeout { get { throw null; } set { } } public int InitialHttp2StreamWindowSize { get { throw null; } set { } } [System.Runtime.Versioning.UnsupportedOSPlatformGuardAttribute("browser")] diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/SocketsHttpHandler.cs index 1e82a7b46ea08..955375e331793 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/SocketsHttpHandler.cs @@ -196,6 +196,12 @@ public bool EnableMultipleHttp2Connections set => throw new PlatformNotSupportedException(); } + public bool EnableMultipleHttp3Connections + { + get => throw new PlatformNotSupportedException(); + set => throw new PlatformNotSupportedException(); + } + public Func>? ConnectCallback { get => throw new PlatformNotSupportedException(); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs index 5cd1ee218c324..95026565c6ae4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs @@ -61,6 +61,8 @@ internal sealed class HttpConnectionSettings internal bool _enableMultipleHttp2Connections; + internal bool _enableMultipleHttp3Connections; + internal Func>? _connectCallback; internal Func>? _plaintextStreamFilter; @@ -123,6 +125,7 @@ public HttpConnectionSettings CloneAndNormalize() _requestHeaderEncodingSelector = _requestHeaderEncodingSelector, _responseHeaderEncodingSelector = _responseHeaderEncodingSelector, _enableMultipleHttp2Connections = _enableMultipleHttp2Connections, + _enableMultipleHttp3Connections = _enableMultipleHttp3Connections, _connectCallback = _connectCallback, _plaintextStreamFilter = _plaintextStreamFilter, _initialHttp2StreamWindowSize = _initialHttp2StreamWindowSize, @@ -140,6 +143,8 @@ public HttpConnectionSettings CloneAndNormalize() public bool EnableMultipleHttp2Connections => _enableMultipleHttp2Connections; + public bool EnableMultipleHttp3Connections => _enableMultipleHttp3Connections; + private byte[]? _http3SettingsFrame; [SupportedOSPlatform("windows")] diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs index 47d210a6ba838..eab45fd5707f7 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs @@ -372,6 +372,21 @@ public bool EnableMultipleHttp2Connections } } + /// + /// Gets or sets a value that indicates whether additional HTTP/3 connections can be established to the same server + /// when the maximum of concurrent streams is reached on all existing connections. + /// + public bool EnableMultipleHttp3Connections + { + get => _settings._enableMultipleHttp3Connections; + set + { + CheckDisposedOrStarted(); + + _settings._enableMultipleHttp3Connections = value; + } + } + internal const bool SupportsAutomaticDecompression = true; internal const bool SupportsProxy = true; internal const bool SupportsRedirectConfiguration = true; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index 62594756490ec..8c8b02536522b 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -2841,6 +2841,7 @@ private async Task VerifySendTasks(IReadOnlyList> send private static SocketsHttpHandler CreateHandler() => new SocketsHttpHandler { EnableMultipleHttp2Connections = true, + EnableMultipleHttp3Connections = true, PooledConnectionIdleTimeout = TimeSpan.FromHours(1), PooledConnectionLifetime = TimeSpan.FromHours(1), SslOptions = { RemoteCertificateValidationCallback = delegate { return true; } } diff --git a/src/libraries/System.Net.Http/tests/StressTests/HttpStress/StressClient.cs b/src/libraries/System.Net.Http/tests/StressTests/HttpStress/StressClient.cs index 4abd52e025edf..7520b49fae7f1 100644 --- a/src/libraries/System.Net.Http/tests/StressTests/HttpStress/StressClient.cs +++ b/src/libraries/System.Net.Http/tests/StressTests/HttpStress/StressClient.cs @@ -57,6 +57,8 @@ HttpMessageHandler CreateHttpHandler() return new SocketsHttpHandler() { PooledConnectionLifetime = _config.ConnectionLifetime.GetValueOrDefault(Timeout.InfiniteTimeSpan), + EnableMultipleHttp2Connections = true, + EnableMultipleHttp3Connections = true, SslOptions = new SslClientAuthenticationOptions { RemoteCertificateValidationCallback = delegate { return true; } From c1959d482b34919cf4a53f39ec4f48904aa99b0c Mon Sep 17 00:00:00 2001 From: ManickaP Date: Fri, 12 Apr 2024 20:04:43 +0200 Subject: [PATCH 03/28] H3 connection pool --- .../src/Resources/Strings.resx | 3 + .../Http/SocketsHttpHandler/ConnectHelper.cs | 10 +- .../HttpConnectionPool.Http3.cs | 570 ++++++++++++++++-- .../ConnectionPool/HttpConnectionPool.cs | 12 +- .../ConnectionPool/HttpConnectionWaiter.cs | 2 +- .../SocketsHttpHandler/Http3Connection.cs | 63 ++ 6 files changed, 587 insertions(+), 73 deletions(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index f234af52c717c..ea0161ab02b02 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -471,6 +471,9 @@ Received an invalid header name: '{0}'. + + An HTTP/3 connection could not be established because the server did not complete the HTTP/3 handshake. + The HTTP/3 server sent invalid data on the connection. HTTP/3 error code '{0}' (0x{1}). diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs index d8d43a4d250ea..c4b67fb3b8dc0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs @@ -118,7 +118,7 @@ public static async ValueTask ConnectQuicAsync(HttpRequestMessag try { - return await QuicConnection.ConnectAsync(new QuicClientConnectionOptions() + QuicConnection quicConnection = await QuicConnection.ConnectAsync(new QuicClientConnectionOptions() { MaxInboundBidirectionalStreams = 0, // Client doesn't support inbound streams: https://www.rfc-editor.org/rfc/rfc9114.html#name-bidirectional-streams. An extension might change this. MaxInboundUnidirectionalStreams = 5, // Minimum is 3: https://www.rfc-editor.org/rfc/rfc9114.html#unidirectional-streams (1x control stream + 2x QPACK). Set to 100 if/when support for PUSH streams is added. @@ -128,6 +128,14 @@ public static async ValueTask ConnectQuicAsync(HttpRequestMessag RemoteEndPoint = endPoint, ClientAuthenticationOptions = clientAuthenticationOptions }, cancellationToken).ConfigureAwait(false); + + if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) + { + await quicConnection.DisposeAsync().ConfigureAwait(false); + throw new HttpRequestException(HttpRequestError.ConnectionError, "QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnConnectionFailure); + } + + return quicConnection; } catch (Exception ex) when (ex is not OperationCanceledException) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index 227aa11ef8163..a03ce94049cf4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -3,9 +3,11 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Net.Http.Headers; using System.Net.Quic; using System.Net.Security; +using System.Runtime.ExceptionServices; using System.Runtime.Versioning; using System.Threading; using System.Threading.Tasks; @@ -28,9 +30,16 @@ internal sealed partial class HttpConnectionPool [SupportedOSPlatformGuard("Windows")] internal static bool IsHttp3Supported() => (OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); + /// List of available HTTP/3 connections stored in the pool. + private List? _availableHttp3Connections; + /// The number of HTTP/3 connections associated with the pool, including in use, available, and pending. + private int _associatedHttp3ConnectionCount; + /// Indicates whether an HTTP/3 connection is in the process of being established. + private bool _pendingHttp3Connection; + /// Queue of requests waiting for an HTTP/3 connection. + private RequestQueue _http3RequestQueue; + private bool _http3Enabled; - private Http3Connection? _http3Connection; - private SemaphoreSlim? _http3ConnectionCreateLock; internal readonly byte[]? _http3EncodedAuthorityHostHeader; /// Initially set to null, this can be set to enable HTTP/3 based on Alt-Svc. @@ -50,12 +59,19 @@ internal sealed partial class HttpConnectionPool private CancellationTokenSource? _altSvcBlocklistTimerCancellation; private volatile bool _altSvcEnabled = true; + private bool EnableMultipleHttp3Connections => _poolManager.Settings.EnableMultipleHttp3Connections; + // Returns null if HTTP3 cannot be used. [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] private async ValueTask TrySendUsingHttp3Async(HttpRequestMessage request, CancellationToken cancellationToken) { + Debug.Assert(IsHttp3Supported()); + + Debug.Assert(_kind == HttpConnectionKind.Https); + Debug.Assert(_http3Enabled); + // Loop in case we get a 421 and need to send the request to a different authority. while (true) { @@ -80,9 +96,10 @@ internal sealed partial class HttpConnectionPool long queueStartingTimestamp = HttpTelemetry.Log.IsEnabled() || Settings._metrics!.RequestsQueueDuration.Enabled ? Stopwatch.GetTimestamp() : 0; - ValueTask connectionTask = GetHttp3ConnectionAsync(request, authority, cancellationToken); - - Http3Connection connection = await connectionTask.ConfigureAwait(false); + if (!TryGetPooledHttp3Connection(request, authority, out Http3Connection? connection, out Http3ConnectionWaiter? http3ConnectionWaiter)) + { + connection = await http3ConnectionWaiter.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false); + } HttpResponseMessage response = await connection.SendAsync(request, queueStartingTimestamp, cancellationToken).ConfigureAwait(false); @@ -100,103 +117,528 @@ internal sealed partial class HttpConnectionPool } } + [SupportedOSPlatformGuard("linux")] + [SupportedOSPlatformGuard("macOS")] + [SupportedOSPlatformGuard("Windows")] + private bool TryGetPooledHttp3Connection(HttpRequestMessage request, HttpAuthority authority, [NotNullWhen(true)] out Http3Connection? connection, [NotNullWhen(false)] out Http3ConnectionWaiter? waiter) + { + Debug.Assert(IsHttp3Supported()); + + // Look for a usable connection. + while (true) + { + lock (SyncObj) + { + int availableConnectionCount = _availableHttp3Connections?.Count ?? 0; + if (availableConnectionCount > 0) + { + // We have a connection that we can attempt to use. + // Validate it below outside the lock, to avoid doing expensive operations while holding the lock. + connection = _availableHttp3Connections![availableConnectionCount - 1]; + } + else + { + // No available connections. Add to the request queue. + waiter = new Http3ConnectionWaiter(authority); + _http3RequestQueue.EnqueueRequest(request, waiter); + + CheckForHttp3ConnectionInjection(); + + // There were no available connections. This request has been added to the request queue. + if (NetEventSource.Log.IsEnabled()) Trace($"No available HTTP/3 connections; request queued."); + connection = null; + return false; + } + } + + if (CheckExpirationOnGet(connection)) + { + if (NetEventSource.Log.IsEnabled()) connection.Trace("Found expired HTTP/3 connection in pool."); + + InvalidateHttp3Connection(connection); + continue; + } + + // Disable and remove the connection from the pool only if we can open another. + // If we have only single connection, use the underlying QuicConnection mechanism to wait for available streams. + if (!connection.TryReserveStream() && EnableMultipleHttp3Connections) + { + if (NetEventSource.Log.IsEnabled()) connection.Trace("Found HTTP/3 connection in pool without available streams."); + + bool found = false; + lock (SyncObj) + { + int index = _availableHttp3Connections.IndexOf(connection); + if (index != -1) + { + found = true; + _availableHttp3Connections.RemoveAt(index); + } + } + + // If we didn't find the connection, then someone beat us to removing it (or it shut down) + if (found) + { + DisableHttp3Connection(connection); + } + continue; + } + + if (NetEventSource.Log.IsEnabled()) connection.Trace("Found usable HTTP/3 connection in pool."); + waiter = null; + return true; + } + } + + [SupportedOSPlatformGuard("linux")] + [SupportedOSPlatformGuard("macOS")] + [SupportedOSPlatformGuard("Windows")] + private void CheckForHttp3ConnectionInjection() + { + Debug.Assert(IsHttp3Supported()); + + Debug.Assert(HasSyncObjLock); + + _http3RequestQueue.PruneCompletedRequestsFromHeadOfQueue(this); + + // Determine if we can and should add a new connection to the pool. + int availableHttp3ConnectionCount = _availableHttp3Connections?.Count ?? 0; + bool willInject = availableHttp3ConnectionCount == 0 && // No available connections + !_pendingHttp3Connection && // Only allow one pending HTTP3 connection at a time + _http3RequestQueue.Count > 0 && // There are requests left on the queue + (_associatedHttp3ConnectionCount == 0 || EnableMultipleHttp3Connections) && // We allow multiple connections, or don't have a connection currently + _http3RequestQueue.RequestsWithoutAConnectionAttempt > 0; // There are requests we haven't issued a connection attempt for + + if (NetEventSource.Log.IsEnabled()) + { + Trace($"Available HTTP/3.0 connections: {availableHttp3ConnectionCount}, " + + $"Pending HTTP/3.0 connection: {_pendingHttp3Connection}, " + + $"Requests in the queue: {_http3RequestQueue.Count}, " + + $"Requests without a connection attempt: {_http3RequestQueue.RequestsWithoutAConnectionAttempt}, " + + $"Total associated HTTP/3.0 connections: {_associatedHttp3ConnectionCount}, " + + $"Will inject connection: {willInject}."); + } + + if (willInject) + { + _associatedHttp3ConnectionCount++; + _pendingHttp3Connection = true; + + RequestQueue.QueueItem queueItem = _http3RequestQueue.PeekNextRequestForConnectionAttempt(); + _ = InjectNewHttp3ConnectionAsync(queueItem); // ignore returned task + } + } + [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] - private async ValueTask GetHttp3ConnectionAsync(HttpRequestMessage request, HttpAuthority authority, CancellationToken cancellationToken) + private async ValueTask CreateHttp3ConnectionAsync(HttpRequestMessage request, HttpAuthority authority, CancellationToken cancellationToken) { - Debug.Assert(_kind == HttpConnectionKind.Https); - Debug.Assert(_http3Enabled); + Debug.Assert(IsHttp3Supported()); - Http3Connection? http3Connection = Volatile.Read(ref _http3Connection); + if (NetEventSource.Log.IsEnabled()) + { + Trace("Attempting new HTTP3 connection."); + } - if (http3Connection != null) + QuicConnection quicConnection; + try { - if (CheckExpirationOnGet(http3Connection) || http3Connection.Authority != authority) + if (IsAltSvcBlocked(authority, out Exception? reasonException)) { - // Connection expired. - if (NetEventSource.Log.IsEnabled()) http3Connection.Trace("Found expired HTTP3 connection."); - http3Connection.Dispose(); - InvalidateHttp3Connection(http3Connection); + ThrowGetVersionException(request, 3, reasonException); } - else + quicConnection = await ConnectHelper.ConnectQuicAsync(request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) + { + if (NetEventSource.Log.IsEnabled()) Trace($"QUIC connection failed: {ex}"); + + // Block list authority only if the connection attempt was not cancelled. + if (ex is not OperationCanceledException oce || !cancellationToken.IsCancellationRequested || oce.CancellationToken != cancellationToken) { - // Connection exists and it is still good to use. - if (NetEventSource.Log.IsEnabled()) Trace("Using existing HTTP3 connection."); - return http3Connection; + // Disables HTTP/3 until server announces it can handle it via Alt-Svc. + BlocklistAuthority(authority, ex); } + throw; } - // Ensure that the connection creation semaphore is created - if (_http3ConnectionCreateLock == null) + if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) { - lock (SyncObj) + BlocklistAuthority(authority); + throw new HttpRequestException(HttpRequestError.ConnectionError, "QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnConnectionFailure); + } + + // if the authority was sent as an option through alt-svc then include alt-used header + Http3Connection http3Connection = new Http3Connection(this, authority, quicConnection, includeAltUsedHeader: _http3Authority == authority); + + if (NetEventSource.Log.IsEnabled()) + { + Trace("New HTTP3 connection established."); + } + + return http3Connection; + } + + [SupportedOSPlatformGuard("linux")] + [SupportedOSPlatformGuard("macOS")] + [SupportedOSPlatformGuard("Windows")] + private async Task InjectNewHttp3ConnectionAsync(RequestQueue.QueueItem queueItem) + { + Debug.Assert(IsHttp3Supported()); + + if (NetEventSource.Log.IsEnabled()) Trace("Creating new HTTP/3 connection for pool."); + + // Queue the remainder of the work so that this method completes quickly + // and escapes locks held by the caller. + await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding); + + Http3Connection? connection = null; + Exception? connectionException = null; + Http3ConnectionWaiter waiter = (Http3ConnectionWaiter)queueItem.Waiter; + HttpAuthority authority = waiter.Authority; + + CancellationTokenSource cts = GetConnectTimeoutCancellationTokenSource(); + waiter.ConnectionCancellationTokenSource = cts; + try + { + QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(queueItem.Request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, cts.Token).ConfigureAwait(false); + + // if the authority was sent as an option through alt-svc then include alt-used header + connection = new Http3Connection(this, authority, quicConnection, includeAltUsedHeader: _http3Authority == authority); + + } + catch (Exception e) + { + connectionException = e is OperationCanceledException oce && oce.CancellationToken == cts.Token && !waiter.CancelledByOriginatingRequestCompletion ? + CreateConnectTimeoutException(oce) : + e; + } + finally + { + lock (waiter) { - _http3ConnectionCreateLock ??= new SemaphoreSlim(1); + waiter.ConnectionCancellationTokenSource = null; + cts.Dispose(); } } - await _http3ConnectionCreateLock.WaitAsync(cancellationToken).ConfigureAwait(false); - try + if (connection is not null) + { + // Add the new connection to the pool. + ReturnHttp3Connection(connection, isNewConnection: true, waiter); + } + else + { + Debug.Assert(connectionException is not null); + + // Block list authority only if the connection attempt was not cancelled. + if (connectionException is not OperationCanceledException) + { + // Disables HTTP/3 until server announces it can handle it via Alt-Svc. + BlocklistAuthority(authority, connectionException); + } + + HandleHttp3ConnectionFailure(waiter, connectionException); + } + } + + [SupportedOSPlatformGuard("linux")] + [SupportedOSPlatformGuard("macOS")] + [SupportedOSPlatformGuard("Windows")] + private void HandleHttp3ConnectionFailure(Http3ConnectionWaiter requestWaiter, Exception e) + { + Debug.Assert(IsHttp3Supported()); + + if (NetEventSource.Log.IsEnabled()) Trace($"HTTP3 connection failed: {e}"); + + // We don't care if this fails; that means the request was previously canceled or handled by a different connection. + requestWaiter.TrySetException(e); + + lock (SyncObj) + { + Debug.Assert(_associatedHttp3ConnectionCount > 0); + Debug.Assert(_pendingHttp3Connection); + + _associatedHttp3ConnectionCount--; + _pendingHttp3Connection = false; + + CheckForHttp3ConnectionInjection(); + } + } + + [SupportedOSPlatformGuard("linux")] + [SupportedOSPlatformGuard("macOS")] + [SupportedOSPlatformGuard("Windows")] + private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnection, Http3ConnectionWaiter? initialRequestWaiter = null) + { + Debug.Assert(IsHttp3Supported()); + + if (NetEventSource.Log.IsEnabled()) connection.Trace($"{nameof(isNewConnection)}={isNewConnection}"); + + Debug.Assert(!HasSyncObjLock); + Debug.Assert(isNewConnection || initialRequestWaiter is null, "Shouldn't have a request unless the connection is new"); + + if (!isNewConnection && CheckExpirationOnReturn(connection)) { - if (_http3Connection != null) + lock (SyncObj) { - // Someone beat us to creating the connection. + Debug.Assert(_availableHttp3Connections is null || !_availableHttp3Connections.Contains(connection)); + Debug.Assert(_associatedHttp3ConnectionCount > (_availableHttp3Connections?.Count ?? 0)); + _associatedHttp3ConnectionCount--; + } + + if (NetEventSource.Log.IsEnabled()) connection.Trace("Disposing HTTP3 connection return to pool. Connection lifetime expired."); + connection.Dispose(); + return; + } - if (NetEventSource.Log.IsEnabled()) + while (connection.TryReserveStream() || !EnableMultipleHttp3Connections) + { + // Loop in case we get a request that has already been canceled or handled by a different connection. + while (true) + { + HttpConnectionWaiter? waiter = null; + bool added = false; + lock (SyncObj) { - Trace("Using existing HTTP3 connection."); + Debug.Assert(_availableHttp3Connections is null || !_availableHttp3Connections.Contains(connection), $"HTTP3 connection already in available list"); + Debug.Assert(_associatedHttp3ConnectionCount > (_availableHttp3Connections?.Count ?? 0), + $"Expected _associatedHttp3ConnectionCount={_associatedHttp3ConnectionCount} > _availableHttp3Connections.Count={(_availableHttp3Connections?.Count ?? 0)}"); + + if (isNewConnection) + { + Debug.Assert(_pendingHttp3Connection); + _pendingHttp3Connection = false; + isNewConnection = false; + } + + if (initialRequestWaiter is not null) + { + // Try to handle the request that we initiated the connection for first + waiter = initialRequestWaiter; + initialRequestWaiter = null; + + // If this method found a request to service, that request must be removed from the queue if it was at the head to avoid rooting it forever. + // Normally, TryDequeueWaiter would handle the removal. TryDequeueSpecificWaiter matches this behavior for the initial request case. + // We don't care if this fails; that means the request was previously canceled, handled by a different connection, or not at the head of the queue. + _http3RequestQueue.TryDequeueSpecificWaiter(waiter); + } + else if (_http3RequestQueue.TryDequeueWaiter(this, out waiter)) + { + Debug.Assert((_availableHttp3Connections?.Count ?? 0) == 0, $"With {(_availableHttp3Connections?.Count ?? 0)} available HTTP3 connections, we shouldn't have a waiter."); + } + else if (_disposed) + { + // The pool has been disposed. We will dispose this connection below outside the lock. + // We do this check after processing the request queue so that any queued requests will be handled by existing connections if possible. + _associatedHttp3ConnectionCount--; + } + else + { + // Add connection to the pool. + added = true; + _availableHttp3Connections ??= new List(); + _availableHttp3Connections.Add(connection); + } } - return _http3Connection; + if (waiter is not null) + { + Debug.Assert(!added); + + if (waiter.TrySignal(connection)) + { + break; + } + + // Loop and process the queue again + } + else + { + connection.ReleaseStream(); + if (added) + { + if (NetEventSource.Log.IsEnabled()) connection.Trace("Put HTTP3 connection in pool."); + return; + } + else + { + Debug.Assert(_disposed); + if (NetEventSource.Log.IsEnabled()) connection.Trace("Disposing HTTP3 connection returned to pool. Pool was disposed."); + connection.Dispose(); + return; + } + } } + } + + if (isNewConnection) + { + Debug.Assert(initialRequestWaiter is not null, "Expect request for a new connection"); + + // The new connection could not handle even one request, either because it shut down before we could use it for any requests, + // or because it immediately set the max concurrent streams limit to 0. + // We don't want to get stuck in a loop where we keep trying to create new connections for the same request. + // So, treat this as a connection failure. - if (NetEventSource.Log.IsEnabled()) + if (NetEventSource.Log.IsEnabled()) connection.Trace("New HTTP3 connection is unusable due to no available streams."); + connection.Dispose(); + + HttpRequestException hre = new HttpRequestException(SR.net_http_http3_connection_not_established); + ExceptionDispatchInfo.SetCurrentStackTrace(hre); + HandleHttp3ConnectionFailure(initialRequestWaiter, hre); + } + else + { + // Since we only inject one connection at a time, we may want to inject another now. + lock (SyncObj) { - Trace("Attempting new HTTP3 connection."); + CheckForHttp3ConnectionInjection(); } - QuicConnection quicConnection; - try + // We need to wait until the connection is usable again. + DisableHttp3Connection(connection); + } + } + + /// + /// Disable usage of the specified connection because it cannot handle any more streams at the moment. + /// We will register to be notified when it can handle more streams (or becomes permanently unusable). + /// + [SupportedOSPlatformGuard("linux")] + [SupportedOSPlatformGuard("macOS")] + [SupportedOSPlatformGuard("Windows")] + private void DisableHttp3Connection(Http3Connection connection) + { + Debug.Assert(IsHttp3Supported()); + + if (NetEventSource.Log.IsEnabled()) connection.Trace(""); + + _ = DisableHttp3ConnectionAsync(connection); // ignore returned task + + async Task DisableHttp3ConnectionAsync(Http3Connection connection) + { + bool usable = await connection.WaitForAvailableStreamsAsync().ConfigureAwait(ConfigureAwaitOptions.ForceYielding); + + if (NetEventSource.Log.IsEnabled()) connection.Trace($"{nameof(connection.WaitForAvailableStreamsAsync)} completed, {nameof(usable)}={usable}"); + + if (usable) { - quicConnection = await ConnectHelper.ConnectQuicAsync(request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, cancellationToken).ConfigureAwait(false); + ReturnHttp3Connection(connection, isNewConnection: false); } - catch (Exception ex) + else { - if (NetEventSource.Log.IsEnabled()) Trace($"QUIC connection failed: {ex}"); - - // Block list authority only if the connection attempt was not cancelled. - if (ex is not OperationCanceledException oce || !cancellationToken.IsCancellationRequested || oce.CancellationToken != cancellationToken) + // Connection has shut down. + lock (SyncObj) { - // Disables HTTP/3 until server announces it can handle it via Alt-Svc. - BlocklistAuthority(authority, ex); + Debug.Assert(_availableHttp3Connections is null || !_availableHttp3Connections.Contains(connection)); + Debug.Assert(_associatedHttp3ConnectionCount > 0); + + _associatedHttp3ConnectionCount--; + + CheckForHttp3ConnectionInjection(); } - throw; - } - if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) - { - BlocklistAuthority(authority); - throw new HttpRequestException(HttpRequestError.ConnectionError, "QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnConnectionFailure); + if (NetEventSource.Log.IsEnabled()) connection.Trace("HTTP3 connection no longer usable"); + connection.Dispose(); } + }; + } - // if the authority was sent as an option through alt-svc then include alt-used header - http3Connection = new Http3Connection(this, authority, quicConnection, includeAltUsedHeader: _http3Authority == authority); - _http3Connection = http3Connection; + /// + /// Called when an Http3Connection from this pool is no longer usable. + /// + [SupportedOSPlatformGuard("linux")] + [SupportedOSPlatformGuard("macOS")] + [SupportedOSPlatformGuard("Windows")] + public void InvalidateHttp3Connection(Http3Connection connection) + { + Debug.Assert(IsHttp3Supported()); + + if (NetEventSource.Log.IsEnabled()) connection.Trace(""); - if (NetEventSource.Log.IsEnabled()) + bool found = false; + lock (SyncObj) + { + if (_availableHttp3Connections is not null) { - Trace("New HTTP3 connection established."); + Debug.Assert(_associatedHttp3ConnectionCount >= _availableHttp3Connections.Count); + + int index = _availableHttp3Connections.IndexOf(connection); + if (index != -1) + { + found = true; + _availableHttp3Connections.RemoveAt(index); + _associatedHttp3ConnectionCount--; + } } - return http3Connection; + CheckForHttp3ConnectionInjection(); } - finally + + // If we found the connection in the available list, then dispose it now. + // Otherwise, when we try to put it back in the pool, we will see it is shut down and dispose it (and adjust connection counts). + if (found) { - _http3ConnectionCreateLock.Release(); + connection.Dispose(); } } + [SupportedOSPlatformGuard("linux")] + [SupportedOSPlatformGuard("macOS")] + [SupportedOSPlatformGuard("Windows")] + private static int ScavengeHttp3ConnectionList(List list, ref List? toDispose, long nowTicks, TimeSpan pooledConnectionLifetime, TimeSpan pooledConnectionIdleTimeout) + { + Debug.Assert(IsHttp3Supported()); + + int freeIndex = 0; + while (freeIndex < list.Count && list[freeIndex].IsUsable(nowTicks, pooledConnectionLifetime, pooledConnectionIdleTimeout)) + { + freeIndex++; + } + + // If freeIndex == list.Count, nothing needs to be removed. + // But if it's < list.Count, at least one connection needs to be purged. + int removed = 0; + if (freeIndex < list.Count) + { + // We know the connection at freeIndex is unusable, so dispose of it. + toDispose ??= new List(); + toDispose.Add(list[freeIndex]); + + // Find the first item after the one to be removed that should be kept. + int current = freeIndex + 1; + while (current < list.Count) + { + // Look for the first item to be kept. Along the way, any + // that shouldn't be kept are disposed of. + while (current < list.Count && !list[current].IsUsable(nowTicks, pooledConnectionLifetime, pooledConnectionIdleTimeout)) + { + toDispose.Add(list[current]); + current++; + } + + // If we found something to keep, copy it down to the known free slot. + if (current < list.Count) + { + // copy item to the free slot + list[freeIndex++] = list[current++]; + } + + // Keep going until there are no more good items. + } + + // At this point, good connections have been moved below freeIndex, and garbage connections have + // been added to the dispose list, so clear the end of the list past freeIndex. + removed = list.Count - freeIndex; + list.RemoveRange(freeIndex, removed); + } + + return removed; + } + + /// Check for the Alt-Svc header, to upgrade to HTTP/3. private void ProcessAltSvc(HttpResponseMessage response) { @@ -436,17 +878,6 @@ internal void BlocklistAuthority(HttpAuthority badAuthority, Exception? exceptio } } - public void InvalidateHttp3Connection(Http3Connection connection) - { - lock (SyncObj) - { - if (_http3Connection == connection) - { - _http3Connection = null; - } - } - } - public void OnNetworkChanged() { lock (SyncObj) @@ -460,4 +891,9 @@ public void OnNetworkChanged() } } } + + internal sealed class Http3ConnectionWaiter(HttpAuthority authority) : HttpConnectionWaiter + { + public HttpAuthority Authority { get; init; } = authority; + } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs index 3fc2f43b01a8c..864915af14941 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs @@ -245,6 +245,10 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK { _http2RequestQueue = new RequestQueue(); } + if (IsHttp3Supported() && _http3Enabled) + { + _http3RequestQueue = new RequestQueue(); + } if (_proxyUri != null && HttpUtilities.IsSupportedSecureScheme(_proxyUri.Scheme)) { @@ -881,11 +885,11 @@ public void Dispose() _availableHttp2Connections.Clear(); } - if (_http3Connection is not null) + if (IsHttp3Supported() && _availableHttp3Connections is not null) { - toDispose ??= new(); - toDispose.Add(_http3Connection); - _http3Connection = null; + toDispose = [.. _availableHttp3Connections]; + _associatedHttp3ConnectionCount -= _availableHttp3Connections.Count; + _availableHttp3Connections.Clear(); } if (_authorityExpireTimer != null) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs index 3ca1412ebe711..560da5106f78a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs @@ -7,7 +7,7 @@ namespace System.Net.Http { - internal sealed class HttpConnectionWaiter : TaskCompletionSourceWithCancellation + internal class HttpConnectionWaiter : TaskCompletionSourceWithCancellation where T : HttpConnectionBase? { // When a connection attempt is pending, reference the connection's CTS, so we can tear it down if the initiating request is cancelled diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 75b9d9b1f8a96..b93a9aacff335 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -54,6 +54,10 @@ internal sealed class Http3Connection : HttpConnectionBase public Exception? AbortException => Volatile.Read(ref _abortException); private object SyncObj => _activeRequests; + private int _reservedStreams; + private TaskCompletionSource? _availableStreamsWaiter; + private bool _streamsAvailableRegistered; + /// /// If true, we've received GOAWAY, are aborting due to a connection-level error, or are disposing due to pool limits. /// @@ -66,6 +70,9 @@ private bool ShuttingDown } } + + public int AvailableRequestStreamsCount => _connection?.AvailableBidirectionalStreamsCount ?? 0; + public Http3Connection(HttpConnectionPool pool, HttpAuthority authority, QuicConnection connection, bool includeAltUsedHeader) : base(pool, connection.RemoteEndPoint) { @@ -128,6 +135,9 @@ private void CheckForShutdown() { // Close the QuicConnection in the background. + _availableStreamsWaiter?.SetResult(false); + _availableStreamsWaiter = null; + _connectionClosedTask ??= _connection.CloseAsync((long)Http3ErrorCode.NoError).AsTask(); QuicConnection connection = _connection; @@ -162,6 +172,59 @@ private void CheckForShutdown() } } + public bool TryReserveStream() + { + lock (SyncObj) + { + if (_reservedStreams >= AvailableRequestStreamsCount) + { + return false; + } + ++_reservedStreams; + return true; + } + } + + public void ReleaseStream() + { + lock (SyncObj) + { + Debug.Assert(_reservedStreams > 0); + --_reservedStreams; + } + } + + public Task WaitForAvailableStreamsAsync() + { + lock (SyncObj) + { + if (ShuttingDown) + { + return Task.FromResult(false); + } + if (_reservedStreams < AvailableRequestStreamsCount) + { + return Task.FromResult(true); + } + + Debug.Assert(_availableStreamsWaiter is null); + _availableStreamsWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + if (!_streamsAvailableRegistered) + { + _connection!.StreamsAvailable += (_, _) => + { + lock (SyncObj) + { + _availableStreamsWaiter?.SetResult(!ShuttingDown); + _availableStreamsWaiter = null; + } + }; + _streamsAvailableRegistered = true; + } + return _availableStreamsWaiter.Task; + } + } + public async Task SendAsync(HttpRequestMessage request, long queueStartingTimestamp, CancellationToken cancellationToken) { // Allocate an active request From 79d5cdb6e7b3fbede77b94a5b16891e3fc14511c Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 25 Apr 2024 10:00:14 +0200 Subject: [PATCH 04/28] Fixed unused methods. --- .../HttpConnectionPool.Http3.cs | 51 ------------------- .../ConnectionPool/HttpConnectionPool.cs | 8 +++ .../SocketsHttpHandler/Http3Connection.cs | 12 ++++- 3 files changed, 19 insertions(+), 52 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index a03ce94049cf4..59ba45975b6e7 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -229,57 +229,6 @@ private void CheckForHttp3ConnectionInjection() } } - [SupportedOSPlatform("windows")] - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macos")] - private async ValueTask CreateHttp3ConnectionAsync(HttpRequestMessage request, HttpAuthority authority, CancellationToken cancellationToken) - { - Debug.Assert(IsHttp3Supported()); - - if (NetEventSource.Log.IsEnabled()) - { - Trace("Attempting new HTTP3 connection."); - } - - QuicConnection quicConnection; - try - { - if (IsAltSvcBlocked(authority, out Exception? reasonException)) - { - ThrowGetVersionException(request, 3, reasonException); - } - quicConnection = await ConnectHelper.ConnectQuicAsync(request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, cancellationToken).ConfigureAwait(false); - } - catch (Exception ex) - { - if (NetEventSource.Log.IsEnabled()) Trace($"QUIC connection failed: {ex}"); - - // Block list authority only if the connection attempt was not cancelled. - if (ex is not OperationCanceledException oce || !cancellationToken.IsCancellationRequested || oce.CancellationToken != cancellationToken) - { - // Disables HTTP/3 until server announces it can handle it via Alt-Svc. - BlocklistAuthority(authority, ex); - } - throw; - } - - if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) - { - BlocklistAuthority(authority); - throw new HttpRequestException(HttpRequestError.ConnectionError, "QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnConnectionFailure); - } - - // if the authority was sent as an option through alt-svc then include alt-used header - Http3Connection http3Connection = new Http3Connection(this, authority, quicConnection, includeAltUsedHeader: _http3Authority == authority); - - if (NetEventSource.Log.IsEnabled()) - { - Trace("New HTTP3 connection established."); - } - - return http3Connection; - } - [SupportedOSPlatformGuard("linux")] [SupportedOSPlatformGuard("macOS")] [SupportedOSPlatformGuard("Windows")] diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs index 864915af14941..2e54207ba8c09 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs @@ -960,6 +960,14 @@ public bool CleanCacheAndDisposeIfUnused() // Note: Http11 connections will decrement the _associatedHttp11ConnectionCount when disposed. // Http2 connections will not, hence the difference in handing _associatedHttp2ConnectionCount. } + if (_availableHttp3Connections is not null) + { + int removed = ScavengeHttp3ConnectionList(_availableHttp3Connections, ref toDispose, nowTicks, pooledConnectionLifetime, pooledConnectionIdleTimeout); + _associatedHttp3ConnectionCount -= removed; + + // Note: Http11 connections will decrement the _associatedHttp11ConnectionCount when disposed. + // Http3 connections will not, hence the difference in handing _associatedHttp3ConnectionCount. + } } // Dispose the stale connections outside the pool lock, to avoid holding the lock too long. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index b93a9aacff335..d1f56db633b80 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -426,7 +426,17 @@ public void RemoveStream(QuicStream stream) } } - public override long GetIdleTicks(long nowTicks) => throw new NotImplementedException("We aren't scavenging HTTP3 connections yet"); + public override long GetIdleTicks(long nowTicks) + { + // The pool is holding the lock as part of its scavenging logic. + // We must not lock on Http2Connection.SyncObj here as that could lead to lock ordering problems. + Debug.Assert(_pool.HasSyncObjLock); + + // There is a race condition here where the connection pool may see this connection as idle right before + // we start processing a new request and start its disposal. This is okay as we will either + // return false from TryReserveStream, or process pending requests before tearing down the transport. + return _activeRequests.Count == 0 && _reservedStreams == 0 ? base.GetIdleTicks(nowTicks) : 0; + } public override void Trace(string message, [CallerMemberName] string? memberName = null) => Trace(0, message, memberName); From 653779316dfd3e88f26da18d0b8a76796e35fe0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= <11718369+ManickaP@users.noreply.github.com> Date: Thu, 25 Apr 2024 12:50:55 +0200 Subject: [PATCH 05/28] Update src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs Co-authored-by: Radek Zikmund <32671551+rzikm@users.noreply.github.com> --- .../src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index d1f56db633b80..403a0451e09fd 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -429,7 +429,7 @@ public void RemoveStream(QuicStream stream) public override long GetIdleTicks(long nowTicks) { // The pool is holding the lock as part of its scavenging logic. - // We must not lock on Http2Connection.SyncObj here as that could lead to lock ordering problems. + // We must not lock on Http3Connection.SyncObj here as that could lead to lock ordering problems. Debug.Assert(_pool.HasSyncObjLock); // There is a race condition here where the connection pool may see this connection as idle right before From 81bdce1818062cf557cc1526672b6b5dc34aeb33 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 25 Apr 2024 15:52:52 +0200 Subject: [PATCH 06/28] Fix release reserved stream count once it's opened. --- .../src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 403a0451e09fd..feb89a6484231 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -258,6 +258,7 @@ public async Task SendAsync(HttpRequestMessage request, lon catch (QuicException e) when (e.QuicError != QuicError.OperationAborted) { } finally { + ReleaseStream(); if (queueStartingTimestamp != 0) { TimeSpan duration = Stopwatch.GetElapsedTime(queueStartingTimestamp); From 7c6744f8375de79ffe36d3aa8d45d6ea6f561115 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Fri, 26 Apr 2024 19:09:24 +0200 Subject: [PATCH 07/28] Available streams count is thread safe --- .../src/System/Net/Quic/QuicConnection.cs | 35 +++++++++++++------ .../src/System/Net/Quic/QuicStream.cs | 15 +++++++- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index 2c2de01fcf7b9..a580b6d776b1e 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -180,9 +180,11 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// private IPEndPoint _localEndPoint = null!; /// + /// Represents how many bidirectional streams can be accepted by the peer. Is only manipulated from MsQuic thread. /// private int _availableBidirectionalStreamsCount; /// + /// Represents how many unidirectional streams can be accepted by the peer. Is only manipulated from MsQuic thread. /// private int _availableUnidirectionalStreamsCount; /// @@ -439,6 +441,25 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str return valueTask; } + /// + /// In order to provide meaningful increments in , available streams count can be only manipulated from MsQuic thread. + /// For that purpose we pass this function to so that it can call it from START_COMPLETE event handler. + /// + /// Note that MsQuic itself manipulates stream counts right before indicating START_COMPLETE event. + /// + /// Type of the stream to decrement appropriate field. + private void DecrementAvailableStreamCount(QuicStreamType streamType) + { + if (streamType == QuicStreamType.Unidirectional) + { + --_availableUnidirectionalStreamsCount; + } + else + { + --_availableBidirectionalStreamsCount; + } + } + /// /// Create an outbound uni/bidirectional . /// In case the connection doesn't have any available stream capacity, i.e.: the peer limits the concurrent stream count, @@ -461,15 +482,7 @@ public async ValueTask OpenOutboundStreamAsync(QuicStreamType type, NetEventSource.Info(this, $"{this} New outbound {type} stream {stream}."); } - await stream.StartAsync(cancellationToken).ConfigureAwait(false); - if (type == QuicStreamType.Unidirectional) - { - Interlocked.Decrement(ref _availableUnidirectionalStreamsCount); - } - else - { - Interlocked.Decrement(ref _availableBidirectionalStreamsCount); - } + await stream.StartAsync(DecrementAvailableStreamCount, cancellationToken).ConfigureAwait(false); } catch (Exception ex) { @@ -653,8 +666,8 @@ private unsafe int HandleEventStreamsAvailable(ref STREAMS_AVAILABLE_DATA data) { int bidirectionalStreamsCountIncrement = data.BidirectionalCount - _availableBidirectionalStreamsCount; int unidirectionalStreamsCountIncrement = data.UnidirectionalCount - _availableUnidirectionalStreamsCount; - Volatile.Write(ref _availableBidirectionalStreamsCount, data.BidirectionalCount); - Volatile.Write(ref _availableUnidirectionalStreamsCount, data.UnidirectionalCount); + _availableBidirectionalStreamsCount = data.BidirectionalCount; + _availableUnidirectionalStreamsCount = data.UnidirectionalCount; OnStreamsAvailable(bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); return QUIC_STATUS_SUCCESS; } diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs index 55058958a2141..1307a9ffceca8 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs @@ -120,6 +120,12 @@ public sealed partial class QuicStream private long _id = -1; private readonly QuicStreamType _type; + /// + /// Provided via from so that can decrement its available stream count field. + /// When START_COMPLETE arrives it gets invoked and unset back to null to not to hold any unintended reference to . + /// + private Action? _decrementAvailableStreamCount; + /// /// Stream id, see . /// @@ -239,9 +245,10 @@ internal unsafe QuicStream(MsQuicContextSafeHandle connectionHandle, QUIC_HANDLE /// If no more concurrent streams can be opened at the moment, the operation will wait until it can, /// either by closing some existing streams or receiving more available stream ids from the peer. /// + /// /// A cancellation token that can be used to cancel the asynchronous operation. /// An asynchronous task that completes with the opened . - internal ValueTask StartAsync(CancellationToken cancellationToken = default) + internal ValueTask StartAsync(Action decrementAvailableStreamCount, CancellationToken cancellationToken = default) { _startedTcs.TryInitialize(out ValueTask valueTask, this, cancellationToken); { @@ -258,6 +265,7 @@ internal ValueTask StartAsync(CancellationToken cancellationToken = default) } } + _decrementAvailableStreamCount = decrementAvailableStreamCount; return valueTask; } @@ -525,9 +533,13 @@ public void CompleteWrites() private unsafe int HandleEventStartComplete(ref START_COMPLETE_DATA data) { + Debug.Assert(_decrementAvailableStreamCount is not null); + _id = unchecked((long)data.ID); if (StatusSucceeded(data.Status)) { + _decrementAvailableStreamCount(Type); + if (data.PeerAccepted != 0) { _startedTcs.TrySetResult(); @@ -542,6 +554,7 @@ private unsafe int HandleEventStartComplete(ref START_COMPLETE_DATA data) } } + _decrementAvailableStreamCount = null; return QUIC_STATUS_SUCCESS; } private unsafe int HandleEventReceive(ref RECEIVE_DATA data) From c597fa5f31ac6ba27baa6bc191db3fd859787409 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Wed, 15 May 2024 18:24:06 +0200 Subject: [PATCH 08/28] Move StreamsAvailable callback into connection options, remove count properties --- .../System.Net.Quic/ref/System.Net.Quic.cs | 12 +--- .../src/System/Net/Quic/QuicConnection.cs | 60 ++++++------------- .../System/Net/Quic/QuicConnectionOptions.cs | 8 +++ .../src/System/Net/Quic/QuicStream.cs | 2 +- .../FunctionalTests/QuicConnectionTests.cs | 23 ++++--- 5 files changed, 38 insertions(+), 67 deletions(-) diff --git a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs index 5bd411ea17f59..538f8762bde62 100644 --- a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs +++ b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs @@ -23,15 +23,12 @@ public QuicClientConnectionOptions() { } public sealed partial class QuicConnection : System.IAsyncDisposable { internal QuicConnection() { } - public int AvailableBidirectionalStreamsCount { get { throw null; } } - public int AvailableUnidirectionalStreamsCount { get { throw null; } } public static bool IsSupported { get { throw null; } } public System.Net.IPEndPoint LocalEndPoint { get { throw null; } } public System.Net.Security.SslApplicationProtocol NegotiatedApplicationProtocol { get { throw null; } } public System.Security.Cryptography.X509Certificates.X509Certificate? RemoteCertificate { get { throw null; } } public System.Net.IPEndPoint RemoteEndPoint { get { throw null; } } public string TargetHostName { get { throw null; } } - public event System.Net.Quic.QuicConnectionStreamsAvailableEventHandler? StreamsAvailable { add { } remove { } } public System.Threading.Tasks.ValueTask AcceptInboundStreamAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public System.Threading.Tasks.ValueTask CloseAsync(long errorCode, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public static System.Threading.Tasks.ValueTask ConnectAsync(System.Net.Quic.QuicClientConnectionOptions options, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } @@ -50,14 +47,9 @@ internal QuicConnectionOptions() { } public System.TimeSpan KeepAliveInterval { get { throw null; } set { } } public int MaxInboundBidirectionalStreams { get { throw null; } set { } } public int MaxInboundUnidirectionalStreams { get { throw null; } set { } } + public System.Net.Quic.QuicConnectionStreamsAvailableCallback? StreamsAvailableCallback { get { throw null; } set { } } } - public partial class QuicConnectionStreamsAvailableEventArgs : System.EventArgs - { - internal QuicConnectionStreamsAvailableEventArgs() { } - public int BidirectionalStreamsCountIncrement { get { throw null; } } - public int UnidirectionalStreamsCountIncrement { get { throw null; } } - } - public delegate void QuicConnectionStreamsAvailableEventHandler(object? sender, System.Net.Quic.QuicConnectionStreamsAvailableEventArgs e); + public delegate void QuicConnectionStreamsAvailableCallback(System.Net.Quic.QuicConnection sender, int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement); public enum QuicError { Success = 0, diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index a580b6d776b1e..ed402bb12616e 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -180,6 +180,10 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// private IPEndPoint _localEndPoint = null!; /// + /// Occurres when an additional stream capacity has been released by the peer. Corresponds to receiving MAX_STREAMS frame. + /// + private QuicConnectionStreamsAvailableCallback? _streamsAvailableCallback; + /// /// Represents how many bidirectional streams can be accepted by the peer. Is only manipulated from MsQuic thread. /// private int _availableBidirectionalStreamsCount; @@ -216,23 +220,10 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// public IPEndPoint LocalEndPoint => _localEndPoint; - /// - /// Returns the last known number of bidirectional stream that can be opened on this connection. - /// - public int AvailableBidirectionalStreamsCount => Volatile.Read(ref _availableBidirectionalStreamsCount); - /// - /// Returns the last known number of unidirectional stream that can be opened on this connection. - /// - public int AvailableUnidirectionalStreamsCount => Volatile.Read(ref _availableUnidirectionalStreamsCount); - - /// - /// Occurres when an additional stream capacity has been released by the peer. Corresponds to receiving MAX_STREAMS frame. - /// - public event QuicConnectionStreamsAvailableEventHandler? StreamsAvailable; private async void OnStreamsAvailable(int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement) { // Bail out early to avoid queueing work on the thread pool as well as event args instantiation. - if (StreamsAvailable is null) + if (_streamsAvailableCallback is null) { return; } @@ -240,7 +231,7 @@ private async void OnStreamsAvailable(int bidirectionalStreamsCountIncrement, in // Do not invoke user-defined event handler code on MsQuic thread. await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding); - StreamsAvailable?.Invoke(this, new QuicConnectionStreamsAvailableEventArgs(bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement)); + _streamsAvailableCallback?.Invoke(this, bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); } /// @@ -336,6 +327,7 @@ private async ValueTask FinishConnectAsync(QuicClientConnectionOptions options, _canAccept = options.MaxInboundBidirectionalStreams > 0 || options.MaxInboundUnidirectionalStreams > 0; _defaultStreamErrorCode = options.DefaultStreamErrorCode; _defaultCloseErrorCode = options.DefaultCloseErrorCode; + _streamsAvailableCallback = options.StreamsAvailableCallback; if (!options.RemoteEndPoint.TryParse(out string? host, out IPAddress? address, out int port)) { @@ -412,6 +404,7 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str _canAccept = options.MaxInboundBidirectionalStreams > 0 || options.MaxInboundUnidirectionalStreams > 0; _defaultStreamErrorCode = options.DefaultStreamErrorCode; _defaultCloseErrorCode = options.DefaultCloseErrorCode; + _streamsAvailableCallback = options.StreamsAvailableCallback; // RFC 6066 forbids IP literals, avoid setting IP address here for consistency with SslStream if (TargetHostNameHelper.IsValidAddress(targetHost)) @@ -442,7 +435,7 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str } /// - /// In order to provide meaningful increments in , available streams count can be only manipulated from MsQuic thread. + /// In order to provide meaningful increments in , available streams count can be only manipulated from MsQuic thread. /// For that purpose we pass this function to so that it can call it from START_COMPLETE event handler. /// /// Note that MsQuic itself manipulates stream counts right before indicating START_COMPLETE event. @@ -587,9 +580,6 @@ private unsafe int HandleEventConnected(ref CONNECTED_DATA data) QuicAddr localAddress = MsQuicHelpers.GetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_ADDRESS); _localEndPoint = MsQuicHelpers.QuicAddrToIPEndPoint(&localAddress); - _availableBidirectionalStreamsCount = MsQuicHelpers.GetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_BIDI_STREAM_COUNT); - _availableUnidirectionalStreamsCount = MsQuicHelpers.GetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_UNIDI_STREAM_COUNT); - // Final (1-RTT) secrets have been derived, log them if desired to allow decrypting application traffic. _tlsSecret?.WriteSecret(); @@ -805,28 +795,12 @@ public async ValueTask DisposeAsync() } /// -/// Represents the method that will handle the event of a object. +/// Callback that is invoked when new stream limit is released by the peer. Corresponds to receiving MAX_STREAMS frame. +/// The callback values represent increments of stream limits, e.g.: current limit is 10 bidirectional streams, callback arguments notify 5 more additional bidirectional streams => 15 bidirectional streams can be opened in total at the moment. +/// The initial capacity is reported with the first invocation of the callback that might happen before the instance is handed out via either +/// or . /// -/// The source of the event. -/// A object that contains the event data. -public delegate void QuicConnectionStreamsAvailableEventHandler(object? sender, QuicConnectionStreamsAvailableEventArgs e); -/// -/// Provides data for the event. -/// -public partial class QuicConnectionStreamsAvailableEventArgs : EventArgs -{ - /// - /// The increment saying how many additional bidirectional streams can be opened on the connection. At the moment of event, corresponds to how much increased via the latest STREAMS_AVAILABLE frame. - /// - public int BidirectionalStreamsCountIncrement { get; } - /// - /// The increment saying how many additional unidirectional streams can be opened on the connection. At the moment of event, corresponds to how much increased via the latest STREAMS_AVAILABLE frame. - /// - public int UnidirectionalStreamsCountIncrement { get; } - - internal QuicConnectionStreamsAvailableEventArgs(int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement) - { - BidirectionalStreamsCountIncrement = bidirectionalStreamsCountIncrement; - UnidirectionalStreamsCountIncrement = unidirectionalStreamsCountIncrement; - } -} +/// The that received the new stream limits. +/// The increment saying how many additional bidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. +/// The increment saying how many additional unidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. +public delegate void QuicConnectionStreamsAvailableCallback(QuicConnection sender, int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement); diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs index b2d3b0d811f8f..ab9b749eeb873 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs @@ -124,6 +124,14 @@ public QuicReceiveWindowSizes InitialReceiveWindowSizes /// public TimeSpan HandshakeTimeout { get; set; } = QuicDefaults.HandshakeTimeout; + /// + /// Optional callback that is invoked when new stream limit is released by the peer. Corresponds to receiving MAX_STREAMS frame. + /// The callback values represent increments of stream limits, e.g.: current limit is 10 bidirectional streams, callback arguments notify 5 more additional bidirectional streams => 15 bidirectional streams can be opened in total at the moment. + /// The initial capacity is reported with the first invocation of the callback that might happen before the instance is handed out via either + /// or . + /// + public QuicConnectionStreamsAvailableCallback? StreamsAvailableCallback { get; set; } + /// /// Validates the options and potentially sets platform specific defaults. /// diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs index 1307a9ffceca8..6354ccf213139 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs @@ -252,6 +252,7 @@ internal ValueTask StartAsync(Action decrementAvailableStreamCou { _startedTcs.TryInitialize(out ValueTask valueTask, this, cancellationToken); { + _decrementAvailableStreamCount = decrementAvailableStreamCount; unsafe { int status = MsQuicApi.Api.StreamStart( @@ -265,7 +266,6 @@ internal ValueTask StartAsync(Action decrementAvailableStreamCou } } - _decrementAvailableStreamCount = decrementAvailableStreamCount; return valueTask; } diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs index cc039fb449c6a..5f6d79175aa9f 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs @@ -191,42 +191,39 @@ await RunClientServer( [Fact] public async Task GetAvailableStreamsCount_OpenCloseStream_CountsCorrectly() { - (QuicConnection clientConnection, QuicConnection serverConnection) = await CreateConnectedQuicConnection(); - Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, clientConnection.AvailableBidirectionalStreamsCount); - Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, clientConnection.AvailableUnidirectionalStreamsCount); - Assert.Equal(QuicDefaults.DefaultClientMaxInboundBidirectionalStreams, serverConnection.AvailableBidirectionalStreamsCount); - Assert.Equal(QuicDefaults.DefaultClientMaxInboundUnidirectionalStreams, serverConnection.AvailableUnidirectionalStreamsCount); - SemaphoreSlim streamsAvailableFired = new SemaphoreSlim(0); int bidiIncrement = -1, unidiIncrement = -1; - clientConnection.StreamsAvailable += (sender, eventArgs) => + + var clientOptions = CreateQuicClientOptions(new IPEndPoint(0, 0)); + clientOptions.StreamsAvailableCallback = (sender, bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement) => { - bidiIncrement = eventArgs.BidirectionalStreamsCountIncrement; - unidiIncrement = eventArgs.UnidirectionalStreamsCountIncrement; + bidiIncrement = bidirectionalStreamsCountIncrement; + unidiIncrement = unidirectionalStreamsCountIncrement; streamsAvailableFired.Release(); }; + (QuicConnection clientConnection, QuicConnection serverConnection) = await CreateConnectedQuicConnection(clientOptions); + await streamsAvailableFired.WaitAsync(); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, bidiIncrement); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, unidiIncrement); + var clientStreamBidi = await clientConnection.OpenOutboundStreamAsync(QuicStreamType.Bidirectional); - Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams - 1, clientConnection.AvailableBidirectionalStreamsCount); await clientStreamBidi.DisposeAsync(); var serverStreamBidi = await serverConnection.AcceptInboundStreamAsync(); await serverStreamBidi.DisposeAsync(); // STREAMS_AVAILABLE event comes asynchronously, give it a chance to propagate await streamsAvailableFired.WaitAsync(); - Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, clientConnection.AvailableBidirectionalStreamsCount); Assert.Equal(1, bidiIncrement); Assert.Equal(0, unidiIncrement); var clientStreamUnidi = await clientConnection.OpenOutboundStreamAsync(QuicStreamType.Unidirectional); - Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams - 1, clientConnection.AvailableUnidirectionalStreamsCount); await clientStreamUnidi.DisposeAsync(); var serverStreamUnidi = await serverConnection.AcceptInboundStreamAsync(); await serverStreamUnidi.DisposeAsync(); // STREAMS_AVAILABLE event comes asynchronously, give it a chance to propagate await streamsAvailableFired.WaitAsync(); - Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, clientConnection.AvailableUnidirectionalStreamsCount); Assert.Equal(0, bidiIncrement); Assert.Equal(1, unidiIncrement); } From 977e0bc3e6561ceabf78e22f2512f81a7ed2bb36 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 16 May 2024 11:13:57 +0200 Subject: [PATCH 09/28] HTTP adjustments to StreamsAvailable callback in options --- .../Http/SocketsHttpHandler/ConnectHelper.cs | 15 +--- .../HttpConnectionPool.Http3.cs | 15 +++- .../SocketsHttpHandler/Http2Connection.cs | 14 --- .../SocketsHttpHandler/Http3Connection.cs | 88 +++++++++---------- .../Http/SocketsHttpHandler/HttpConnection.cs | 3 - .../SocketsHttpHandler/HttpConnectionBase.cs | 39 +++++--- 6 files changed, 82 insertions(+), 92 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs index c4b67fb3b8dc0..603196aea0512 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs @@ -112,13 +112,13 @@ public static async ValueTask EstablishSslConnectionAsync(SslClientAu [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] - public static async ValueTask ConnectQuicAsync(HttpRequestMessage request, DnsEndPoint endPoint, TimeSpan idleTimeout, SslClientAuthenticationOptions clientAuthenticationOptions, CancellationToken cancellationToken) + public static async ValueTask ConnectQuicAsync(HttpRequestMessage request, DnsEndPoint endPoint, TimeSpan idleTimeout, SslClientAuthenticationOptions clientAuthenticationOptions, QuicConnectionStreamsAvailableCallback streamsAvailableCallback, CancellationToken cancellationToken) { clientAuthenticationOptions = SetUpRemoteCertificateValidationCallback(clientAuthenticationOptions, request); try { - QuicConnection quicConnection = await QuicConnection.ConnectAsync(new QuicClientConnectionOptions() + return await QuicConnection.ConnectAsync(new QuicClientConnectionOptions() { MaxInboundBidirectionalStreams = 0, // Client doesn't support inbound streams: https://www.rfc-editor.org/rfc/rfc9114.html#name-bidirectional-streams. An extension might change this. MaxInboundUnidirectionalStreams = 5, // Minimum is 3: https://www.rfc-editor.org/rfc/rfc9114.html#unidirectional-streams (1x control stream + 2x QPACK). Set to 100 if/when support for PUSH streams is added. @@ -126,16 +126,9 @@ public static async ValueTask ConnectQuicAsync(HttpRequestMessag DefaultStreamErrorCode = (long)Http3ErrorCode.RequestCancelled, DefaultCloseErrorCode = (long)Http3ErrorCode.NoError, RemoteEndPoint = endPoint, - ClientAuthenticationOptions = clientAuthenticationOptions + ClientAuthenticationOptions = clientAuthenticationOptions, + StreamsAvailableCallback = streamsAvailableCallback, }, cancellationToken).ConfigureAwait(false); - - if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) - { - await quicConnection.DisposeAsync().ConfigureAwait(false); - throw new HttpRequestException(HttpRequestError.ConnectionError, "QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnConnectionFailure); - } - - return quicConnection; } catch (Exception ex) when (ex is not OperationCanceledException) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index 59ba45975b6e7..d69005559cfde 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -251,17 +251,24 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue.Q waiter.ConnectionCancellationTokenSource = cts; try { - QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(queueItem.Request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, cts.Token).ConfigureAwait(false); - - // if the authority was sent as an option through alt-svc then include alt-used header - connection = new Http3Connection(this, authority, quicConnection, includeAltUsedHeader: _http3Authority == authority); + // If the authority was sent as an option through alt-svc then include alt-used header. + connection = new Http3Connection(this, authority, includeAltUsedHeader: _http3Authority == authority); + QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(queueItem.Request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, connection.StreamsAvailableCallback, cts.Token).ConfigureAwait(false); + if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) + { + await quicConnection.DisposeAsync().ConfigureAwait(false); + throw new HttpRequestException(HttpRequestError.ConnectionError, "QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnConnectionFailure); + } + connection.InitQuicConnection(quicConnection); } catch (Exception e) { connectionException = e is OperationCanceledException oce && oce.CancellationToken == cts.Token && !waiter.CancelledByOriginatingRequestCompletion ? CreateConnectTimeoutException(oce) : e; + // If the connection hasn't been initialized with QuicConnection there's no need to dispose it. + connection = null; } finally { 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 aec467cf2c740..2575c76201ec3 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 @@ -25,7 +25,6 @@ internal sealed partial class Http2Connection : HttpConnectionBase private TaskCompletionSourceWithCancellation? _initialSettingsReceived; - private readonly HttpConnectionPool _pool; private readonly Stream _stream; // NOTE: These are mutable structs; do not make these readonly. @@ -132,7 +131,6 @@ internal enum KeepAliveState public Http2Connection(HttpConnectionPool pool, Stream stream, IPEndPoint? remoteEndPoint) : base(pool, remoteEndPoint) { - _pool = pool; _stream = stream; _incomingBuffer = new ArrayBuffer(initialSize: 0, usePool: true); @@ -1794,18 +1792,6 @@ private bool ForceSendConnectionWindowUpdate() return true; } - public override long GetIdleTicks(long nowTicks) - { - // The pool is holding the lock as part of its scavenging logic. - // We must not lock on Http2Connection.SyncObj here as that could lead to lock ordering problems. - Debug.Assert(_pool.HasSyncObjLock); - - // There is a race condition here where the connection pool may see this connection as idle right before - // we start processing a new request and start its disposal. This is okay as we will either - // return false from TryReserveStream, or process pending requests before tearing down the transport. - return _streamsInUse == 0 ? base.GetIdleTicks(nowTicks) : 0; - } - /// Abort all streams and cause further processing to fail. /// Exception causing Abort to be called. private void Abort(Exception abortException) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index feb89a6484231..57e0b182699ff 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -19,7 +19,6 @@ namespace System.Net.Http [SupportedOSPlatform("macos")] internal sealed class Http3Connection : HttpConnectionBase { - private readonly HttpConnectionPool _pool; private readonly HttpAuthority _authority; private readonly byte[]? _altUsedEncodedHeader; private QuicConnection? _connection; @@ -33,7 +32,7 @@ internal sealed class Http3Connection : HttpConnectionBase // Our control stream. private QuicStream? _clientControl; - private Task _sendSettingsTask; + private Task? _sendSettingsTask; // Server-advertised SETTINGS_MAX_FIELD_SECTION_SIZE // https://www.rfc-editor.org/rfc/rfc9114.html#section-7.2.4.1-2.2.1 @@ -54,9 +53,8 @@ internal sealed class Http3Connection : HttpConnectionBase public Exception? AbortException => Volatile.Read(ref _abortException); private object SyncObj => _activeRequests; - private int _reservedStreams; + private int _availableRequestStreamsCount; private TaskCompletionSource? _availableStreamsWaiter; - private bool _streamsAvailableRegistered; /// /// If true, we've received GOAWAY, are aborting due to a connection-level error, or are disposing due to pool limits. @@ -70,15 +68,10 @@ private bool ShuttingDown } } - - public int AvailableRequestStreamsCount => _connection?.AvailableBidirectionalStreamsCount ?? 0; - - public Http3Connection(HttpConnectionPool pool, HttpAuthority authority, QuicConnection connection, bool includeAltUsedHeader) - : base(pool, connection.RemoteEndPoint) + public Http3Connection(HttpConnectionPool pool, HttpAuthority authority, bool includeAltUsedHeader) + : base(pool) { - _pool = pool; _authority = authority; - _connection = connection; if (includeAltUsedHeader) { @@ -94,6 +87,13 @@ public Http3Connection(HttpConnectionPool pool, HttpAuthority authority, QuicCon // Use this as an initial value before we receive the SETTINGS frame. _maxHeaderListSize = maxHeaderListSize; } + } + + public void InitQuicConnection(QuicConnection connection) + { + MarkConnectionAsEstablished(connection.RemoteEndPoint); + + _connection = connection; // Errors are observed via Abort(). _sendSettingsTask = SendSettingsAsync(); @@ -161,7 +161,7 @@ private void CheckForShutdown() if (_clientControl != null) { - await _sendSettingsTask.ConfigureAwait(false); + await _sendSettingsTask!.ConfigureAwait(false); await _clientControl.DisposeAsync().ConfigureAwait(false); _clientControl = null; } @@ -176,11 +176,18 @@ public bool TryReserveStream() { lock (SyncObj) { - if (_reservedStreams >= AvailableRequestStreamsCount) + Debug.Assert(_availableRequestStreamsCount >= 0); + + if (_availableRequestStreamsCount == 0) { return false; } - ++_reservedStreams; + + if (_activeRequests.Count == 0) + { + MarkConnectionAsNotIdle(); + } + --_availableRequestStreamsCount; return true; } } @@ -189,8 +196,23 @@ public void ReleaseStream() { lock (SyncObj) { - Debug.Assert(_reservedStreams > 0); - --_reservedStreams; + Debug.Assert(_availableRequestStreamsCount >= 0); + + ++_availableRequestStreamsCount; + } + } + + public void StreamsAvailableCallback(QuicConnection sender, int bidirectionalStreamsCountIncrement, int _) + { + Debug.Assert(_connection is null || sender == _connection); + + lock (SyncObj) + { + Debug.Assert(_availableRequestStreamsCount >= 0); + + _availableRequestStreamsCount += bidirectionalStreamsCountIncrement; + _availableStreamsWaiter?.SetResult(!ShuttingDown); + _availableStreamsWaiter = null; } } @@ -198,29 +220,18 @@ public Task WaitForAvailableStreamsAsync() { lock (SyncObj) { + Debug.Assert(_availableRequestStreamsCount >= 0); + if (ShuttingDown) { return Task.FromResult(false); } - if (_reservedStreams < AvailableRequestStreamsCount) + if (_availableRequestStreamsCount > 0) { return Task.FromResult(true); } - Debug.Assert(_availableStreamsWaiter is null); _availableStreamsWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - if (!_streamsAvailableRegistered) - { - _connection!.StreamsAvailable += (_, _) => - { - lock (SyncObj) - { - _availableStreamsWaiter?.SetResult(!ShuttingDown); - _availableStreamsWaiter = null; - } - }; - _streamsAvailableRegistered = true; - } return _availableStreamsWaiter.Task; } } @@ -243,11 +254,6 @@ public async Task SendAsync(HttpRequestMessage request, lon requestStream = new Http3RequestStream(request, this, quicStream); lock (SyncObj) { - if (_activeRequests.Count == 0) - { - MarkConnectionAsNotIdle(); - } - _activeRequests.Add(quicStream, requestStream); } } @@ -427,18 +433,6 @@ public void RemoveStream(QuicStream stream) } } - public override long GetIdleTicks(long nowTicks) - { - // The pool is holding the lock as part of its scavenging logic. - // We must not lock on Http3Connection.SyncObj here as that could lead to lock ordering problems. - Debug.Assert(_pool.HasSyncObjLock); - - // There is a race condition here where the connection pool may see this connection as idle right before - // we start processing a new request and start its disposal. This is okay as we will either - // return false from TryReserveStream, or process pending requests before tearing down the transport. - return _activeRequests.Count == 0 && _reservedStreams == 0 ? base.GetIdleTicks(nowTicks) : 0; - } - public override void Trace(string message, [CallerMemberName] string? memberName = null) => Trace(0, message, memberName); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index 2208a6265c97d..0d5fca33126b8 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -42,7 +42,6 @@ internal sealed partial class HttpConnection : HttpConnectionBase private static readonly ulong s_http10Bytes = BitConverter.ToUInt64("HTTP/1.0"u8); private static readonly ulong s_http11Bytes = BitConverter.ToUInt64("HTTP/1.1"u8); - private readonly HttpConnectionPool _pool; internal readonly Stream _stream; private readonly TransportContext? _transportContext; @@ -77,10 +76,8 @@ public HttpConnection( IPEndPoint? remoteEndPoint) : base(pool, remoteEndPoint) { - Debug.Assert(pool != null); Debug.Assert(stream != null); - _pool = pool; _stream = stream; _transportContext = transportContext; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs index d1a48491674ed..fa9d5bc4e2b3c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs @@ -16,17 +16,19 @@ namespace System.Net.Http { internal abstract class HttpConnectionBase : IDisposable, IHttpTrace { + protected readonly HttpConnectionPool _pool; + private static long s_connectionCounter = -1; // May be null if none of the counters were enabled when the connection was established. - private readonly ConnectionMetrics? _connectionMetrics; + private ConnectionMetrics? _connectionMetrics; // Indicates whether we've counted this connection as established, so that we can // avoid decrementing the counter once it's closed in case telemetry was enabled in between. - private readonly bool _httpTelemetryMarkedConnectionAsOpened; + private bool _httpTelemetryMarkedConnectionAsOpened; private readonly long _creationTickCount = Environment.TickCount64; - private long _idleSinceTickCount; + private long? _idleSinceTickCount; /// Cached string for the last Date header received on this connection. private string? _lastDateHeaderValue; @@ -35,13 +37,23 @@ internal abstract class HttpConnectionBase : IDisposable, IHttpTrace public long Id { get; } = Interlocked.Increment(ref s_connectionCounter); - public HttpConnectionBase(HttpConnectionPool pool, IPEndPoint? remoteEndPoint) + public HttpConnectionBase(HttpConnectionPool pool) { Debug.Assert(this is HttpConnection or Http2Connection or Http3Connection); - Debug.Assert(pool.Settings._metrics is not null); + Debug.Assert(pool != null); + _pool = pool; + } + public HttpConnectionBase(HttpConnectionPool pool, IPEndPoint? remoteEndPoint) + : this(pool) + { + MarkConnectionAsEstablished(remoteEndPoint); + } - SocketsHttpHandlerMetrics metrics = pool.Settings._metrics; + protected void MarkConnectionAsEstablished(IPEndPoint? remoteEndPoint) + { + Debug.Assert(_pool.Settings._metrics is not null); + SocketsHttpHandlerMetrics metrics = _pool.Settings._metrics; if (metrics.OpenConnections.Enabled || metrics.ConnectionDuration.Enabled) { // While requests may report HTTP/1.0 as the protocol, we treat all HTTP/1.X connections as HTTP/1.1. @@ -53,9 +65,9 @@ public HttpConnectionBase(HttpConnectionPool pool, IPEndPoint? remoteEndPoint) _connectionMetrics = new ConnectionMetrics( metrics, protocol, - pool.IsSecure ? "https" : "http", - pool.OriginAuthority.HostValue, - pool.IsDefaultPort ? null : pool.OriginAuthority.Port, + _pool.IsSecure ? "https" : "http", + _pool.OriginAuthority.HostValue, + _pool.IsDefaultPort ? null : _pool.OriginAuthority.Port, remoteEndPoint?.Address?.ToString()); _connectionMetrics.ConnectionEstablished(); @@ -67,9 +79,9 @@ public HttpConnectionBase(HttpConnectionPool pool, IPEndPoint? remoteEndPoint) { _httpTelemetryMarkedConnectionAsOpened = true; - string scheme = pool.IsSecure ? "https" : "http"; - string host = pool.OriginAuthority.HostValue; - int port = pool.OriginAuthority.Port; + string scheme = _pool.IsSecure ? "https" : "http"; + string host = _pool.OriginAuthority.HostValue; + int port = _pool.OriginAuthority.Port; if (this is HttpConnection) HttpTelemetry.Log.Http11ConnectionEstablished(Id, scheme, host, port, remoteEndPoint); else if (this is Http2Connection) HttpTelemetry.Log.Http20ConnectionEstablished(Id, scheme, host, port, remoteEndPoint); @@ -101,6 +113,7 @@ public void MarkConnectionAsIdle() public void MarkConnectionAsNotIdle() { + _idleSinceTickCount = null; _connectionMetrics?.IdleStateChanged(idle: false); } @@ -146,7 +159,7 @@ protected void TraceConnection(Stream stream) public long GetLifetimeTicks(long nowTicks) => nowTicks - _creationTickCount; - public virtual long GetIdleTicks(long nowTicks) => nowTicks - _idleSinceTickCount; + public long GetIdleTicks(long nowTicks) => _idleSinceTickCount is long idleSinceTickCount ? nowTicks - idleSinceTickCount : 0; /// Check whether a connection is still usable, or should be scavenged. /// True if connection can be used. From a8e8e769c96876e195fdad3f8d019ffe34c79afa Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 16 May 2024 15:37:59 +0200 Subject: [PATCH 10/28] Comments about multiple connections, including mention of being against RFC --- .../Http/SocketsHttpHandler/SocketsHttpHandler.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs index eab45fd5707f7..a20b9c5648541 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs @@ -358,9 +358,11 @@ public HttpKeepAlivePingPolicy KeepAlivePingPolicy } /// - /// Gets or sets a value that indicates whether additional HTTP/2 connections can be established to the same server - /// when the maximum of concurrent streams is reached on all existing connections. + /// Gets or sets a value that indicates whether additional HTTP/2 connections can be established to the same server. /// + /// + /// Enabling multiple connections to the same server explicitly goes against RFC 9113 - HTTP/2. + /// public bool EnableMultipleHttp2Connections { get => _settings._enableMultipleHttp2Connections; @@ -373,9 +375,11 @@ public bool EnableMultipleHttp2Connections } /// - /// Gets or sets a value that indicates whether additional HTTP/3 connections can be established to the same server - /// when the maximum of concurrent streams is reached on all existing connections. + /// Gets or sets a value that indicates whether additional HTTP/3 connections can be established to the same server. /// + /// + /// Enabling multiple connections to the same server explicitly goes against RFC 9114 - HTTP/3. + /// public bool EnableMultipleHttp3Connections { get => _settings._enableMultipleHttp3Connections; From 3ef524b963f4f8ceaab991246d30b4e9c0038459 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 20 May 2024 14:48:58 +0200 Subject: [PATCH 11/28] Miha's Feedback: H/3 authority evaluation, user callback try-catch --- .../HttpConnectionPool.Http3.cs | 128 +++++++++++------- .../ConnectionPool/HttpConnectionPool.cs | 7 +- .../ConnectionPool/HttpConnectionWaiter.cs | 2 +- .../System.Net.Quic/ref/System.Net.Quic.cs | 2 +- .../src/System/Net/Quic/QuicConnection.cs | 17 ++- 5 files changed, 102 insertions(+), 54 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index d69005559cfde..1260eb1534adb 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -37,7 +37,7 @@ internal sealed partial class HttpConnectionPool /// Indicates whether an HTTP/3 connection is in the process of being established. private bool _pendingHttp3Connection; /// Queue of requests waiting for an HTTP/3 connection. - private RequestQueue _http3RequestQueue; + private RequestQueue _http3RequestQueue; private bool _http3Enabled; internal readonly byte[]? _http3EncodedAuthorityHostHeader; @@ -75,32 +75,29 @@ internal sealed partial class HttpConnectionPool // Loop in case we get a 421 and need to send the request to a different authority. while (true) { - HttpAuthority? authority = _http3Authority; - - // If H3 is explicitly requested, assume prenegotiated H3. - if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) - { - authority ??= _originAuthority; - } - - if (authority == null) - { - return null; - } - - Exception? reasonException; - if (IsAltSvcBlocked(authority, out reasonException)) + if (!TryGetHttp3Authority(request, out _, out Exception? reasonException)) { + if (reasonException is null) + { + return null; + } ThrowGetVersionException(request, 3, reasonException); } long queueStartingTimestamp = HttpTelemetry.Log.IsEnabled() || Settings._metrics!.RequestsQueueDuration.Enabled ? Stopwatch.GetTimestamp() : 0; - if (!TryGetPooledHttp3Connection(request, authority, out Http3Connection? connection, out Http3ConnectionWaiter? http3ConnectionWaiter)) + if (!TryGetPooledHttp3Connection(request, out Http3Connection? connection, out HttpConnectionWaiter? http3ConnectionWaiter)) { connection = await http3ConnectionWaiter.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false); } + // Request cannot be sent over H/3 connection, try downgrade or report failure. + // Note that if there's an H/3 suitable origin authority but is unavailable or blocked via Alt-Svc, exception is thrown instead. + if (connection is null) + { + return null; + } + HttpResponseMessage response = await connection.SendAsync(request, queueStartingTimestamp, cancellationToken).ConfigureAwait(false); // If an Alt-Svc authority returns 421, it means it can't actually handle the request. @@ -120,7 +117,7 @@ internal sealed partial class HttpConnectionPool [SupportedOSPlatformGuard("linux")] [SupportedOSPlatformGuard("macOS")] [SupportedOSPlatformGuard("Windows")] - private bool TryGetPooledHttp3Connection(HttpRequestMessage request, HttpAuthority authority, [NotNullWhen(true)] out Http3Connection? connection, [NotNullWhen(false)] out Http3ConnectionWaiter? waiter) + private bool TryGetPooledHttp3Connection(HttpRequestMessage request, [NotNullWhen(true)] out Http3Connection? connection, [NotNullWhen(false)] out HttpConnectionWaiter? waiter) { Debug.Assert(IsHttp3Supported()); @@ -139,8 +136,7 @@ private bool TryGetPooledHttp3Connection(HttpRequestMessage request, HttpAuthori else { // No available connections. Add to the request queue. - waiter = new Http3ConnectionWaiter(authority); - _http3RequestQueue.EnqueueRequest(request, waiter); + waiter = _http3RequestQueue.EnqueueRequest(request); CheckForHttp3ConnectionInjection(); @@ -224,7 +220,7 @@ private void CheckForHttp3ConnectionInjection() _associatedHttp3ConnectionCount++; _pendingHttp3Connection = true; - RequestQueue.QueueItem queueItem = _http3RequestQueue.PeekNextRequestForConnectionAttempt(); + RequestQueue.QueueItem queueItem = _http3RequestQueue.PeekNextRequestForConnectionAttempt(); _ = InjectNewHttp3ConnectionAsync(queueItem); // ignore returned task } } @@ -232,7 +228,7 @@ private void CheckForHttp3ConnectionInjection() [SupportedOSPlatformGuard("linux")] [SupportedOSPlatformGuard("macOS")] [SupportedOSPlatformGuard("Windows")] - private async Task InjectNewHttp3ConnectionAsync(RequestQueue.QueueItem queueItem) + private async Task InjectNewHttp3ConnectionAsync(RequestQueue.QueueItem queueItem) { Debug.Assert(IsHttp3Supported()); @@ -244,23 +240,30 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue.Q Http3Connection? connection = null; Exception? connectionException = null; - Http3ConnectionWaiter waiter = (Http3ConnectionWaiter)queueItem.Waiter; - HttpAuthority authority = waiter.Authority; + HttpAuthority? authority = null; + HttpConnectionWaiter waiter = queueItem.Waiter; CancellationTokenSource cts = GetConnectTimeoutCancellationTokenSource(); waiter.ConnectionCancellationTokenSource = cts; try { - // If the authority was sent as an option through alt-svc then include alt-used header. - connection = new Http3Connection(this, authority, includeAltUsedHeader: _http3Authority == authority); + if (TryGetHttp3Authority(queueItem.Request, out authority, out Exception? reasonException)) + { + // If the authority was sent as an option through alt-svc then include alt-used header. + connection = new Http3Connection(this, authority, includeAltUsedHeader: _http3Authority == authority); - QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(queueItem.Request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, connection.StreamsAvailableCallback, cts.Token).ConfigureAwait(false); - if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) + QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(queueItem.Request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, connection.StreamsAvailableCallback, cts.Token).ConfigureAwait(false); + if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) + { + await quicConnection.DisposeAsync().ConfigureAwait(false); + throw new HttpRequestException(HttpRequestError.ConnectionError, "QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnConnectionFailure); + } + connection.InitQuicConnection(quicConnection); + } + else if (reasonException is not null) { - await quicConnection.DisposeAsync().ConfigureAwait(false); - throw new HttpRequestException(HttpRequestError.ConnectionError, "QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnConnectionFailure); + ThrowGetVersionException(queueItem.Request, 3, reasonException); } - connection.InitQuicConnection(quicConnection); } catch (Exception e) { @@ -286,10 +289,8 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue.Q } else { - Debug.Assert(connectionException is not null); - // Block list authority only if the connection attempt was not cancelled. - if (connectionException is not OperationCanceledException) + if (connectionException is not null && connectionException is not OperationCanceledException && authority is not null) { // Disables HTTP/3 until server announces it can handle it via Alt-Svc. BlocklistAuthority(authority, connectionException); @@ -302,14 +303,21 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue.Q [SupportedOSPlatformGuard("linux")] [SupportedOSPlatformGuard("macOS")] [SupportedOSPlatformGuard("Windows")] - private void HandleHttp3ConnectionFailure(Http3ConnectionWaiter requestWaiter, Exception e) + private void HandleHttp3ConnectionFailure(HttpConnectionWaiter requestWaiter, Exception? e) { Debug.Assert(IsHttp3Supported()); if (NetEventSource.Log.IsEnabled()) Trace($"HTTP3 connection failed: {e}"); // We don't care if this fails; that means the request was previously canceled or handled by a different connection. - requestWaiter.TrySetException(e); + if (e is null) + { + requestWaiter.TrySetResult(null); + } + else + { + requestWaiter.TrySetException(e); + } lock (SyncObj) { @@ -326,7 +334,7 @@ private void HandleHttp3ConnectionFailure(Http3ConnectionWaiter requestWaiter, E [SupportedOSPlatformGuard("linux")] [SupportedOSPlatformGuard("macOS")] [SupportedOSPlatformGuard("Windows")] - private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnection, Http3ConnectionWaiter? initialRequestWaiter = null) + private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnection, HttpConnectionWaiter? initialRequestWaiter = null) { Debug.Assert(IsHttp3Supported()); @@ -349,12 +357,13 @@ private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnect return; } - while (connection.TryReserveStream() || !EnableMultipleHttp3Connections) + bool reserved; + while ((reserved = connection.TryReserveStream()) || !EnableMultipleHttp3Connections) { // Loop in case we get a request that has already been canceled or handled by a different connection. while (true) { - HttpConnectionWaiter? waiter = null; + HttpConnectionWaiter? waiter = null; bool added = false; lock (SyncObj) { @@ -412,7 +421,10 @@ private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnect } else { - connection.ReleaseStream(); + if (reserved) + { + connection.ReleaseStream(); + } if (added) { if (NetEventSource.Log.IsEnabled()) connection.Trace("Put HTTP3 connection in pool."); @@ -594,6 +606,30 @@ private static int ScavengeHttp3ConnectionList(List list, ref L return removed; } + private bool TryGetHttp3Authority(HttpRequestMessage request, [NotNullWhen(true)] out HttpAuthority? authority, out Exception? reasonException) + { + authority = _http3Authority; + + // If H3 is explicitly requested, assume pre-negotiated H3. + if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + authority ??= _originAuthority; + } + + if (authority is null) + { + reasonException = null; + return false; + } + + if (IsAltSvcBlocked(authority, out reasonException)) + { + return false; + } + + return true; + } + /// Check for the Alt-Svc header, to upgrade to HTTP/3. private void ProcessAltSvc(HttpResponseMessage response) @@ -689,7 +725,10 @@ internal void HandleAltSvc(IEnumerable altSvcHeaderValues, TimeSpan? res var wr = (WeakReference)o!; if (wr.TryGetTarget(out HttpConnectionPool? @this)) { - @this.ExpireAltSvcAuthority(); + lock (@this.SyncObj) + { + @this.ExpireAltSvcAuthority(); + } } }, thisRef, nextAuthorityMaxAge, Timeout.InfiniteTimeSpan); } @@ -717,6 +756,8 @@ internal void HandleAltSvc(IEnumerable altSvcHeaderValues, TimeSpan? res /// private void ExpireAltSvcAuthority() { + Debug.Assert(HasSyncObjLock); + // If we ever support prenegotiated HTTP/3, this should be set to origin, not nulled out. _http3Authority = null; } @@ -847,9 +888,4 @@ public void OnNetworkChanged() } } } - - internal sealed class Http3ConnectionWaiter(HttpAuthority authority) : HttpConnectionWaiter - { - public HttpAuthority Authority { get; init; } = authority; - } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs index 2e54207ba8c09..d4f271acfa681 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs @@ -247,7 +247,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK } if (IsHttp3Supported() && _http3Enabled) { - _http3RequestQueue = new RequestQueue(); + _http3RequestQueue = new RequestQueue(); } if (_proxyUri != null && HttpUtilities.IsSupportedSecureScheme(_proxyUri.Scheme)) @@ -887,7 +887,8 @@ public void Dispose() if (IsHttp3Supported() && _availableHttp3Connections is not null) { - toDispose = [.. _availableHttp3Connections]; + toDispose ??= new(); + toDispose.AddRange(_availableHttp3Connections); _associatedHttp3ConnectionCount -= _availableHttp3Connections.Count; _availableHttp3Connections.Clear(); } @@ -960,7 +961,7 @@ public bool CleanCacheAndDisposeIfUnused() // Note: Http11 connections will decrement the _associatedHttp11ConnectionCount when disposed. // Http2 connections will not, hence the difference in handing _associatedHttp2ConnectionCount. } - if (_availableHttp3Connections is not null) + if (IsHttp3Supported() && _availableHttp3Connections is not null) { int removed = ScavengeHttp3ConnectionList(_availableHttp3Connections, ref toDispose, nowTicks, pooledConnectionLifetime, pooledConnectionIdleTimeout); _associatedHttp3ConnectionCount -= removed; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs index 560da5106f78a..3ca1412ebe711 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs @@ -7,7 +7,7 @@ namespace System.Net.Http { - internal class HttpConnectionWaiter : TaskCompletionSourceWithCancellation + internal sealed class HttpConnectionWaiter : TaskCompletionSourceWithCancellation where T : HttpConnectionBase? { // When a connection attempt is pending, reference the connection's CTS, so we can tear it down if the initiating request is cancelled diff --git a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs index 538f8762bde62..a0987763d1070 100644 --- a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs +++ b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs @@ -49,7 +49,7 @@ internal QuicConnectionOptions() { } public int MaxInboundUnidirectionalStreams { get { throw null; } set { } } public System.Net.Quic.QuicConnectionStreamsAvailableCallback? StreamsAvailableCallback { get { throw null; } set { } } } - public delegate void QuicConnectionStreamsAvailableCallback(System.Net.Quic.QuicConnection sender, int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement); + public delegate void QuicConnectionStreamsAvailableCallback(System.Net.Quic.QuicConnection connection, int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement); public enum QuicError { Success = 0, diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index ed402bb12616e..184c05f9aab42 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -231,7 +231,18 @@ private async void OnStreamsAvailable(int bidirectionalStreamsCountIncrement, in // Do not invoke user-defined event handler code on MsQuic thread. await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding); - _streamsAvailableCallback?.Invoke(this, bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); + try + { + _streamsAvailableCallback(this, bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); + } + catch (Exception ex) + { + // Just log the exception, we're on a thread-pool thread and there's no way to report this to anyone. + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Info(this, $"{this} {nameof(QuicConnectionOptions.StreamsAvailableCallback)} failed with {ex}."); + } + } } /// @@ -800,7 +811,7 @@ public async ValueTask DisposeAsync() /// The initial capacity is reported with the first invocation of the callback that might happen before the instance is handed out via either /// or . /// -/// The that received the new stream limits. +/// The that received the new stream limits. /// The increment saying how many additional bidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. /// The increment saying how many additional unidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. -public delegate void QuicConnectionStreamsAvailableCallback(QuicConnection sender, int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement); +public delegate void QuicConnectionStreamsAvailableCallback(QuicConnection connection, int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement); From 119b483c7c00c1b12f024ca8bfb68a068dea0e50 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 23 May 2024 11:35:58 +0200 Subject: [PATCH 12/28] Fix releasing stream --- .../src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 57e0b182699ff..82ee91be4ad15 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -264,7 +264,6 @@ public async Task SendAsync(HttpRequestMessage request, lon catch (QuicException e) when (e.QuicError != QuicError.OperationAborted) { } finally { - ReleaseStream(); if (queueStartingTimestamp != 0) { TimeSpan duration = Stopwatch.GetElapsedTime(queueStartingTimestamp); @@ -280,6 +279,7 @@ public async Task SendAsync(HttpRequestMessage request, lon if (quicStream == null) { + ReleaseStream(); throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_request_aborted, null, RequestRetryType.RetryOnConnectionFailure); } From e96769087e383a528601f1c7302977ffd9ea60d0 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 23 May 2024 13:22:11 +0200 Subject: [PATCH 13/28] Added logging for stream counts --- .../Net/Http/SocketsHttpHandler/Http3Connection.cs | 11 ++++++++--- .../src/System/Net/Quic/QuicConnection.cs | 12 ++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 82ee91be4ad15..d70b00c2ff78b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -178,6 +178,8 @@ public bool TryReserveStream() { Debug.Assert(_availableRequestStreamsCount >= 0); + if (NetEventSource.Log.IsEnabled()) Trace($"TryReserveStream: _availableRequestStreamsCount = {_availableRequestStreamsCount}"); + if (_availableRequestStreamsCount == 0) { return false; @@ -198,18 +200,21 @@ public void ReleaseStream() { Debug.Assert(_availableRequestStreamsCount >= 0); + if (NetEventSource.Log.IsEnabled()) Trace($"ReleaseStream: _availableRequestStreamsCount = {_availableRequestStreamsCount}"); ++_availableRequestStreamsCount; } } - public void StreamsAvailableCallback(QuicConnection sender, int bidirectionalStreamsCountIncrement, int _) + public void StreamsAvailableCallback(QuicConnection connection, int bidirectionalStreamsCountIncrement, int _) { - Debug.Assert(_connection is null || sender == _connection); + Debug.Assert(_connection is null || connection == _connection); lock (SyncObj) { Debug.Assert(_availableRequestStreamsCount >= 0); + if (NetEventSource.Log.IsEnabled()) Trace($"StreamsAvailableCallback: _availableRequestStreamsCount = {_availableRequestStreamsCount} + bidirectionalStreamsCountIncrement = {bidirectionalStreamsCountIncrement}"); + _availableRequestStreamsCount += bidirectionalStreamsCountIncrement; _availableStreamsWaiter?.SetResult(!ShuttingDown); _availableStreamsWaiter = null; @@ -434,7 +439,7 @@ public void RemoveStream(QuicStream stream) } public override void Trace(string message, [CallerMemberName] string? memberName = null) => - Trace(0, message, memberName); + Trace(0, _connection is not null ? $"{_connection} {message}" : message, memberName); internal void Trace(long streamId, string message, [CallerMemberName] string? memberName = null) => NetEventSource.Log.HandlerMessage( diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index 184c05f9aab42..e6ecd315b3fed 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -233,6 +233,10 @@ private async void OnStreamsAvailable(int bidirectionalStreamsCountIncrement, in try { + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Info(this, $"{this} Signaling StreamsAvailable with {bidirectionalStreamsCountIncrement} bidirectional increment (absolute value {_availableBidirectionalStreamsCount}) and {unidirectionalStreamsCountIncrement} unidirectional increment (absolute value {_availableUnidirectionalStreamsCount})."); + } _streamsAvailableCallback(this, bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); } catch (Exception ex) @@ -457,10 +461,18 @@ private void DecrementAvailableStreamCount(QuicStreamType streamType) if (streamType == QuicStreamType.Unidirectional) { --_availableUnidirectionalStreamsCount; + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Info(this, $"{this} decremented stream count for {streamType} to {_availableUnidirectionalStreamsCount}."); + } } else { --_availableBidirectionalStreamsCount; + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Info(this, $"{this} decremented stream count for {streamType} to {_availableBidirectionalStreamsCount}."); + } } } From 19cac0b31b90ab4b4e7bc95b966e78aa4ee39a2a Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 28 May 2024 14:59:51 +0200 Subject: [PATCH 14/28] Fix stream counting in case of open stream failure --- .../SocketsHttpHandler/Http3Connection.cs | 1 - .../src/System/Net/Quic/QuicStream.cs | 23 ++++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index d70b00c2ff78b..be0173b10b675 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -284,7 +284,6 @@ public async Task SendAsync(HttpRequestMessage request, lon if (quicStream == null) { - ReleaseStream(); throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_request_aborted, null, RequestRetryType.RetryOnConnectionFailure); } diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs index 6354ccf213139..5dbcbb7bfecd2 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs @@ -250,19 +250,20 @@ internal unsafe QuicStream(MsQuicContextSafeHandle connectionHandle, QUIC_HANDLE /// An asynchronous task that completes with the opened . internal ValueTask StartAsync(Action decrementAvailableStreamCount, CancellationToken cancellationToken = default) { + Debug.Assert(!_startedTcs.IsCompleted); + + // Always call StreamStart to get consistent behavior (events, stream count, frames send to peer) regardless of cancellation. _startedTcs.TryInitialize(out ValueTask valueTask, this, cancellationToken); + _decrementAvailableStreamCount = decrementAvailableStreamCount; + unsafe { - _decrementAvailableStreamCount = decrementAvailableStreamCount; - unsafe - { - int status = MsQuicApi.Api.StreamStart( - _handle, - QUIC_STREAM_START_FLAGS.SHUTDOWN_ON_FAIL | QUIC_STREAM_START_FLAGS.INDICATE_PEER_ACCEPT); + int status = MsQuicApi.Api.StreamStart( + _handle, + QUIC_STREAM_START_FLAGS.SHUTDOWN_ON_FAIL | QUIC_STREAM_START_FLAGS.INDICATE_PEER_ACCEPT); - if (ThrowHelper.TryGetStreamExceptionForMsQuicStatus(status, out Exception? exception, streamWasSuccessfullyStarted: false)) - { - _startedTcs.TrySetException(exception); - } + if (ThrowHelper.TryGetStreamExceptionForMsQuicStatus(status, out Exception? exception, streamWasSuccessfullyStarted: false)) + { + _startedTcs.TrySetException(exception); } } @@ -641,7 +642,7 @@ private unsafe int HandleEventShutdownComplete(ref SHUTDOWN_COMPLETE_DATA data) _receiveTcs.TrySetException(exception, final: true); _sendTcs.TrySetException(exception, final: true); } - _startedTcs.TrySetResult(); + _startedTcs.TrySetException(ThrowHelper.GetOperationAbortedException()); _shutdownTcs.TrySetResult(); return QUIC_STATUS_SUCCESS; } From 366730aecfbc834d1ada340db4b11e976eb8330c Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 28 May 2024 16:00:32 +0200 Subject: [PATCH 15/28] Feedback --- .../HttpConnectionPool.Http3.cs | 33 +++++-------------- .../SocketsHttpHandler/Http3Connection.cs | 3 ++ 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index 1260eb1534adb..93ea60136d797 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -270,7 +270,9 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue. connectionException = e is OperationCanceledException oce && oce.CancellationToken == cts.Token && !waiter.CancelledByOriginatingRequestCompletion ? CreateConnectTimeoutException(oce) : e; - // If the connection hasn't been initialized with QuicConnection there's no need to dispose it. + + // If the connection hasn't been initialized with QuicConnection, get rid of it. + connection?.Dispose(); connection = null; } finally @@ -441,33 +443,14 @@ private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnect } } - if (isNewConnection) + // Since we only inject one connection at a time, we may want to inject another now. + lock (SyncObj) { - Debug.Assert(initialRequestWaiter is not null, "Expect request for a new connection"); - - // The new connection could not handle even one request, either because it shut down before we could use it for any requests, - // or because it immediately set the max concurrent streams limit to 0. - // We don't want to get stuck in a loop where we keep trying to create new connections for the same request. - // So, treat this as a connection failure. - - if (NetEventSource.Log.IsEnabled()) connection.Trace("New HTTP3 connection is unusable due to no available streams."); - connection.Dispose(); - - HttpRequestException hre = new HttpRequestException(SR.net_http_http3_connection_not_established); - ExceptionDispatchInfo.SetCurrentStackTrace(hre); - HandleHttp3ConnectionFailure(initialRequestWaiter, hre); + CheckForHttp3ConnectionInjection(); } - else - { - // Since we only inject one connection at a time, we may want to inject another now. - lock (SyncObj) - { - CheckForHttp3ConnectionInjection(); - } - // We need to wait until the connection is usable again. - DisableHttp3Connection(connection); - } + // We need to wait until the connection is usable again. + DisableHttp3Connection(connection); } /// diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index be0173b10b675..785c0f8442c3b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -202,6 +202,9 @@ public void ReleaseStream() if (NetEventSource.Log.IsEnabled()) Trace($"ReleaseStream: _availableRequestStreamsCount = {_availableRequestStreamsCount}"); ++_availableRequestStreamsCount; + + _availableStreamsWaiter?.SetResult(!ShuttingDown); + _availableStreamsWaiter = null; } } From a316b892b4898155709abf68608cb9cb2b79282d Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 17 Jun 2024 16:39:56 +0200 Subject: [PATCH 16/28] Renamed API according to review feedback --- .../System.Net.Quic/ref/System.Net.Quic.cs | 8 +++-- .../src/System/Net/Quic/QuicConnection.cs | 33 +++++++------------ .../System/Net/Quic/QuicConnectionOptions.cs | 17 +++++++++- .../FunctionalTests/QuicConnectionTests.cs | 6 ++-- 4 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs index a0987763d1070..7098adae27694 100644 --- a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs +++ b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs @@ -47,9 +47,8 @@ internal QuicConnectionOptions() { } public System.TimeSpan KeepAliveInterval { get { throw null; } set { } } public int MaxInboundBidirectionalStreams { get { throw null; } set { } } public int MaxInboundUnidirectionalStreams { get { throw null; } set { } } - public System.Net.Quic.QuicConnectionStreamsAvailableCallback? StreamsAvailableCallback { get { throw null; } set { } } + public System.Action? StreamCapacityCallback { get { throw null; } set { } } } - public delegate void QuicConnectionStreamsAvailableCallback(System.Net.Quic.QuicConnection connection, int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement); public enum QuicError { Success = 0, @@ -143,6 +142,11 @@ public override void Write(System.ReadOnlySpan buffer) { } public override System.Threading.Tasks.ValueTask WriteAsync(System.ReadOnlyMemory buffer, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public override void WriteByte(byte value) { } } + public readonly partial struct QuicStreamCapacityChangedArgs + { + public int BidirectionalIncrement { get { throw null; } init { } } + public int UnidirectionalIncrement { get { throw null; } init { } } + } public enum QuicStreamType { Unidirectional = 0, diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index e6ecd315b3fed..5596dc15773aa 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -182,7 +182,7 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// /// Occurres when an additional stream capacity has been released by the peer. Corresponds to receiving MAX_STREAMS frame. /// - private QuicConnectionStreamsAvailableCallback? _streamsAvailableCallback; + private Action? _streamsCapacityCallback; /// /// Represents how many bidirectional streams can be accepted by the peer. Is only manipulated from MsQuic thread. /// @@ -220,10 +220,10 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// public IPEndPoint LocalEndPoint => _localEndPoint; - private async void OnStreamsAvailable(int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement) + private async void OnStreamsCapacityIncreased(int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement) { // Bail out early to avoid queueing work on the thread pool as well as event args instantiation. - if (_streamsAvailableCallback is null) + if (_streamsCapacityCallback is null) { return; } @@ -235,16 +235,16 @@ private async void OnStreamsAvailable(int bidirectionalStreamsCountIncrement, in { if (NetEventSource.Log.IsEnabled()) { - NetEventSource.Info(this, $"{this} Signaling StreamsAvailable with {bidirectionalStreamsCountIncrement} bidirectional increment (absolute value {_availableBidirectionalStreamsCount}) and {unidirectionalStreamsCountIncrement} unidirectional increment (absolute value {_availableUnidirectionalStreamsCount})."); + NetEventSource.Info(this, $"{this} Signaling StreamsCapacityIncreased with {bidirectionalStreamsCountIncrement} bidirectional increment (absolute value {_availableBidirectionalStreamsCount}) and {unidirectionalStreamsCountIncrement} unidirectional increment (absolute value {_availableUnidirectionalStreamsCount})."); } - _streamsAvailableCallback(this, bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); + _streamsCapacityCallback(this, new QuicStreamCapacityChangedArgs { BidirectionalIncrement = bidirectionalStreamsCountIncrement, UnidirectionalIncrement = unidirectionalStreamsCountIncrement }); } catch (Exception ex) { // Just log the exception, we're on a thread-pool thread and there's no way to report this to anyone. if (NetEventSource.Log.IsEnabled()) { - NetEventSource.Info(this, $"{this} {nameof(QuicConnectionOptions.StreamsAvailableCallback)} failed with {ex}."); + NetEventSource.Info(this, $"{this} {nameof(QuicConnectionOptions.StreamCapacityCallback)} failed with {ex}."); } } } @@ -342,7 +342,7 @@ private async ValueTask FinishConnectAsync(QuicClientConnectionOptions options, _canAccept = options.MaxInboundBidirectionalStreams > 0 || options.MaxInboundUnidirectionalStreams > 0; _defaultStreamErrorCode = options.DefaultStreamErrorCode; _defaultCloseErrorCode = options.DefaultCloseErrorCode; - _streamsAvailableCallback = options.StreamsAvailableCallback; + _streamsCapacityCallback = options.StreamCapacityCallback; if (!options.RemoteEndPoint.TryParse(out string? host, out IPAddress? address, out int port)) { @@ -419,7 +419,7 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str _canAccept = options.MaxInboundBidirectionalStreams > 0 || options.MaxInboundUnidirectionalStreams > 0; _defaultStreamErrorCode = options.DefaultStreamErrorCode; _defaultCloseErrorCode = options.DefaultCloseErrorCode; - _streamsAvailableCallback = options.StreamsAvailableCallback; + _streamsCapacityCallback = options.StreamCapacityCallback; // RFC 6066 forbids IP literals, avoid setting IP address here for consistency with SslStream if (TargetHostNameHelper.IsValidAddress(targetHost)) @@ -450,7 +450,7 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str } /// - /// In order to provide meaningful increments in , available streams count can be only manipulated from MsQuic thread. + /// In order to provide meaningful increments in , available streams count can be only manipulated from MsQuic thread. /// For that purpose we pass this function to so that it can call it from START_COMPLETE event handler. /// /// Note that MsQuic itself manipulates stream counts right before indicating START_COMPLETE event. @@ -681,14 +681,14 @@ private unsafe int HandleEventStreamsAvailable(ref STREAMS_AVAILABLE_DATA data) int unidirectionalStreamsCountIncrement = data.UnidirectionalCount - _availableUnidirectionalStreamsCount; _availableBidirectionalStreamsCount = data.BidirectionalCount; _availableUnidirectionalStreamsCount = data.UnidirectionalCount; - OnStreamsAvailable(bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); + OnStreamsCapacityIncreased(bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); return QUIC_STATUS_SUCCESS; } private unsafe int HandleEventPeerCertificateReceived(ref PEER_CERTIFICATE_RECEIVED_DATA data) { // // The certificate validation is an expensive operation and we don't want to delay MsQuic - // worker thread. So we offload the validation to the .NET threadpool. Incidentally, this + // worker thread. So we offload the validation to the .NET thread pool. Incidentally, this // also prevents potential user RemoteCertificateValidationCallback from blocking MsQuic // worker threads. // @@ -816,14 +816,3 @@ public async ValueTask DisposeAsync() } } } - -/// -/// Callback that is invoked when new stream limit is released by the peer. Corresponds to receiving MAX_STREAMS frame. -/// The callback values represent increments of stream limits, e.g.: current limit is 10 bidirectional streams, callback arguments notify 5 more additional bidirectional streams => 15 bidirectional streams can be opened in total at the moment. -/// The initial capacity is reported with the first invocation of the callback that might happen before the instance is handed out via either -/// or . -/// -/// The that received the new stream limits. -/// The increment saying how many additional bidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. -/// The increment saying how many additional unidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. -public delegate void QuicConnectionStreamsAvailableCallback(QuicConnection connection, int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement); diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs index ab9b749eeb873..9378f54457953 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs @@ -50,6 +50,21 @@ static void ValidatePowerOf2(string argumentName, int value, [CallerArgumentExpr } } +/// +/// Arguments for . +/// +public readonly struct QuicStreamCapacityChangedArgs +{ + /// + /// The increment saying how many additional bidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. + /// + public int BidirectionalIncrement { get; init; } + /// + /// The increment saying how many additional unidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. + /// + public int UnidirectionalIncrement { get; init; } +} + /// /// Shared options for both client (outbound) and server (inbound) . /// @@ -130,7 +145,7 @@ public QuicReceiveWindowSizes InitialReceiveWindowSizes /// The initial capacity is reported with the first invocation of the callback that might happen before the instance is handed out via either /// or . /// - public QuicConnectionStreamsAvailableCallback? StreamsAvailableCallback { get; set; } + public Action? StreamCapacityCallback { get; set; } /// /// Validates the options and potentially sets platform specific defaults. diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs index 5f6d79175aa9f..ca344233208fc 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs @@ -195,10 +195,10 @@ public async Task GetAvailableStreamsCount_OpenCloseStream_CountsCorrectly() int bidiIncrement = -1, unidiIncrement = -1; var clientOptions = CreateQuicClientOptions(new IPEndPoint(0, 0)); - clientOptions.StreamsAvailableCallback = (sender, bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement) => + clientOptions.StreamCapacityCallback = (connection, args) => { - bidiIncrement = bidirectionalStreamsCountIncrement; - unidiIncrement = unidirectionalStreamsCountIncrement; + bidiIncrement = args.BidirectionalIncrement; + unidiIncrement = args.UnidirectionalIncrement; streamsAvailableFired.Release(); }; From 2c3a47db5b6b644fdeec2d2f1a54213a649baf00 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 17 Jun 2024 16:49:54 +0200 Subject: [PATCH 17/28] Renamed API according to review feedback --- .../src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs | 4 ++-- .../ConnectionPool/HttpConnectionPool.Http3.cs | 2 +- .../System/Net/Http/SocketsHttpHandler/Http3Connection.cs | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs index 603196aea0512..7da2758b5d07c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs @@ -112,7 +112,7 @@ public static async ValueTask EstablishSslConnectionAsync(SslClientAu [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] - public static async ValueTask ConnectQuicAsync(HttpRequestMessage request, DnsEndPoint endPoint, TimeSpan idleTimeout, SslClientAuthenticationOptions clientAuthenticationOptions, QuicConnectionStreamsAvailableCallback streamsAvailableCallback, CancellationToken cancellationToken) + public static async ValueTask ConnectQuicAsync(HttpRequestMessage request, DnsEndPoint endPoint, TimeSpan idleTimeout, SslClientAuthenticationOptions clientAuthenticationOptions, Action streamCapacityCallback, CancellationToken cancellationToken) { clientAuthenticationOptions = SetUpRemoteCertificateValidationCallback(clientAuthenticationOptions, request); @@ -127,7 +127,7 @@ public static async ValueTask ConnectQuicAsync(HttpRequestMessag DefaultCloseErrorCode = (long)Http3ErrorCode.NoError, RemoteEndPoint = endPoint, ClientAuthenticationOptions = clientAuthenticationOptions, - StreamsAvailableCallback = streamsAvailableCallback, + StreamCapacityCallback = streamCapacityCallback, }, cancellationToken).ConfigureAwait(false); } catch (Exception ex) when (ex is not OperationCanceledException) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index 93ea60136d797..2462be6ef82b5 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -252,7 +252,7 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue. // If the authority was sent as an option through alt-svc then include alt-used header. connection = new Http3Connection(this, authority, includeAltUsedHeader: _http3Authority == authority); - QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(queueItem.Request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, connection.StreamsAvailableCallback, cts.Token).ConfigureAwait(false); + QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(queueItem.Request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, connection.StreamCapacityCallback, cts.Token).ConfigureAwait(false); if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) { await quicConnection.DisposeAsync().ConfigureAwait(false); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 785c0f8442c3b..81acedd2594ac 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -208,7 +208,7 @@ public void ReleaseStream() } } - public void StreamsAvailableCallback(QuicConnection connection, int bidirectionalStreamsCountIncrement, int _) + public void StreamCapacityCallback(QuicConnection connection, QuicStreamCapacityChangedArgs args) { Debug.Assert(_connection is null || connection == _connection); @@ -216,9 +216,9 @@ public void StreamsAvailableCallback(QuicConnection connection, int bidirectiona { Debug.Assert(_availableRequestStreamsCount >= 0); - if (NetEventSource.Log.IsEnabled()) Trace($"StreamsAvailableCallback: _availableRequestStreamsCount = {_availableRequestStreamsCount} + bidirectionalStreamsCountIncrement = {bidirectionalStreamsCountIncrement}"); + if (NetEventSource.Log.IsEnabled()) Trace($"StreamCapacityCallback: _availableRequestStreamsCount = {_availableRequestStreamsCount} + bidirectionalStreamsCountIncrement = {args.BidirectionalIncrement}"); - _availableRequestStreamsCount += bidirectionalStreamsCountIncrement; + _availableRequestStreamsCount += args.BidirectionalIncrement; _availableStreamsWaiter?.SetResult(!ShuttingDown); _availableStreamsWaiter = null; } From 0dff95bb07bd224e2ca12e6336aa549fe9ec566e Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 17 Jun 2024 17:14:17 +0200 Subject: [PATCH 18/28] More renames; Stored Action callback feedback --- .../src/System/Net/Quic/QuicConnection.cs | 49 +++++++++++-------- .../src/System/Net/Quic/QuicStream.cs | 14 +++--- .../FunctionalTests/QuicConnectionTests.cs | 2 +- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index 5596dc15773aa..f40792dde9d72 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -182,15 +182,20 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// /// Occurres when an additional stream capacity has been released by the peer. Corresponds to receiving MAX_STREAMS frame. /// - private Action? _streamsCapacityCallback; + private Action? _streamCapacityCallback; + /// + /// Optimization to avoid `Action` instantiation with every . + /// Holds method. + /// + private Action _decrementStreamCapacity; /// /// Represents how many bidirectional streams can be accepted by the peer. Is only manipulated from MsQuic thread. /// - private int _availableBidirectionalStreamsCount; + private int _bidirectionalStreamCapacity; /// /// Represents how many unidirectional streams can be accepted by the peer. Is only manipulated from MsQuic thread. /// - private int _availableUnidirectionalStreamsCount; + private int _unidirectionalStreamCapacity; /// /// Keeps track whether has been accessed so that we know whether to dispose the certificate or not. /// @@ -220,10 +225,10 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// public IPEndPoint LocalEndPoint => _localEndPoint; - private async void OnStreamsCapacityIncreased(int bidirectionalStreamsCountIncrement, int unidirectionalStreamsCountIncrement) + private async void OnStreamCapacityIncreased(int bidirectionalIncrement, int unidirectionalIncrement) { // Bail out early to avoid queueing work on the thread pool as well as event args instantiation. - if (_streamsCapacityCallback is null) + if (_streamCapacityCallback is null) { return; } @@ -235,9 +240,9 @@ private async void OnStreamsCapacityIncreased(int bidirectionalStreamsCountIncre { if (NetEventSource.Log.IsEnabled()) { - NetEventSource.Info(this, $"{this} Signaling StreamsCapacityIncreased with {bidirectionalStreamsCountIncrement} bidirectional increment (absolute value {_availableBidirectionalStreamsCount}) and {unidirectionalStreamsCountIncrement} unidirectional increment (absolute value {_availableUnidirectionalStreamsCount})."); + NetEventSource.Info(this, $"{this} Signaling StreamCapacityIncreased with {bidirectionalIncrement} bidirectional increment (absolute value {_bidirectionalStreamCapacity}) and {unidirectionalIncrement} unidirectional increment (absolute value {_unidirectionalStreamCapacity})."); } - _streamsCapacityCallback(this, new QuicStreamCapacityChangedArgs { BidirectionalIncrement = bidirectionalStreamsCountIncrement, UnidirectionalIncrement = unidirectionalStreamsCountIncrement }); + _streamCapacityCallback(this, new QuicStreamCapacityChangedArgs { BidirectionalIncrement = bidirectionalIncrement, UnidirectionalIncrement = unidirectionalIncrement }); } catch (Exception ex) { @@ -304,6 +309,7 @@ private unsafe QuicConnection() NetEventSource.Info(this, $"{this} New outbound connection."); } + _decrementStreamCapacity = DecrementStreamCapacity; _tlsSecret = MsQuicTlsSecret.Create(_handle); } @@ -332,6 +338,7 @@ internal unsafe QuicConnection(QUIC_HANDLE* handle, QUIC_NEW_CONNECTION_INFO* in _remoteEndPoint = MsQuicHelpers.QuicAddrToIPEndPoint(info->RemoteAddress); _localEndPoint = MsQuicHelpers.QuicAddrToIPEndPoint(info->LocalAddress); + _decrementStreamCapacity = DecrementStreamCapacity; _tlsSecret = MsQuicTlsSecret.Create(_handle); } @@ -342,7 +349,7 @@ private async ValueTask FinishConnectAsync(QuicClientConnectionOptions options, _canAccept = options.MaxInboundBidirectionalStreams > 0 || options.MaxInboundUnidirectionalStreams > 0; _defaultStreamErrorCode = options.DefaultStreamErrorCode; _defaultCloseErrorCode = options.DefaultCloseErrorCode; - _streamsCapacityCallback = options.StreamCapacityCallback; + _streamCapacityCallback = options.StreamCapacityCallback; if (!options.RemoteEndPoint.TryParse(out string? host, out IPAddress? address, out int port)) { @@ -419,7 +426,7 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str _canAccept = options.MaxInboundBidirectionalStreams > 0 || options.MaxInboundUnidirectionalStreams > 0; _defaultStreamErrorCode = options.DefaultStreamErrorCode; _defaultCloseErrorCode = options.DefaultCloseErrorCode; - _streamsCapacityCallback = options.StreamCapacityCallback; + _streamCapacityCallback = options.StreamCapacityCallback; // RFC 6066 forbids IP literals, avoid setting IP address here for consistency with SslStream if (TargetHostNameHelper.IsValidAddress(targetHost)) @@ -450,28 +457,28 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str } /// - /// In order to provide meaningful increments in , available streams count can be only manipulated from MsQuic thread. + /// In order to provide meaningful increments in , available streams count can be only manipulated from MsQuic thread. /// For that purpose we pass this function to so that it can call it from START_COMPLETE event handler. /// /// Note that MsQuic itself manipulates stream counts right before indicating START_COMPLETE event. /// /// Type of the stream to decrement appropriate field. - private void DecrementAvailableStreamCount(QuicStreamType streamType) + private void DecrementStreamCapacity(QuicStreamType streamType) { if (streamType == QuicStreamType.Unidirectional) { - --_availableUnidirectionalStreamsCount; + --_unidirectionalStreamCapacity; if (NetEventSource.Log.IsEnabled()) { - NetEventSource.Info(this, $"{this} decremented stream count for {streamType} to {_availableUnidirectionalStreamsCount}."); + NetEventSource.Info(this, $"{this} decremented stream count for {streamType} to {_unidirectionalStreamCapacity}."); } } else { - --_availableBidirectionalStreamsCount; + --_bidirectionalStreamCapacity; if (NetEventSource.Log.IsEnabled()) { - NetEventSource.Info(this, $"{this} decremented stream count for {streamType} to {_availableBidirectionalStreamsCount}."); + NetEventSource.Info(this, $"{this} decremented stream count for {streamType} to {_bidirectionalStreamCapacity}."); } } } @@ -498,7 +505,7 @@ public async ValueTask OpenOutboundStreamAsync(QuicStreamType type, NetEventSource.Info(this, $"{this} New outbound {type} stream {stream}."); } - await stream.StartAsync(DecrementAvailableStreamCount, cancellationToken).ConfigureAwait(false); + await stream.StartAsync(DecrementStreamCapacity, cancellationToken).ConfigureAwait(false); } catch (Exception ex) { @@ -677,11 +684,11 @@ private unsafe int HandleEventPeerStreamStarted(ref PEER_STREAM_STARTED_DATA dat } private unsafe int HandleEventStreamsAvailable(ref STREAMS_AVAILABLE_DATA data) { - int bidirectionalStreamsCountIncrement = data.BidirectionalCount - _availableBidirectionalStreamsCount; - int unidirectionalStreamsCountIncrement = data.UnidirectionalCount - _availableUnidirectionalStreamsCount; - _availableBidirectionalStreamsCount = data.BidirectionalCount; - _availableUnidirectionalStreamsCount = data.UnidirectionalCount; - OnStreamsCapacityIncreased(bidirectionalStreamsCountIncrement, unidirectionalStreamsCountIncrement); + int bidirectionalIncrement = data.BidirectionalCount - _bidirectionalStreamCapacity; + int unidirectionalIncrement = data.UnidirectionalCount - _unidirectionalStreamCapacity; + _bidirectionalStreamCapacity = data.BidirectionalCount; + _unidirectionalStreamCapacity = data.UnidirectionalCount; + OnStreamCapacityIncreased(bidirectionalIncrement, unidirectionalIncrement); return QUIC_STATUS_SUCCESS; } private unsafe int HandleEventPeerCertificateReceived(ref PEER_CERTIFICATE_RECEIVED_DATA data) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs index 5dbcbb7bfecd2..71ba2dbfcabd5 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs @@ -124,7 +124,7 @@ public sealed partial class QuicStream /// Provided via from so that can decrement its available stream count field. /// When START_COMPLETE arrives it gets invoked and unset back to null to not to hold any unintended reference to . /// - private Action? _decrementAvailableStreamCount; + private Action? _decrementStreamCapacity; /// /// Stream id, see . @@ -245,16 +245,16 @@ internal unsafe QuicStream(MsQuicContextSafeHandle connectionHandle, QUIC_HANDLE /// If no more concurrent streams can be opened at the moment, the operation will wait until it can, /// either by closing some existing streams or receiving more available stream ids from the peer. /// - /// + /// /// A cancellation token that can be used to cancel the asynchronous operation. /// An asynchronous task that completes with the opened . - internal ValueTask StartAsync(Action decrementAvailableStreamCount, CancellationToken cancellationToken = default) + internal ValueTask StartAsync(Action decrementStreamCapacity, CancellationToken cancellationToken = default) { Debug.Assert(!_startedTcs.IsCompleted); // Always call StreamStart to get consistent behavior (events, stream count, frames send to peer) regardless of cancellation. _startedTcs.TryInitialize(out ValueTask valueTask, this, cancellationToken); - _decrementAvailableStreamCount = decrementAvailableStreamCount; + _decrementStreamCapacity = decrementStreamCapacity; unsafe { int status = MsQuicApi.Api.StreamStart( @@ -534,12 +534,12 @@ public void CompleteWrites() private unsafe int HandleEventStartComplete(ref START_COMPLETE_DATA data) { - Debug.Assert(_decrementAvailableStreamCount is not null); + Debug.Assert(_decrementStreamCapacity is not null); _id = unchecked((long)data.ID); if (StatusSucceeded(data.Status)) { - _decrementAvailableStreamCount(Type); + _decrementStreamCapacity(Type); if (data.PeerAccepted != 0) { @@ -555,7 +555,7 @@ private unsafe int HandleEventStartComplete(ref START_COMPLETE_DATA data) } } - _decrementAvailableStreamCount = null; + _decrementStreamCapacity = null; return QUIC_STATUS_SUCCESS; } private unsafe int HandleEventReceive(ref RECEIVE_DATA data) diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs index ca344233208fc..0e329eeaad0b5 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs @@ -189,7 +189,7 @@ await RunClientServer( } [Fact] - public async Task GetAvailableStreamsCount_OpenCloseStream_CountsCorrectly() + public async Task GetStreamCapacity_OpenCloseStream_CountsCorrectly() { SemaphoreSlim streamsAvailableFired = new SemaphoreSlim(0); int bidiIncrement = -1, unidiIncrement = -1; From ff47d4b527cc182d35354aeb47fa615fbb825692 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 17 Jun 2024 18:04:54 +0200 Subject: [PATCH 19/28] Fixed borked platform guard --- .../Http/SocketsHttpHandler/ConnectHelper.cs | 2 +- .../HttpConnectionPool.Http3.cs | 52 +++++++++---------- .../SocketsHttpHandler/Http3Connection.cs | 2 +- .../SocketsHttpHandler/Http3RequestStream.cs | 2 +- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs index 7da2758b5d07c..ef6c5db24f33f 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs @@ -109,9 +109,9 @@ public static async ValueTask EstablishSslConnectionAsync(SslClientAu return sslStream; } - [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] + [SupportedOSPlatform("windows")] public static async ValueTask ConnectQuicAsync(HttpRequestMessage request, DnsEndPoint endPoint, TimeSpan idleTimeout, SslClientAuthenticationOptions clientAuthenticationOptions, Action streamCapacityCallback, CancellationToken cancellationToken) { clientAuthenticationOptions = SetUpRemoteCertificateValidationCallback(clientAuthenticationOptions, request); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index 2462be6ef82b5..25ce474273a8f 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -27,7 +27,7 @@ internal sealed partial class HttpConnectionPool [SupportedOSPlatformGuard("linux")] [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatformGuard("windows")] internal static bool IsHttp3Supported() => (OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); /// List of available HTTP/3 connections stored in the pool. @@ -62,9 +62,9 @@ internal sealed partial class HttpConnectionPool private bool EnableMultipleHttp3Connections => _poolManager.Settings.EnableMultipleHttp3Connections; // Returns null if HTTP3 cannot be used. - [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] + [SupportedOSPlatform("windows")] private async ValueTask TrySendUsingHttp3Async(HttpRequestMessage request, CancellationToken cancellationToken) { Debug.Assert(IsHttp3Supported()); @@ -114,9 +114,9 @@ internal sealed partial class HttpConnectionPool } } - [SupportedOSPlatformGuard("linux")] - [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macOS")] + [SupportedOSPlatform("windows")] private bool TryGetPooledHttp3Connection(HttpRequestMessage request, [NotNullWhen(true)] out Http3Connection? connection, [NotNullWhen(false)] out HttpConnectionWaiter? waiter) { Debug.Assert(IsHttp3Supported()); @@ -186,9 +186,9 @@ private bool TryGetPooledHttp3Connection(HttpRequestMessage request, [NotNullWhe } } - [SupportedOSPlatformGuard("linux")] - [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macOS")] + [SupportedOSPlatform("windows")] private void CheckForHttp3ConnectionInjection() { Debug.Assert(IsHttp3Supported()); @@ -225,9 +225,9 @@ private void CheckForHttp3ConnectionInjection() } } - [SupportedOSPlatformGuard("linux")] - [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macOS")] + [SupportedOSPlatform("windows")] private async Task InjectNewHttp3ConnectionAsync(RequestQueue.QueueItem queueItem) { Debug.Assert(IsHttp3Supported()); @@ -302,9 +302,9 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue. } } - [SupportedOSPlatformGuard("linux")] - [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macOS")] + [SupportedOSPlatform("windows")] private void HandleHttp3ConnectionFailure(HttpConnectionWaiter requestWaiter, Exception? e) { Debug.Assert(IsHttp3Supported()); @@ -333,9 +333,9 @@ private void HandleHttp3ConnectionFailure(HttpConnectionWaiter } } - [SupportedOSPlatformGuard("linux")] - [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macOS")] + [SupportedOSPlatform("windows")] private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnection, HttpConnectionWaiter? initialRequestWaiter = null) { Debug.Assert(IsHttp3Supported()); @@ -457,9 +457,9 @@ private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnect /// Disable usage of the specified connection because it cannot handle any more streams at the moment. /// We will register to be notified when it can handle more streams (or becomes permanently unusable). /// - [SupportedOSPlatformGuard("linux")] - [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macOS")] + [SupportedOSPlatform("windows")] private void DisableHttp3Connection(Http3Connection connection) { Debug.Assert(IsHttp3Supported()); @@ -500,9 +500,9 @@ async Task DisableHttp3ConnectionAsync(Http3Connection connection) /// /// Called when an Http3Connection from this pool is no longer usable. /// - [SupportedOSPlatformGuard("linux")] - [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macOS")] + [SupportedOSPlatform("windows")] public void InvalidateHttp3Connection(Http3Connection connection) { Debug.Assert(IsHttp3Supported()); @@ -536,9 +536,9 @@ public void InvalidateHttp3Connection(Http3Connection connection) } } - [SupportedOSPlatformGuard("linux")] - [SupportedOSPlatformGuard("macOS")] - [SupportedOSPlatformGuard("Windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macOS")] + [SupportedOSPlatform("windows")] private static int ScavengeHttp3ConnectionList(List list, ref List? toDispose, long nowTicks, TimeSpan pooledConnectionLifetime, TimeSpan pooledConnectionIdleTimeout) { Debug.Assert(IsHttp3Supported()); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 81acedd2594ac..6e7ebe682b4d6 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -14,9 +14,9 @@ namespace System.Net.Http { - [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] + [SupportedOSPlatform("windows")] internal sealed class Http3Connection : HttpConnectionBase { private readonly HttpAuthority _authority; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index ef2532b2b22d0..9e4c967c58de0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -17,9 +17,9 @@ namespace System.Net.Http { - [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] + [SupportedOSPlatform("windows")] internal sealed class Http3RequestStream : IHttpStreamHeadersHandler, IAsyncDisposable, IDisposable { private readonly HttpRequestMessage _request; From e830ecb596ef1f575f90b03b9b1945b34ba32a44 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 17 Jun 2024 18:10:23 +0200 Subject: [PATCH 20/28] Use the cached delegate :facepalm: --- .../System.Net.Quic/src/System/Net/Quic/QuicConnection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index f40792dde9d72..edebed18f0d7a 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -505,7 +505,7 @@ public async ValueTask OpenOutboundStreamAsync(QuicStreamType type, NetEventSource.Info(this, $"{this} New outbound {type} stream {stream}."); } - await stream.StartAsync(DecrementStreamCapacity, cancellationToken).ConfigureAwait(false); + await stream.StartAsync(_decrementStreamCapacity, cancellationToken).ConfigureAwait(false); } catch (Exception ex) { From 2721f0f33bebab233220a741fc205935fbf85e0b Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 18 Jun 2024 17:20:53 +0200 Subject: [PATCH 21/28] Easy feedback --- .../System/Net/Http/SocketsHttpHandler/Http3Connection.cs | 6 +++--- .../src/System/Net/Quic/Internal/MsQuicExtensions.cs | 2 ++ .../System.Net.Quic/src/System/Net/Quic/QuicConnection.cs | 2 +- .../src/System/Net/Quic/QuicConnectionOptions.cs | 6 +++--- .../System.Net.Quic/src/System/Net/Quic/QuicStream.cs | 1 + 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 6e7ebe682b4d6..e8318e1d297da 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -178,7 +178,7 @@ public bool TryReserveStream() { Debug.Assert(_availableRequestStreamsCount >= 0); - if (NetEventSource.Log.IsEnabled()) Trace($"TryReserveStream: _availableRequestStreamsCount = {_availableRequestStreamsCount}"); + if (NetEventSource.Log.IsEnabled()) Trace($"_availableRequestStreamsCount = {_availableRequestStreamsCount}"); if (_availableRequestStreamsCount == 0) { @@ -200,7 +200,7 @@ public void ReleaseStream() { Debug.Assert(_availableRequestStreamsCount >= 0); - if (NetEventSource.Log.IsEnabled()) Trace($"ReleaseStream: _availableRequestStreamsCount = {_availableRequestStreamsCount}"); + if (NetEventSource.Log.IsEnabled()) Trace($"_availableRequestStreamsCount = {_availableRequestStreamsCount}"); ++_availableRequestStreamsCount; _availableStreamsWaiter?.SetResult(!ShuttingDown); @@ -216,7 +216,7 @@ public void StreamCapacityCallback(QuicConnection connection, QuicStreamCapacity { Debug.Assert(_availableRequestStreamsCount >= 0); - if (NetEventSource.Log.IsEnabled()) Trace($"StreamCapacityCallback: _availableRequestStreamsCount = {_availableRequestStreamsCount} + bidirectionalStreamsCountIncrement = {args.BidirectionalIncrement}"); + if (NetEventSource.Log.IsEnabled()) Trace($"_availableRequestStreamsCount = {_availableRequestStreamsCount} + bidirectionalStreamsCountIncrement = {args.BidirectionalIncrement}"); _availableRequestStreamsCount += args.BidirectionalIncrement; _availableStreamsWaiter?.SetResult(!ShuttingDown); diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicExtensions.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicExtensions.cs index ba8148784b19a..cfc5a09a37171 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicExtensions.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicExtensions.cs @@ -54,6 +54,8 @@ public override string ToString() => $"{{ {nameof(DATAGRAM_RECEIVED.Flags)} = {DATAGRAM_RECEIVED.Flags} }}", QUIC_CONNECTION_EVENT_TYPE.DATAGRAM_SEND_STATE_CHANGED => $"{{ {nameof(DATAGRAM_SEND_STATE_CHANGED.ClientContext)} = 0x{(IntPtr)DATAGRAM_SEND_STATE_CHANGED.ClientContext:X11}, {nameof(DATAGRAM_SEND_STATE_CHANGED.State)} = {DATAGRAM_SEND_STATE_CHANGED.State} }}", + QUIC_CONNECTION_EVENT_TYPE.RESUMED + => $"{{ {nameof(RESUMED.ResumptionStateLength)} = {RESUMED.ResumptionStateLength} }}", QUIC_CONNECTION_EVENT_TYPE.RESUMPTION_TICKET_RECEIVED => $"{{ {nameof(RESUMPTION_TICKET_RECEIVED.ResumptionTicketLength)} = {RESUMPTION_TICKET_RECEIVED.ResumptionTicketLength} }}", QUIC_CONNECTION_EVENT_TYPE.PEER_CERTIFICATE_RECEIVED diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index edebed18f0d7a..ef28e544fc94b 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -180,7 +180,7 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// private IPEndPoint _localEndPoint = null!; /// - /// Occurres when an additional stream capacity has been released by the peer. Corresponds to receiving MAX_STREAMS frame. + /// Occurres when an additional stream capacity has been released by the peer. Corresponds to receiving a MAX_STREAMS frame. /// private Action? _streamCapacityCallback; /// diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs index 9378f54457953..a007f64fd5b6f 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnectionOptions.cs @@ -56,11 +56,11 @@ static void ValidatePowerOf2(string argumentName, int value, [CallerArgumentExpr public readonly struct QuicStreamCapacityChangedArgs { /// - /// The increment saying how many additional bidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. + /// The increment saying how many additional bidirectional streams can be opened on the connection, increased via the latest MAX_STREAMS frame. /// public int BidirectionalIncrement { get; init; } /// - /// The increment saying how many additional unidirectional streams can be opened on the connection, increased via the latest STREAMS_AVAILABLE frame. + /// The increment saying how many additional unidirectional streams can be opened on the connection, increased via the latest MAX_STREAMS frame. /// public int UnidirectionalIncrement { get; init; } } @@ -140,7 +140,7 @@ public QuicReceiveWindowSizes InitialReceiveWindowSizes public TimeSpan HandshakeTimeout { get; set; } = QuicDefaults.HandshakeTimeout; /// - /// Optional callback that is invoked when new stream limit is released by the peer. Corresponds to receiving MAX_STREAMS frame. + /// Optional callback that is invoked when new stream limit is released by the peer. Corresponds to receiving a MAX_STREAMS frame. /// The callback values represent increments of stream limits, e.g.: current limit is 10 bidirectional streams, callback arguments notify 5 more additional bidirectional streams => 15 bidirectional streams can be opened in total at the moment. /// The initial capacity is reported with the first invocation of the callback that might happen before the instance is handed out via either /// or . diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs index 71ba2dbfcabd5..c2469482c130c 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs @@ -263,6 +263,7 @@ internal ValueTask StartAsync(Action decrementStreamCapacity, Ca if (ThrowHelper.TryGetStreamExceptionForMsQuicStatus(status, out Exception? exception, streamWasSuccessfullyStarted: false)) { + _decrementStreamCapacity = null; _startedTcs.TrySetException(exception); } } From 114e89a92d9424d84de30fff20f0f02d7c0a4b49 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 18 Jun 2024 17:31:35 +0200 Subject: [PATCH 22/28] Generated ref source --- src/libraries/System.Net.Quic/ref/System.Net.Quic.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs index 7098adae27694..21ffbdf461b88 100644 --- a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs +++ b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs @@ -144,6 +144,8 @@ public override void WriteByte(byte value) { } } public readonly partial struct QuicStreamCapacityChangedArgs { + private readonly object _dummy; + private readonly int _dummyPrimitive; public int BidirectionalIncrement { get { throw null; } init { } } public int UnidirectionalIncrement { get { throw null; } init { } } } From 01e75dcf56ac85e18f32aa4959f97dcc47ca00a7 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Wed, 19 Jun 2024 10:52:58 +0200 Subject: [PATCH 23/28] SupportedOS ordering, because I cannot stand it different --- .../HttpConnectionPool.Http3.cs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index 25ce474273a8f..b2d17af903c61 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -62,9 +62,9 @@ internal sealed partial class HttpConnectionPool private bool EnableMultipleHttp3Connections => _poolManager.Settings.EnableMultipleHttp3Connections; // Returns null if HTTP3 cannot be used. + [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] - [SupportedOSPlatform("windows")] private async ValueTask TrySendUsingHttp3Async(HttpRequestMessage request, CancellationToken cancellationToken) { Debug.Assert(IsHttp3Supported()); @@ -114,9 +114,9 @@ internal sealed partial class HttpConnectionPool } } - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macOS")] [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] private bool TryGetPooledHttp3Connection(HttpRequestMessage request, [NotNullWhen(true)] out Http3Connection? connection, [NotNullWhen(false)] out HttpConnectionWaiter? waiter) { Debug.Assert(IsHttp3Supported()); @@ -186,9 +186,9 @@ private bool TryGetPooledHttp3Connection(HttpRequestMessage request, [NotNullWhe } } - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macOS")] [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] private void CheckForHttp3ConnectionInjection() { Debug.Assert(IsHttp3Supported()); @@ -225,9 +225,9 @@ private void CheckForHttp3ConnectionInjection() } } - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macOS")] [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] private async Task InjectNewHttp3ConnectionAsync(RequestQueue.QueueItem queueItem) { Debug.Assert(IsHttp3Supported()); @@ -302,9 +302,9 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue. } } - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macOS")] [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] private void HandleHttp3ConnectionFailure(HttpConnectionWaiter requestWaiter, Exception? e) { Debug.Assert(IsHttp3Supported()); @@ -333,9 +333,9 @@ private void HandleHttp3ConnectionFailure(HttpConnectionWaiter } } - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macOS")] [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnection, HttpConnectionWaiter? initialRequestWaiter = null) { Debug.Assert(IsHttp3Supported()); @@ -457,9 +457,9 @@ private void ReturnHttp3Connection(Http3Connection connection, bool isNewConnect /// Disable usage of the specified connection because it cannot handle any more streams at the moment. /// We will register to be notified when it can handle more streams (or becomes permanently unusable). /// - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macOS")] [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] private void DisableHttp3Connection(Http3Connection connection) { Debug.Assert(IsHttp3Supported()); @@ -500,9 +500,9 @@ async Task DisableHttp3ConnectionAsync(Http3Connection connection) /// /// Called when an Http3Connection from this pool is no longer usable. /// - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macOS")] [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] public void InvalidateHttp3Connection(Http3Connection connection) { Debug.Assert(IsHttp3Supported()); @@ -536,9 +536,9 @@ public void InvalidateHttp3Connection(Http3Connection connection) } } - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macOS")] [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] private static int ScavengeHttp3ConnectionList(List list, ref List? toDispose, long nowTicks, TimeSpan pooledConnectionLifetime, TimeSpan pooledConnectionIdleTimeout) { Debug.Assert(IsHttp3Supported()); From fd285fe039899dc7ad65d96f32b71e3c7f73028c Mon Sep 17 00:00:00 2001 From: ManickaP Date: Wed, 19 Jun 2024 10:54:21 +0200 Subject: [PATCH 24/28] Idle connection back where it was --- .../System/Net/Http/SocketsHttpHandler/Http3Connection.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index e8318e1d297da..5b6442a62a9b5 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -185,10 +185,6 @@ public bool TryReserveStream() return false; } - if (_activeRequests.Count == 0) - { - MarkConnectionAsNotIdle(); - } --_availableRequestStreamsCount; return true; } @@ -262,6 +258,10 @@ public async Task SendAsync(HttpRequestMessage request, lon requestStream = new Http3RequestStream(request, this, quicStream); lock (SyncObj) { + if (_activeRequests.Count == 0) + { + MarkConnectionAsNotIdle(); + } _activeRequests.Add(quicStream, requestStream); } } From 7f6e3c404a15fe88e0d633930ab3945393847d1b Mon Sep 17 00:00:00 2001 From: ManickaP Date: Wed, 19 Jun 2024 10:56:10 +0200 Subject: [PATCH 25/28] Assert --- .../src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 5b6442a62a9b5..c00071ac3897c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -235,6 +235,7 @@ public Task WaitForAvailableStreamsAsync() return Task.FromResult(true); } + Debug.Assert(_availableStreamsWaiter is null); _availableStreamsWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); return _availableStreamsWaiter.Task; } From 2710d18adc116e2662baa2593508ee787ad6a9d1 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Wed, 19 Jun 2024 16:38:00 +0200 Subject: [PATCH 26/28] Fix handling negative stream capacity and 0 reporting STREAMS_AVAILABLE --- .../src/System/Net/Quic/QuicConnection.cs | 9 ++- .../FunctionalTests/QuicConnectionTests.cs | 57 +++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index ef28e544fc94b..fcc9efacf40cb 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -232,6 +232,11 @@ private async void OnStreamCapacityIncreased(int bidirectionalIncrement, int uni { return; } + // No increment, nothing to report. + if (bidirectionalIncrement == 0 && unidirectionalIncrement == 0) + { + return; + } // Do not invoke user-defined event handler code on MsQuic thread. await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding); @@ -465,7 +470,7 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str /// Type of the stream to decrement appropriate field. private void DecrementStreamCapacity(QuicStreamType streamType) { - if (streamType == QuicStreamType.Unidirectional) + if (streamType == QuicStreamType.Unidirectional && _unidirectionalStreamCapacity > 0) { --_unidirectionalStreamCapacity; if (NetEventSource.Log.IsEnabled()) @@ -473,7 +478,7 @@ private void DecrementStreamCapacity(QuicStreamType streamType) NetEventSource.Info(this, $"{this} decremented stream count for {streamType} to {_unidirectionalStreamCapacity}."); } } - else + if (streamType == QuicStreamType.Bidirectional && _bidirectionalStreamCapacity > 0) { --_bidirectionalStreamCapacity; if (NetEventSource.Log.IsEnabled()) diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs index 0e329eeaad0b5..6e91f4c0af97e 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs @@ -2,11 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Linq; using System.Net.Sockets; using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.XUnitExtensions; +using TestUtilities; using Xunit; using Xunit.Abstractions; @@ -228,6 +230,61 @@ public async Task GetStreamCapacity_OpenCloseStream_CountsCorrectly() Assert.Equal(1, unidiIncrement); } + [Fact] + public async Task GetStreamCapacity_OpenCloseStreamIntoNegative_CountsCorrectly() + { + //using var _ = new TestEventListener(_output, TestEventListener.NetworkingEvents); + SemaphoreSlim streamsAvailableFired = new SemaphoreSlim(0); + int bidiIncrement = -1, unidiIncrement = -1; + + var clientOptions = CreateQuicClientOptions(new IPEndPoint(0, 0)); + clientOptions.StreamCapacityCallback = (connection, args) => + { + bidiIncrement = args.BidirectionalIncrement; + unidiIncrement = args.UnidirectionalIncrement; + streamsAvailableFired.Release(); + }; + + (QuicConnection clientConnection, QuicConnection serverConnection) = await CreateConnectedQuicConnection(clientOptions); + await streamsAvailableFired.WaitAsync(); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, bidiIncrement); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, unidiIncrement); + + List clientStreams = (await Task.WhenAll(Enumerable.Range(0, QuicDefaults.DefaultServerMaxInboundBidirectionalStreams) + .Select(i => clientConnection.OpenOutboundStreamAsync(QuicStreamType.Bidirectional).AsTask()))) + .ToList(); + List> pendingClientStreams = Enumerable.Range(0, QuicDefaults.DefaultServerMaxInboundBidirectionalStreams) + .Select(i => clientConnection.OpenOutboundStreamAsync(QuicStreamType.Bidirectional).AsTask()) + .ToList(); + foreach (var task in pendingClientStreams) + { + Assert.False(task.IsCompleted); + } + Assert.False(streamsAvailableFired.CurrentCount > 0); + + // Dispose streams to release capacity. + foreach (var clientStream in clientStreams) + { + await clientStream.DisposeAsync(); + await (await serverConnection.AcceptInboundStreamAsync()).DisposeAsync(); + } + clientStreams.Clear(); + + // Pending streams should get opened. + Assert.False(streamsAvailableFired.CurrentCount > 0); + clientStreams.AddRange(await Task.WhenAll(pendingClientStreams)); + + // Disposing the streams now should lead to stream capacity increments. + foreach (var clientStream in clientStreams) + { + await clientStream.DisposeAsync(); + await (await serverConnection.AcceptInboundStreamAsync()).DisposeAsync(); + await streamsAvailableFired.WaitAsync(); + Assert.Equal(1, bidiIncrement); + Assert.Equal(0, unidiIncrement); + } + } + [Fact] public async Task ConnectionClosedByPeer_WithPendingAcceptAndConnect_PendingAndSubsequentThrowConnectionAbortedException() { From 346a2788c997714f8e6a0ef152cabbb690dd94b8 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 25 Jun 2024 09:16:10 +0200 Subject: [PATCH 27/28] Handling cummulative increments, canceled streams and more tests. --- .../src/System/Net/Quic/QuicConnection.cs | 20 +- .../FunctionalTests/QuicConnectionTests.cs | 185 ++++++++++++++++-- 2 files changed, 184 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index fcc9efacf40cb..f1cd5e7c3fe28 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -470,7 +470,7 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str /// Type of the stream to decrement appropriate field. private void DecrementStreamCapacity(QuicStreamType streamType) { - if (streamType == QuicStreamType.Unidirectional && _unidirectionalStreamCapacity > 0) + if (streamType == QuicStreamType.Unidirectional) { --_unidirectionalStreamCapacity; if (NetEventSource.Log.IsEnabled()) @@ -478,7 +478,7 @@ private void DecrementStreamCapacity(QuicStreamType streamType) NetEventSource.Info(this, $"{this} decremented stream count for {streamType} to {_unidirectionalStreamCapacity}."); } } - if (streamType == QuicStreamType.Bidirectional && _bidirectionalStreamCapacity > 0) + if (streamType == QuicStreamType.Bidirectional) { --_bidirectionalStreamCapacity; if (NetEventSource.Log.IsEnabled()) @@ -689,10 +689,18 @@ private unsafe int HandleEventPeerStreamStarted(ref PEER_STREAM_STARTED_DATA dat } private unsafe int HandleEventStreamsAvailable(ref STREAMS_AVAILABLE_DATA data) { - int bidirectionalIncrement = data.BidirectionalCount - _bidirectionalStreamCapacity; - int unidirectionalIncrement = data.UnidirectionalCount - _unidirectionalStreamCapacity; - _bidirectionalStreamCapacity = data.BidirectionalCount; - _unidirectionalStreamCapacity = data.UnidirectionalCount; + int bidirectionalIncrement = 0; + int unidirectionalIncrement = 0; + if (data.BidirectionalCount > 0) + { + bidirectionalIncrement = data.BidirectionalCount - _bidirectionalStreamCapacity; + _bidirectionalStreamCapacity = data.BidirectionalCount; + } + if (data.UnidirectionalCount > 0) + { + unidirectionalIncrement = data.UnidirectionalCount - _unidirectionalStreamCapacity; + _unidirectionalStreamCapacity = data.UnidirectionalCount; + } OnStreamCapacityIncreased(bidirectionalIncrement, unidirectionalIncrement); return QUIC_STATUS_SUCCESS; } diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs index 6e91f4c0af97e..397c7285d1a92 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs @@ -230,18 +230,25 @@ public async Task GetStreamCapacity_OpenCloseStream_CountsCorrectly() Assert.Equal(1, unidiIncrement); } - [Fact] - public async Task GetStreamCapacity_OpenCloseStreamIntoNegative_CountsCorrectly() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task GetStreamCapacity_OpenCloseStreamIntoNegative_CountsCorrectly(bool unidirectional) { - //using var _ = new TestEventListener(_output, TestEventListener.NetworkingEvents); + using var _ = new TestEventListener(Console.Out, TestEventListener.NetworkingEvents); + SemaphoreSlim streamsAvailableFired = new SemaphoreSlim(0); int bidiIncrement = -1, unidiIncrement = -1; + int bidiTotal = 0; + int unidiTotal = 0; var clientOptions = CreateQuicClientOptions(new IPEndPoint(0, 0)); clientOptions.StreamCapacityCallback = (connection, args) => { - bidiIncrement = args.BidirectionalIncrement; - unidiIncrement = args.UnidirectionalIncrement; + Interlocked.Exchange(ref bidiIncrement, args.BidirectionalIncrement); + Interlocked.Exchange(ref unidiIncrement, args.UnidirectionalIncrement); + Interlocked.Add(ref bidiTotal, args.BidirectionalIncrement); + Interlocked.Add(ref unidiTotal, args.UnidirectionalIncrement); streamsAvailableFired.Release(); }; @@ -249,12 +256,17 @@ public async Task GetStreamCapacity_OpenCloseStreamIntoNegative_CountsCorrectly( await streamsAvailableFired.WaitAsync(); Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, bidiIncrement); Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, unidiIncrement); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, bidiTotal); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, unidiTotal); - List clientStreams = (await Task.WhenAll(Enumerable.Range(0, QuicDefaults.DefaultServerMaxInboundBidirectionalStreams) - .Select(i => clientConnection.OpenOutboundStreamAsync(QuicStreamType.Bidirectional).AsTask()))) + // Open # of streams up to the capacity. + List clientStreams = (await Task.WhenAll(Enumerable.Range(0, unidirectional ? QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams : QuicDefaults.DefaultServerMaxInboundBidirectionalStreams) + .Select(i => clientConnection.OpenOutboundStreamAsync(unidirectional ? QuicStreamType.Unidirectional : QuicStreamType.Bidirectional).AsTask()))) .ToList(); - List> pendingClientStreams = Enumerable.Range(0, QuicDefaults.DefaultServerMaxInboundBidirectionalStreams) - .Select(i => clientConnection.OpenOutboundStreamAsync(QuicStreamType.Bidirectional).AsTask()) + // Open another # of streams up to 2x capacity all together. + CancellationTokenSource cts = new CancellationTokenSource(); + List> pendingClientStreams = Enumerable.Range(0, unidirectional ? QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams : QuicDefaults.DefaultServerMaxInboundBidirectionalStreams) + .Select(i => clientConnection.OpenOutboundStreamAsync(unidirectional ? QuicStreamType.Unidirectional : QuicStreamType.Bidirectional, cts.Token).AsTask()) .ToList(); foreach (var task in pendingClientStreams) { @@ -262,27 +274,170 @@ public async Task GetStreamCapacity_OpenCloseStreamIntoNegative_CountsCorrectly( } Assert.False(streamsAvailableFired.CurrentCount > 0); - // Dispose streams to release capacity. + // Dispose streams to release capacity up to 0 (nothing gets reported yet). foreach (var clientStream in clientStreams) { await clientStream.DisposeAsync(); await (await serverConnection.AcceptInboundStreamAsync()).DisposeAsync(); } clientStreams.Clear(); - - // Pending streams should get opened. Assert.False(streamsAvailableFired.CurrentCount > 0); + + // All the pending streams should get accepted now. clientStreams.AddRange(await Task.WhenAll(pendingClientStreams)); - // Disposing the streams now should lead to stream capacity increments. + // Disposing the pending streams now should lead to stream capacity increments. + bool first = true; // The stream capacity is cumulatively reported only after the STREAMS_AVAILABLE reached over 0. + foreach (var clientStream in clientStreams) + { + await clientStream.DisposeAsync(); + await (await serverConnection.AcceptInboundStreamAsync()).DisposeAsync(); + await streamsAvailableFired.WaitAsync(); + Assert.Equal(unidirectional ? 0 : (first ? QuicDefaults.DefaultServerMaxInboundBidirectionalStreams + 1 : 1), bidiIncrement); + Assert.Equal(unidirectional ? (first ? QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams + 1 : 1) : 0, unidiIncrement); + first = false; + } + Assert.False(streamsAvailableFired.CurrentCount > 0); + Assert.Equal(unidirectional ? QuicDefaults.DefaultServerMaxInboundBidirectionalStreams : QuicDefaults.DefaultServerMaxInboundBidirectionalStreams * 3, bidiTotal); + Assert.Equal(unidirectional ? QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams * 3 : QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, unidiTotal); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task GetStreamCapacity_OpenCloseStreamCanceledIntoNegative_CountsCorrectly(bool unidirectional) + { + using var _ = new TestEventListener(Console.Out, TestEventListener.NetworkingEvents); + + SemaphoreSlim streamsAvailableFired = new SemaphoreSlim(0); + int bidiIncrement = -1, unidiIncrement = -1; + int bidiTotal = 0; + int unidiTotal = 0; + + var clientOptions = CreateQuicClientOptions(new IPEndPoint(0, 0)); + clientOptions.StreamCapacityCallback = (connection, args) => + { + Interlocked.Exchange(ref bidiIncrement, args.BidirectionalIncrement); + Interlocked.Exchange(ref unidiIncrement, args.UnidirectionalIncrement); + Interlocked.Add(ref bidiTotal, args.BidirectionalIncrement); + Interlocked.Add(ref unidiTotal, args.UnidirectionalIncrement); + streamsAvailableFired.Release(); + }; + + (QuicConnection clientConnection, QuicConnection serverConnection) = await CreateConnectedQuicConnection(clientOptions); + await streamsAvailableFired.WaitAsync(); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, bidiIncrement); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, unidiIncrement); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundBidirectionalStreams, bidiTotal); + Assert.Equal(QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, unidiTotal); + + // Open # of streams up to the capacity. + List clientStreams = (await Task.WhenAll(Enumerable.Range(0, unidirectional ? QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams : QuicDefaults.DefaultServerMaxInboundBidirectionalStreams) + .Select(i => clientConnection.OpenOutboundStreamAsync(unidirectional ? QuicStreamType.Unidirectional : QuicStreamType.Bidirectional).AsTask()))) + .ToList(); + // Open another # of streams up to 2x capacity all together. + CancellationTokenSource cts = new CancellationTokenSource(); + List> pendingClientStreams = Enumerable.Range(0, unidirectional ? QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams : QuicDefaults.DefaultServerMaxInboundBidirectionalStreams) + .Select(i => clientConnection.OpenOutboundStreamAsync(unidirectional ? QuicStreamType.Unidirectional : QuicStreamType.Bidirectional, cts.Token).AsTask()) + .ToList(); + foreach (var task in pendingClientStreams) + { + Assert.False(task.IsCompleted); + } + Assert.False(streamsAvailableFired.CurrentCount > 0); + + // Cancel pending streams if requested. + cts.Cancel(); + + // Dispose streams to release capacity up to 0 (nothing gets reported yet). foreach (var clientStream in clientStreams) { await clientStream.DisposeAsync(); await (await serverConnection.AcceptInboundStreamAsync()).DisposeAsync(); + } + clientStreams.Clear(); + Assert.False(streamsAvailableFired.CurrentCount > 0); + + // Pending streams should get cancelled and disposing the streams now should lead to stream capacity increments. + bool first = true; // The stream capacity is cumulatively reported only after the STREAMS_AVAILABLE reached over 0. + foreach (var cancelledStream in pendingClientStreams) + { + Assert.True(cancelledStream.IsCanceled); + await (await serverConnection.AcceptInboundStreamAsync()).DisposeAsync(); await streamsAvailableFired.WaitAsync(); - Assert.Equal(1, bidiIncrement); - Assert.Equal(0, unidiIncrement); + Assert.Equal(unidirectional ? 0 : (first ? QuicDefaults.DefaultServerMaxInboundBidirectionalStreams + 1 : 1), bidiIncrement); + Assert.Equal(unidirectional ? (first ? QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams + 1 : 1) : 0, unidiIncrement); + first = false; } + Assert.False(streamsAvailableFired.CurrentCount > 0); + Assert.Equal(unidirectional ? QuicDefaults.DefaultServerMaxInboundBidirectionalStreams : QuicDefaults.DefaultServerMaxInboundBidirectionalStreams * 3, bidiTotal); + Assert.Equal(unidirectional ? QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams * 3 : QuicDefaults.DefaultServerMaxInboundUnidirectionalStreams, unidiTotal); + } + + [Fact] + public async Task GetStreamCapacity_SumInvariant() + { + int maxStreamIndex = 0; + const int Limit = 5; + + var clientOptions = CreateQuicClientOptions(new IPEndPoint(0, 0)); + clientOptions.StreamCapacityCallback = (connection, args) => + { + Interlocked.Add(ref maxStreamIndex, args.BidirectionalIncrement); + }; + + var listenerOptions = CreateQuicListenerOptions(); + listenerOptions.ConnectionOptionsCallback = (_, _, _) => + { + var options = CreateQuicServerOptions(); + options.MaxInboundBidirectionalStreams = Limit; + return ValueTask.FromResult(options); + }; + + (QuicConnection clientConnection, QuicConnection serverConnection) = await CreateConnectedQuicConnection(clientOptions, listenerOptions); + + Assert.Equal(Limit, maxStreamIndex); + + Queue<(QuicStream client, QuicStream server)> streams = new(); + + for (int i = 0; i < Limit; i++) + { + QuicStream clientStream = await clientConnection.OpenOutboundStreamAsync(QuicStreamType.Bidirectional); + await clientStream.WriteAsync(new byte[1]); + QuicStream serverStream = await serverConnection.AcceptInboundStreamAsync(); + streams.Enqueue((clientStream, serverStream)); + } + + Queue> tasks = new(); + // enqueue more stream creations + for (int i = 0; i < Limit; i++) + { + var newClientStreamTask = clientConnection.OpenOutboundStreamAsync(QuicStreamType.Bidirectional); + Assert.False(newClientStreamTask.IsCompleted, "Stream creation should not be completed synchronously"); + tasks.Enqueue(newClientStreamTask.AsTask()); + } + + // dispose streams + while (streams.Count > 0) + { + var (clientStream, serverStream) = streams.Dequeue(); + await clientStream.DisposeAsync(); + await serverStream.DisposeAsync(); + + if (tasks.TryDequeue(out var task)) + { + clientStream = await task; + await clientStream.WriteAsync(new byte[1]); + serverStream = await serverConnection.AcceptInboundStreamAsync(); + streams.Enqueue((clientStream, serverStream)); + } + } + + // give time to update the count + await Task.Delay(1000); + + // by now, we opened and closed 2 * Limit, and expect a budget of 'Limit' more + Assert.Equal(3 * Limit, maxStreamIndex); } [Fact] From ede84077a4c50db6e2c6db9ec6050eedeeb2d9c9 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 25 Jun 2024 10:41:37 +0200 Subject: [PATCH 28/28] Removed logging --- .../tests/FunctionalTests/QuicConnectionTests.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs index 397c7285d1a92..668711c68a2e8 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicConnectionTests.cs @@ -235,8 +235,6 @@ public async Task GetStreamCapacity_OpenCloseStream_CountsCorrectly() [InlineData(false)] public async Task GetStreamCapacity_OpenCloseStreamIntoNegative_CountsCorrectly(bool unidirectional) { - using var _ = new TestEventListener(Console.Out, TestEventListener.NetworkingEvents); - SemaphoreSlim streamsAvailableFired = new SemaphoreSlim(0); int bidiIncrement = -1, unidiIncrement = -1; int bidiTotal = 0; @@ -307,8 +305,6 @@ public async Task GetStreamCapacity_OpenCloseStreamIntoNegative_CountsCorrectly( [InlineData(false)] public async Task GetStreamCapacity_OpenCloseStreamCanceledIntoNegative_CountsCorrectly(bool unidirectional) { - using var _ = new TestEventListener(Console.Out, TestEventListener.NetworkingEvents); - SemaphoreSlim streamsAvailableFired = new SemaphoreSlim(0); int bidiIncrement = -1, unidiIncrement = -1; int bidiTotal = 0;