diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index ca428eae8a..f1556af04e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -254,7 +254,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - public ValueTask WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, in ReadOnlySequence data, bool endStream, bool forceFlush) + public ValueTask WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, in ReadOnlySequence 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 WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence data, long dataLength, bool endStream) + private async ValueTask WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence 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) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs index cc30a8ee66..c7c772f385 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs +++ b/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); } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index d5a40a1127..8212a68451 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/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(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]); - - // Notify test that write has started - writeTcs.SetResult(null); - - // Wait for write to complete - await writeTask; + return c.Response.Body.WriteAsync(new byte[responseBodySize]).AsTask(); }); + // 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(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(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