Don't write empty data for Flush (#7415)

src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs

@@ -100,7 +100,53 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
         public ValueTask<FlushResult> FlushAsync(CancellationToken cancellationToken = default)
         {
-            return WriteAsync(Constants.EmptyData, cancellationToken);
+            lock (_contextLock)
+            {
+                if (_pipeWriterCompleted)
+                {
+                    return default;
+                }
+
+                if (_autoChunk)
+                {
+                    if (_advancedBytesForChunk > 0)
+                    {
+                        // If there is data that was chunked before flushing (ex someone did GetMemory->Advance->FlushAsync)
+                        // make sure to write whatever was advanced first
+                        return FlushAsyncChunked(this, cancellationToken);
+                    }
+                    else
+                    {
+                        // If there is an empty write, we still need to update the current chunk
+                        _currentChunkMemoryUpdated = false;
+                    }
+                }
+
+                var bytesWritten = _unflushedBytes;
+                _unflushedBytes = 0;
+
+                return _flusher.FlushAsync(_minResponseDataRateFeature.MinDataRate, bytesWritten, this, cancellationToken);
+            }
+
+            ValueTask<FlushResult> FlushAsyncChunked(Http1OutputProducer producer, CancellationToken token)
+            {
+                // Local function so in the common-path the stack space for BufferWriter isn't reservered and cleared when it isn't used.
+
+                Debug.Assert(!producer._pipeWriterCompleted);
+                Debug.Assert(producer._autoChunk && producer._advancedBytesForChunk > 0);
+
+                var writer = new BufferWriter<PipeWriter>(producer._pipeWriter);
+                producer.WriteCurrentMemoryToPipeWriter(ref writer);
+                writer.Commit();
+
+                var bytesWritten = producer._unflushedBytes + writer.BytesCommitted;
+                producer._unflushedBytes = 0;
+
+                // If there is an empty write, we still need to update the current chunk
+                _currentChunkMemoryUpdated = false;
+
+                return producer._flusher.FlushAsync(producer._minResponseDataRateFeature.MinDataRate, bytesWritten, producer, token);
+            }
         }
 
         public Memory<byte> GetMemory(int sizeHint = 0)
@@ -187,17 +233,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
                 // Make sure any memory used with GetMemory/Advance is written before the chunk
                 // passed in.
-                WriteCurrentMemoryToPipeWriter();
+                if (_advancedBytesForChunk > 0 || buffer.Length > 0)
+                {
+                    var writer = new BufferWriter<PipeWriter>(_pipeWriter);
+                    if (_advancedBytesForChunk > 0)
+                    {
+                        WriteCurrentMemoryToPipeWriter(ref writer);
+                    }
 
                     if (buffer.Length > 0)
                     {
-                    var writer = new BufferWriter<PipeWriter>(_pipeWriter);
 
                         writer.WriteBeginChunkBytes(buffer.Length);
                         writer.Write(buffer);
                         writer.WriteEndChunkBytes();
-                    writer.Commit();
+                    }
 
+                    writer.Commit();
                     _unflushedBytes += writer.BytesCommitted;
                 }
             }
@@ -297,23 +349,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                     return default;
                 }
 
+                var writer = new BufferWriter<PipeWriter>(_pipeWriter);
                 if (_autoChunk)
+                {
+                    if (_advancedBytesForChunk > 0)
                     {
                         // If there is data that was chunked before writing (ex someone did GetMemory->Advance->WriteAsync)
                         // make sure to write whatever was advanced first
-                    WriteCurrentMemoryToPipeWriter();
+                        WriteCurrentMemoryToPipeWriter(ref writer);
+                    }
+                    else
+                    {
+                        // If there is an empty write, we still need to update the current chunk
+                        _currentChunkMemoryUpdated = false;
+                    }
                 }
 
-                var writer = new BufferWriter<PipeWriter>(_pipeWriter);
                 if (buffer.Length > 0)
                 {
                     writer.Write(buffer);
-
-                    _unflushedBytes += buffer.Length;
                 }
                 writer.Commit();
 
-                var bytesWritten = _unflushedBytes;
+                var bytesWritten = _unflushedBytes + writer.BytesCommitted;
                 _unflushedBytes = 0;
 
                 return _flusher.FlushAsync(
@@ -342,7 +400,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             if (_advancedBytesForChunk >= memoryMaxLength - Math.Min(MemorySizeThreshold, sizeHint))
             {
                 // Chunk is completely written, commit it to the pipe so GetMemory will return a new chunk of memory.
-                WriteCurrentMemoryToPipeWriter();
+                var writer = new BufferWriter<PipeWriter>(_pipeWriter);
+                WriteCurrentMemoryToPipeWriter(ref writer);
+                writer.Commit();
+
+                _unflushedBytes += writer.BytesCommitted;
                 _currentChunkMemory = _pipeWriter.GetMemory(sizeHint);
                 _currentChunkMemoryUpdated = true;
             }
@@ -356,14 +418,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             return actualMemory;
         }
 
-        private void WriteCurrentMemoryToPipeWriter()
+        private void WriteCurrentMemoryToPipeWriter(ref BufferWriter<PipeWriter> writer)
         {
-            var writer = new BufferWriter<PipeWriter>(_pipeWriter);
-
             Debug.Assert(_advancedBytesForChunk <= _currentChunkMemory.Length);
+            Debug.Assert(_advancedBytesForChunk > 0);
 
-            if (_advancedBytesForChunk > 0)
-            {
             var bytesWritten = writer.WriteBeginChunkBytes(_advancedBytesForChunk);
 
             Debug.Assert(bytesWritten <= BeginChunkLengthMax);
@@ -378,13 +437,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
             writer.Advance(_advancedBytesForChunk);
             writer.WriteEndChunkBytes();
-                writer.Commit();
-                _advancedBytesForChunk = 0;
-                _unflushedBytes += writer.BytesCommitted;
-            }
 
-            // If there is an empty write, we still need to update the current chunk
+            _advancedBytesForChunk = 0;
-            _currentChunkMemoryUpdated = false;
         }
 
         private Memory<byte> GetFakeMemory(int sizeHint)

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs

@@ -19,14 +19,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 {
     public partial class HttpProtocol : IHttpRequestFeature,
                                         IHttpResponseFeature,
+                                        IResponseBodyPipeFeature,
                                         IHttpUpgradeFeature,
                                         IHttpConnectionFeature,
                                         IHttpRequestLifetimeFeature,
                                         IHttpRequestIdentifierFeature,
                                         IHttpBodyControlFeature,
                                         IHttpMaxRequestBodySizeFeature,
-                                        IHttpResponseStartFeature,
+                                        IHttpResponseStartFeature
-                                        IResponseBodyPipeFeature
     {
         // NOTE: When feature interfaces are added to or removed from this HttpProtocol class implementation,
         // then the list of `implementedFeatures` in the generated code project MUST also be updated.

src/Servers/Kestrel/Core/src/Internal/Infrastructure/Constants.cs

@@ -37,7 +37,5 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
         public const string ServerName = "Kestrel";
 
         public static readonly TimeSpan RequestBodyDrainTimeout = TimeSpan.FromSeconds(5);
-
-        public static readonly ArraySegment<byte> EmptyData = new ArraySegment<byte>(new byte[0]);
     }
 }