[release/7.0] HTTP/3: Improve handling of connection timeout (dotnet#…

…43820)
kordevelopers · Sep 9, 2022 · 64cacff · 64cacff
1 parent 0c90eea
commit 64cacff
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 2 deletions.
diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionContext.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionContext.cs
@@ -76,7 +76,7 @@ public override void Abort(ConnectionAbortedException abortReason)
         lock (_shutdownLock)
         {
             // Check if connection has already been already aborted.
-            if (_abortReason != null)
+            if (_abortReason != null || _closeTask != null)
             {
                 return;
             }
@@ -147,7 +147,7 @@ public override void Abort(ConnectionAbortedException abortReason)
         {
             lock (_shutdownLock)
             {
-                // This error should only happen when shutdown has been initiated by the server.
+                // OperationAborted should only happen when shutdown has been initiated by the server.
                 // If there is no abort reason and we have this error then the connection is in an
                 // unexpected state. Abort connection and throw reason error.
                 if (_abortReason == null)
@@ -158,6 +158,20 @@ public override void Abort(ConnectionAbortedException abortReason)
                 _abortReason!.Throw();
             }
         }
+        catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionTimeout)
+        {
+            lock (_shutdownLock)
+            {
+                // ConnectionTimeout can happen when the client app is shutdown without aborting the connection.
+                // For example, a console app makes a HTTP/3 request with HttpClient and then exits without disposing the client.
+                if (_abortReason == null)
+                {
+                    Abort(new ConnectionAbortedException("The connection timed out waiting for a response from the peer.", ex));
+                }
+
+                _abortReason!.Throw();
+            }
+        }
         catch (OperationCanceledException)
         {
             Debug.Assert(cancellationToken.IsCancellationRequested, "Error requires cancellation is requested.");

diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionContextTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionContextTests.cs
@@ -19,6 +19,30 @@ public class QuicConnectionContextTests : TestApplicationErrorLoggerLoggedTest
 {
     private static readonly byte[] TestData = Encoding.UTF8.GetBytes("Hello world");
 
+    [ConditionalFact]
+    [MsQuicSupported]
+    public async Task Abort_AbortAfterDispose_Ignored()
+    {
+        // Arrange
+        await using var connectionListener = await QuicTestHelpers.CreateConnectionListenerFactory(
+            LoggerFactory,
+            defaultCloseErrorCode: (long)Http3ErrorCode.RequestCancelled);
+
+        // Act
+        var acceptTask = connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout();
+
+        var options = QuicTestHelpers.CreateClientConnectionOptions(connectionListener.EndPoint);
+
+        await using var clientConnection = await QuicConnection.ConnectAsync(options);
+
+        await using var serverConnection = await acceptTask.DefaultTimeout();
+
+        await serverConnection.DisposeAsync();
+
+        // Assert
+        serverConnection.Abort(); // Doesn't throw ODE.
+    }
+
     [ConditionalFact]
     [MsQuicSupported]
     public async Task DisposeAsync_DisposeConnectionAfterAcceptingStream_DefaultCloseErrorCodeReported()