Always flush HTTP/2 headers despite flow control (#20831)

src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs

@@ -254,7 +254,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
             }
         }
 
-        public ValueTask<FlushResult> WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, in ReadOnlySequence<byte> data, bool endStream, bool forceFlush)
+        public ValueTask<FlushResult> WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, in ReadOnlySequence<byte> data, bool endStream, bool firstWrite, bool forceFlush)
         {
             // The Length property of a ReadOnlySequence can be expensive, so we cache the value.
             var dataLength = data.Length;
@@ -270,7 +270,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                 // https://httpwg.org/specs/rfc7540.html#rfc.section.6.9.1
                 if (dataLength != 0 && dataLength > flowControl.Available)
                 {
-                    return WriteDataAsync(streamId, flowControl, data, dataLength, endStream);
+                    return WriteDataAsync(streamId, flowControl, data, dataLength, endStream, firstWrite);
                 }
 
                 // This cast is safe since if dataLength would overflow an int, it's guaranteed to be greater than the available flow control window.
@@ -370,7 +370,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
             }
         }
 
-        private async ValueTask<FlushResult> WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence<byte> data, long dataLength, bool endStream)
+        private async ValueTask<FlushResult> WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence<byte> data, long dataLength, bool endStream, bool firstWrite)
         {
             FlushResult flushResult = default;
 
@@ -406,6 +406,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                         // flow control induced backpressure below.
                         writeTask = _flusher.FlushAsync();
                     }
+                    else if (firstWrite)
+                    {
+                        // If we're facing flow control induced backpressure on the first write for a given stream's response body,
+                        // we make sure to flush the response headers immediately.
+                        writeTask = _flusher.FlushAsync();
+                    }
+
+                    firstWrite = false;
 
                     if (_minResponseDataRate != null)
                     {

src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs

@@ -405,6 +405,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
                     do
                     {
+                        var firstWrite = true;
+
                         readResult = await _pipeReader.ReadAsync();
 
                         if (readResult.IsCanceled)
@@ -420,7 +422,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                             {
                                 // Only flush if required (i.e. content length exceeds flow control availability)
                                 // Writing remaining content without flushing allows content and trailers to be sent in the same packet
-                                await _frameWriter.WriteDataAsync(StreamId, _flowControl, readResult.Buffer, endStream: false, forceFlush: false);
+                                await _frameWriter.WriteDataAsync(StreamId, _flowControl, readResult.Buffer, endStream: false, firstWrite, forceFlush: false);
                             }
 
                             _stream.ResponseTrailers.SetReadOnly();
@@ -440,13 +442,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                         else
                         {
                             var endStream = readResult.IsCompleted;
+
                             if (endStream)
                             {
                                 _stream.DecrementActiveClientStreamCount();
                             }
-                            flushResult = await _frameWriter.WriteDataAsync(StreamId, _flowControl, readResult.Buffer, endStream, forceFlush: true);
+
+                            flushResult = await _frameWriter.WriteDataAsync(StreamId, _flowControl, readResult.Buffer, endStream, firstWrite, forceFlush: true);
                         }
 
+                        firstWrite = false;
                         _pipeReader.AdvanceTo(readResult.Buffer.End);
                     } while (!readResult.IsCompleted);
                 }

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs

@@ -28,6 +28,38 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 {
     public class Http2ConnectionTests : Http2TestBase
     {
+        [Fact]
+        public async Task FlowControl_NoAvailability_ResponseHeadersStillFlushed()
+        {
+            _clientSettings.InitialWindowSize = 0;
+
+            await InitializeConnectionAsync(c =>
+            {
+                return c.Response.Body.WriteAsync(new byte[1]).AsTask();
+            });
+
+            await StartStreamAsync(1, _browserRequestHeaders, endStream: true);
+
+            await ExpectAsync(Http2FrameType.HEADERS,
+                withLength: 32,
+                withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
+                withStreamId: 1);
+
+            await SendWindowUpdateAsync(streamId: 1, 1);
+
+            await ExpectAsync(Http2FrameType.DATA,
+                withLength: 1,
+                withFlags: (byte)Http2DataFrameFlags.NONE,
+                withStreamId: 1);
+
+            await ExpectAsync(Http2FrameType.DATA,
+                withLength: 0,
+                withFlags: (byte)Http2DataFrameFlags.END_STREAM,
+                withStreamId: 1);
+
+            await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
+        }
+
         [Fact]
         [QuarantinedTest]
         public async Task FlowControl_ParallelStreams_FirstInFirstOutOrder()
@@ -38,34 +70,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // 3. Update the connection window one byte at a time.
             // 4. Read from them in a FIFO order until they are each complete.
 
-            var writeTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
+            // Ensure there that all backpressure is due to connection flow control, not the pipe.
+            _serviceContext.ServerOptions.Limits.MaxResponseBufferSize = long.MaxValue;
+            // Ensure there that all backpressure is due to connection flow control, not stream flow control.
+            _clientSettings.InitialWindowSize = int.MaxValue;
 
-            await InitializeConnectionAsync(async c =>
+            await InitializeConnectionAsync(c =>
             {
-                // Send headers
-                await c.Response.Body.FlushAsync();
-
                 var responseBodySize = Convert.ToInt32(c.Request.Headers["ResponseBodySize"]);
-                var writeTask = c.Response.Body.WriteAsync(new byte[responseBodySize]);
+                return c.Response.Body.WriteAsync(new byte[responseBodySize]).AsTask();
-
-                // Notify test that write has started
-                writeTcs.SetResult(null);
-
-                // Wait for write to complete
-                await writeTask;
             });
 
+            // Consume the entire connection output flow control window with a large response.
             await StartStreamAsync(1, GetHeaders(responseBodySize: 65535), endStream: true);
-            // Ensure the stream window size is large enough
-            await SendWindowUpdateAsync(streamId: 1, 65535);
 
             await ExpectAsync(Http2FrameType.HEADERS,
                 withLength: 32,
                 withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
                 withStreamId: 1);
-
-            await writeTcs.Task;
-
             await ExpectAsync(Http2FrameType.DATA,
                 withLength: 16384,
                 withFlags: (byte)Http2DataFrameFlags.NONE,
@@ -87,8 +109,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 withFlags: (byte)Http2DataFrameFlags.END_STREAM,
                 withStreamId: 1);
 
-            writeTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
-
             await StartStreamAsync(3, GetHeaders(responseBodySize: 3), endStream: true);
 
             await ExpectAsync(Http2FrameType.HEADERS,
@@ -96,10 +116,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
                 withStreamId: 3);
 
-            await writeTcs.Task;
-
-            writeTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
-
             await StartStreamAsync(5, GetHeaders(responseBodySize: 3), endStream: true);
 
             await ExpectAsync(Http2FrameType.HEADERS,
@@ -107,8 +123,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
                 withStreamId: 5);
 
-            await writeTcs.Task;
-
             await SendWindowUpdateAsync(streamId: 0, 1);
 
             // FIFO means stream 3 returns data first