Merge pull request #3049 from dotnet-maestro-bot/merge/release/2.2-to-master

[automated] Merge branch 'release/2.2' => 'master'

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

@@ -104,6 +104,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
             }
         }
 
+        public Task FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken)
+        {
+            lock (_writeLock)
+            {
+                if (_completed)
+                {
+                    return Task.CompletedTask;
+                }
+
+                return _flusher.FlushAsync(outputAborter, cancellationToken);
+            }
+        }
+
         public Task Write100ContinueAsync(int streamId)
         {
             lock (_writeLock)
@@ -158,11 +171,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                     throw new InvalidOperationException(hex.Message, hex); // Report the error to the user if this was the first write.
                 }
             }
-
-            _ = _flusher.FlushAsync();
         }
 
-        public Task WriteResponseTrailersAsync(int streamId, HttpResponseTrailers headers)
+        public Task WriteResponseTrailers(int streamId, HttpResponseTrailers headers)
         {
             lock (_writeLock)
             {

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

@@ -28,6 +28,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         private readonly object _dataWriterLock = new object();
         private readonly Pipe _dataPipe;
         private readonly Task _dataWriteProcessingTask;
+        private bool _startedWritingDataFrames;
         private bool _completed;
         private bool _disposed;
 
@@ -100,7 +101,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                     return Task.CompletedTask;
                 }
 
-                return _flusher.FlushAsync(this, cancellationToken);
+                if (_startedWritingDataFrames)
+                {
+                    // If there's already been response data written to the stream, just wait for that. Any header
+                    // should be in front of the data frames in the connection pipe. Trailers could change things.
+                    return _flusher.FlushAsync(this, cancellationToken);
+                }
+                else
+                {
+                    // Flushing the connection pipe ensures headers already in the pipe are flushed even if no data
+                    // frames have been written.
+                    return _frameWriter.FlushAsync(this, cancellationToken);
+                }
             }
         }
 
@@ -148,6 +160,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                     return Task.CompletedTask;
                 }
 
+                _startedWritingDataFrames = true;
+
                 _dataPipe.Writer.Write(data);
                 return _flusher.FlushAsync(this, cancellationToken);
             }
@@ -198,7 +212,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                             await _frameWriter.WriteDataAsync(_streamId, _flowControl, _stream.MinResponseDataRate, readResult.Buffer, endStream: false);
                         }
 
-                        await _frameWriter.WriteResponseTrailersAsync(_streamId, _stream.Trailers);
+                        await _frameWriter.WriteResponseTrailers(_streamId, _stream.Trailers);
                     }
                     else
                     {

test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs

@@ -2083,54 +2083,5 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
             Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
         }
-
-        [Fact]
-        public async Task ResponseHeaders_NotBlockedByFlowControl()
-        {
-            var headers = new[]
-            {
-                new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
-                new KeyValuePair<string, string>(HeaderNames.Path, "/"),
-                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
-            };
-            await InitializeConnectionAsync(context =>
-            {
-                return context.Response.WriteAsync("hello world");
-            });
-
-            _clientSettings.InitialWindowSize = 0;
-            await SendSettingsAsync();
-            await ExpectAsync(Http2FrameType.SETTINGS,
-                withLength: 0,
-                withFlags: (byte)Http2SettingsFrameFlags.ACK,
-                withStreamId: 0);
-
-            await StartStreamAsync(1, headers, endStream: true);
-
-            var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
-                withLength: 37,
-                withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
-                withStreamId: 1);
-
-            await SendWindowUpdateAsync(1, 11);
-
-            await ExpectAsync(Http2FrameType.DATA,
-                withLength: 11,
-                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);
-
-            _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this);
-
-            Assert.Equal(2, _decodedHeaders.Count);
-            Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
-            Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
-        }
     }
 }