diff --git a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs index 492aa5880a5c5f..ae39fdc92e1373 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using System.Linq; using System.Net.Http.Functional.Tests; +using Xunit; namespace System.Net.Test.Common { @@ -32,10 +33,13 @@ internal sealed class Http3LoopbackConnection : GenericLoopbackConnection public const long H3_VERSION_FALLBACK = 0x110; private readonly QuicConnection _connection; + + // This is specifically request streams, not control streams private readonly Dictionary _openStreams = new Dictionary(); - private Http3LoopbackStream _controlStream; // Our outbound control stream private Http3LoopbackStream _currentStream; - private bool _closed; + + private Http3LoopbackStream _inboundControlStream; // Inbound control stream from client + private Http3LoopbackStream _outboundControlStream; // Our outbound control stream public Http3LoopbackConnection(QuicConnection connection) { @@ -44,23 +48,30 @@ public Http3LoopbackConnection(QuicConnection connection) public override void Dispose() { + // Close any remaining request streams (but NOT control streams, as these should not be closed while the connection is open) foreach (Http3LoopbackStream stream in _openStreams.Values) { stream.Dispose(); } - if (!_closed) - { - // CloseAsync(H3_INTERNAL_ERROR).GetAwaiter().GetResult(); - } - - //_connection.Dispose(); +// We don't dispose the connection currently, because this causes races when the server connection is closed before +// the client has received and handled all response data. +// See discussion in https://github.com/dotnet/runtime/pull/57223#discussion_r687447832 +#if false + // Dispose the connection + // If we already waited for graceful shutdown from the client, then the connection is already closed and this will simply release the handle. + // If not, then this will silently abort the connection. + _connection.Dispose(); + + // Dispose control streams so that we release their handles too. + _inboundControlStream?.Dispose(); + _outboundControlStream?.Dispose(); +#endif } public async Task CloseAsync(long errorCode) { await _connection.CloseAsync(errorCode).ConfigureAwait(false); - _closed = true; } public Http3LoopbackStream OpenUnidirectionalStream() @@ -96,23 +107,50 @@ public async Task AcceptStreamAsync() QuicStream quicStream = await _connection.AcceptStreamAsync().ConfigureAwait(false); var stream = new Http3LoopbackStream(quicStream); - _openStreams.Add(checked((int)quicStream.StreamId), stream); - _currentStream = stream; + if (quicStream.CanWrite) + { + _openStreams.Add(checked((int)quicStream.StreamId), stream); + _currentStream = stream; + } return stream; } + private async Task HandleControlStreamAsync(Http3LoopbackStream controlStream) + { + if (_inboundControlStream is not null) + { + throw new Exception("Received second control stream from client???"); + } + + long? streamType = await controlStream.ReadIntegerAsync(); + Assert.Equal(Http3LoopbackStream.ControlStream, streamType); + + List<(long settingId, long settingValue)> settings = await controlStream.ReadSettingsAsync(); + (long settingId, long settingValue) = Assert.Single(settings); + + Assert.Equal(Http3LoopbackStream.MaxHeaderListSize, settingId); + + _inboundControlStream = controlStream; + } + + // This will automatically handle the control stream, including validating its contents public async Task AcceptRequestStreamAsync() { Http3LoopbackStream stream; - do + while (true) { stream = await AcceptStreamAsync().ConfigureAwait(false); - } - while (!stream.CanWrite); // skip control stream. - return stream; + if (stream.CanWrite) + { + return stream; + } + + // Must be the control stream + await HandleControlStreamAsync(stream); + } } public async Task<(Http3LoopbackStream clientControlStream, Http3LoopbackStream requestStream)> AcceptControlAndRequestStreamAsync() @@ -141,9 +179,9 @@ public async Task AcceptRequestStreamAsync() public async Task EstablishControlStreamAsync() { - _controlStream = OpenUnidirectionalStream(); - await _controlStream.SendUnidirectionalStreamTypeAsync(Http3LoopbackStream.ControlStream); - await _controlStream.SendSettingsFrameAsync(); + _outboundControlStream = OpenUnidirectionalStream(); + await _outboundControlStream.SendUnidirectionalStreamTypeAsync(Http3LoopbackStream.ControlStream); + await _outboundControlStream.SendSettingsFrameAsync(); } public override async Task ReadRequestBodyAsync() @@ -185,7 +223,7 @@ public override async Task HandleRequestAsync(HttpStatusCode st // We are about to close the connection, after we send the response. // So, send a GOAWAY frame now so the client won't inadvertantly try to reuse the connection. - await _controlStream.SendGoAwayFrameAsync(stream.StreamId + 4); + await _outboundControlStream.SendGoAwayFrameAsync(stream.StreamId + 4); await stream.SendResponseAsync(statusCode, headers, content).ConfigureAwait(false); @@ -221,6 +259,10 @@ public async Task WaitForClientDisconnectAsync(bool refuseNewRequests = true) } } + // The client's control stream should throw QuicConnectionAbortedException, indicating that it was + // aborted because the connection was closed (and was not explicitly closed or aborted prior to the connection being closed) + await Assert.ThrowsAsync(async () => await _inboundControlStream.ReadFrameAsync()); + await CloseAsync(H3_NO_ERROR); } 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 dca12ee814794a..2e0047e6ce971f 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 @@ -113,12 +113,6 @@ private void CheckForShutdown() return; } - if (_clientControl != null) - { - _clientControl.Dispose(); - _clientControl = null; - } - if (_connection != null) { // Close the QuicConnection in the background. @@ -146,6 +140,13 @@ private void CheckForShutdown() { Trace($"{nameof(QuicConnection)} failed to dispose: {ex}"); } + + if (_clientControl != null) + { + _clientControl.Dispose(); + _clientControl = null; + } + }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); } } diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/Mock/MockConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/Mock/MockConnection.cs index c2594cf9df9d71..7487a958db91f9 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/Mock/MockConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/Mock/MockConnection.cs @@ -29,6 +29,20 @@ internal sealed class MockConnection : QuicConnectionProvider internal PeerStreamLimit? LocalStreamLimit => _isClient ? _state?._clientStreamLimit : _state?._serverStreamLimit; internal PeerStreamLimit? RemoteStreamLimit => _isClient ? _state?._serverStreamLimit : _state?._clientStreamLimit; + internal long? ConnectionError + { + get + { + long? errorCode = _isClient ? _state?._serverErrorCode : _state?._clientErrorCode; + if (errorCode == -1) + { + errorCode = null; + } + + return errorCode; + } + } + internal override X509Certificate? RemoteCertificate => null; // Constructor for outbound connections diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/Mock/MockStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/Mock/MockStream.cs index 1b58009a2fd355..726086e8ef2211 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/Mock/MockStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/Mock/MockStream.cs @@ -84,6 +84,11 @@ internal override async ValueTask ReadAsync(Memory buffer, Cancellati int bytesRead = await streamBuffer.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); if (bytesRead == 0) { + if (_connection.ConnectionError is long connectonError) + { + throw new QuicConnectionAbortedException(connectonError); + } + long errorCode = _isInitiator ? _streamState._inboundReadErrorCode : _streamState._outboundReadErrorCode; if (errorCode != 0) { @@ -135,6 +140,11 @@ internal override async ValueTask WriteAsync(ReadOnlyMemory buffer, bool e throw new NotSupportedException(); } + if (_connection.ConnectionError is long connectonError) + { + throw new QuicConnectionAbortedException(connectonError); + } + long errorCode = _isInitiator ? _streamState._inboundWriteErrorCode : _streamState._outboundWriteErrorCode; if (errorCode != 0) {