HTTP/3: Complete outbound control stream with connection (#33956)

Author: JamesNK

src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs

@@ -224,14 +224,21 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> appl
             // the maximum capacity of the dynamic table to zero.
 
             // Don't create Encoder and Decoder as they aren't used now.
-            Exception? error = null;
 
-            // Don't delay setting up the connection on the control stream being ready.
-            // Task is awaited when connection finishes.
-            var controlStreamTask = CreateControlStreamAsync(application);
+            Exception? error = null;
+            ValueTask outboundControlStreamTask = default;
 
             try
             {
+                var outboundControlStream = await CreateNewUnidirectionalStreamAsync(application);
+                lock (_sync)
+                {
+                    OutboundControlStream = outboundControlStream;
+                }
+
+                // Don't delay on waiting to send outbound control stream settings.
+                outboundControlStreamTask = ProcessOutboundControlStreamAsync(outboundControlStream);
+
                 while (_isClosed == 0)
                 {
                     // Don't pass a cancellation token to AcceptAsync.
@@ -341,23 +348,26 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> appl
                         stream.Abort(connectionError, (Http3ErrorCode)_errorCodeFeature.Error);
                     }
 
+                    lock (_sync)
+                    {
+                        OutboundControlStream?.Abort(connectionError, (Http3ErrorCode)_errorCodeFeature.Error);
+                    }
+
                     while (_activeRequestCount > 0)
                     {
                         await _streamCompletionAwaitable;
                     }
 
+                    await outboundControlStreamTask;
+
                     _context.TimeoutControl.CancelTimeout();
                     _context.TimeoutControl.StartDrainTimeout(Limits.MinResponseDataRate, Limits.MaxResponseBufferSize);
                 }
                 catch
                 {
-                    Abort(connectionError, Http3ErrorCode.NoError);
+                    Abort(connectionError, Http3ErrorCode.InternalError);
                     throw;
                 }
-
-                // Ensure control stream creation task finished. At this point the connection, including the control
-                // stream should be closed/aborted. Error handling inside method ensures await won't throw.
-                await controlStreamTask;
             }
         }
 
@@ -400,18 +410,12 @@ private void UpdateConnectionState()
             }
         }
 
-        private async ValueTask CreateControlStreamAsync<TContext>(IHttpApplication<TContext> application) where TContext : notnull
+        private async ValueTask ProcessOutboundControlStreamAsync(Http3ControlStream controlStream)
         {
             try
             {
-                var stream = await CreateNewUnidirectionalStreamAsync(application);
-                lock (_sync)
-                {
-                    OutboundControlStream = stream;
-                }
-
-                await stream.SendStreamIdAsync(id: 0);
-                await stream.SendSettingsFrameAsync();
+                await controlStream.SendStreamIdAsync(id: 0);
+                await controlStream.SendSettingsFrameAsync();
             }
             catch (Exception ex)
             {
@@ -449,15 +453,27 @@ private async ValueTask<Http3ControlStream> CreateNewUnidirectionalStreamAsync<T
             return new Http3ControlStream<TContext>(application, httpConnectionContext);
         }
 
-        private ValueTask<FlushResult> SendGoAway(long id)
+        private async ValueTask<FlushResult> SendGoAway(long id)
         {
+            Http3ControlStream? stream;
             lock (_sync)
             {
-                if (OutboundControlStream != null)
+                stream = OutboundControlStream;
+            }
+
+            if (stream != null)
+            {
+                try
+                {
+                    return await stream.SendGoAway(id);
+                }
+                catch
                 {
-                    return OutboundControlStream.SendGoAway(id);
+                    // The control stream may not be healthy.
+                    // Ignore error sending go away.
                 }
             }
+
             return default;
         }
 

src/Servers/Kestrel/Core/src/Internal/Http3/Http3ConnectionErrorException.cs

@@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
     internal class Http3ConnectionErrorException : Exception
     {
         public Http3ConnectionErrorException(string message, Http3ErrorCode errorCode)
-            : base($"HTTP/3 connection error ({errorCode}): {message}")
+            : base($"HTTP/3 connection error ({Http3Formatting.ToFormattedErrorCode(errorCode)}): {message}")
         {
             ErrorCode = errorCode;
         }

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs

@@ -160,7 +160,15 @@ await WaitForConnectionErrorAsync<Http3ConnectionErrorException>(
         [Fact]
         public async Task ControlStream_ServerToClient_ErrorInitializing_ConnectionError()
         {
-            OnCreateServerControlStream = () => throw new Exception();
+            OnCreateServerControlStream = () =>
+            {
+                var controlStream = new Http3ControlStream(this, StreamInitiator.Server);
+
+                // Make server connection error when trying to write to control stream.
+                controlStream.StreamContext.Transport.Output.Complete();
+
+                return controlStream;
+            };
 
             await InitializeConnectionAsync(_noopApplication);
 

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3TestBase.cs

@@ -187,13 +187,16 @@ internal async ValueTask<Http3ControlStream> GetInboundControlStream()
             if (_inboundControlStream == null)
             {
                 var reader = MultiplexedConnectionContext.ToClientAcceptQueue.Reader;
-                while (await reader.WaitToReadAsync())
+                while (await reader.WaitToReadAsync().DefaultTimeout())
                 {
                     while (reader.TryRead(out var stream))
                     {
                         _inboundControlStream = stream;
                         var streamId = await stream.TryReadStreamIdAsync();
-                        Debug.Assert(streamId == 0, "StreamId sent that was non-zero, which isn't handled by tests");
+
+                        // -1 means stream was completed.
+                        Debug.Assert(streamId == 0 || streamId == -1, "StreamId sent that was non-zero, which isn't handled by tests");
+
                         return _inboundControlStream;
                     }
                 }
@@ -231,6 +234,12 @@ internal async Task WaitForConnectionErrorAsync<TException>(bool ignoreNonGoAway
             }
 
             AssertConnectionError<TException>(expectedErrorCode, expectedErrorMessage);
+
+            // Verify HttpConnection.ProcessRequestsAsync has exited.
+            await _connectionTask.DefaultTimeout();
+
+            // Verify server-to-client control stream has completed.
+            await _inboundControlStream.ReceiveEndAsync();
         }
 
         internal void AssertConnectionError<TException>(Http3ErrorCode expectedErrorCode, params string[] expectedErrorMessage) where TException : Exception