Made changes to FileBufferWriteStream (#21223)

* Made changes to FileBufferWriteStream
- Make the internal FileStream write only
- Make a new readable stream over the same file in DrainBufferAsync to copy data to the buffer.
- Added an overload to DrainBufferAsync into a PipeWriter and use this overload in the 2 formatters in MVC. This should reduce the amount of copying from the internal buffer and reduces pinning (since these buffers are already pinned)
- Improved formatter tests

Author: davidfowl

src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp.cs

@@ -77,6 +77,8 @@ protected override void Dispose(bool disposing) { }
         [System.Diagnostics.DebuggerStepThroughAttribute]
         public override System.Threading.Tasks.ValueTask DisposeAsync() { throw null; }
         [System.Diagnostics.DebuggerStepThroughAttribute]
+        public System.Threading.Tasks.Task DrainBufferAsync(System.IO.Pipelines.PipeWriter destination, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
+        [System.Diagnostics.DebuggerStepThroughAttribute]
         public System.Threading.Tasks.Task DrainBufferAsync(System.IO.Stream destination, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
         public override void Flush() { }
         public override System.Threading.Tasks.Task FlushAsync(System.Threading.CancellationToken cancellationToken) { throw null; }

src/Http/WebUtilities/src/FileBufferingWriteStream.cs

@@ -5,6 +5,8 @@
 using System.Buffers;
 using System.Diagnostics;
 using System.IO;
+using System.IO.Pipelines;
+using System.Security.Cryptography;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Internal;
@@ -184,9 +186,31 @@ public async Task DrainBufferAsync(Stream destination, CancellationToken cancell
             // unspooled content. Copy the FileStream content first when available.
             if (FileStream != null)
             {
-                FileStream.Position = 0;
-                await FileStream.CopyToAsync(destination, cancellationToken);
+                // We make a new stream for async reads from disk and async writes to the destination
+                await using var readStream = new FileStream(FileStream.Name, FileMode.Open, FileAccess.Read, FileShare.Delete | FileShare.ReadWrite, bufferSize: 1, useAsync: true);
 
+                await readStream.CopyToAsync(destination, cancellationToken);
+
+                // This is created with delete on close
+                await FileStream.DisposeAsync();
+                FileStream = null;
+            }
+
+            await PagedByteBuffer.MoveToAsync(destination, cancellationToken);
+        }
+
+        public async Task DrainBufferAsync(PipeWriter destination, CancellationToken cancellationToken = default)
+        {
+            // When not null, FileStream always has "older" spooled content. The PagedByteBuffer always has "newer"
+            // unspooled content. Copy the FileStream content first when available.
+            if (FileStream != null)
+            {
+                // We make a new stream for async reads from disk and async writes to the destination
+                await using var readStream = new FileStream(FileStream.Name, FileMode.Open, FileAccess.Read, FileShare.Delete | FileShare.ReadWrite, bufferSize: 1, useAsync: true);
+
+                await readStream.CopyToAsync(destination, cancellationToken);
+
+                // This is created with delete on close
                 await FileStream.DisposeAsync();
                 FileStream = null;
             }
@@ -227,10 +251,10 @@ private void EnsureFileStream()
                 FileStream = new FileStream(
                     tempFileName,
                     FileMode.Create,
-                    FileAccess.ReadWrite,
-                    FileShare.Delete,
+                    FileAccess.Write,
+                    FileShare.Delete | FileShare.ReadWrite,
                     bufferSize: 1,
-                    FileOptions.Asynchronous | FileOptions.SequentialScan | FileOptions.DeleteOnClose);
+                    FileOptions.SequentialScan | FileOptions.DeleteOnClose);
             }
         }
 

src/Http/WebUtilities/src/PagedByteBuffer.cs

@@ -5,6 +5,7 @@
 using System.Buffers;
 using System.Collections.Generic;
 using System.IO;
+using System.IO.Pipelines;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -84,6 +85,23 @@ public void MoveTo(Stream stream)
             ClearBuffers();
         }
 
+        public async Task MoveToAsync(PipeWriter writer, CancellationToken cancellationToken)
+        {
+            ThrowIfDisposed();
+
+            for (var i = 0; i < Pages.Count; i++)
+            {
+                var page = Pages[i];
+                var length = (i == Pages.Count - 1) ?
+                    _currentPageIndex :
+                    page.Length;
+
+                await writer.WriteAsync(page.AsMemory(0, length), cancellationToken);
+            }
+
+            ClearBuffers();
+        }
+
         public async Task MoveToAsync(Stream stream, CancellationToken cancellationToken)
         {
             ThrowIfDisposed();

src/Http/WebUtilities/test/FileBufferingWriteStreamTests.cs

@@ -7,6 +7,7 @@
 using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
+using Microsoft.Extensions.Configuration;
 using Xunit;
 
 namespace Microsoft.AspNetCore.WebUtilities
@@ -383,9 +384,9 @@ public void Dispose()
 
         private static byte[] ReadFileContent(FileStream fileStream)
         {
-            fileStream.Position = 0;
+            var fs = new FileStream(fileStream.Name, FileMode.Open, FileAccess.Read, FileShare.Delete | FileShare.ReadWrite);
             using var memoryStream = new MemoryStream();
-            fileStream.CopyTo(memoryStream);
+            fs.CopyTo(memoryStream);
 
             return memoryStream.ToArray();
         }

src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs

@@ -261,7 +261,7 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co
                 if (fileBufferingWriteStream != null)
                 {
                     response.ContentLength = fileBufferingWriteStream.Length;
-                    await fileBufferingWriteStream.DrainBufferAsync(response.Body);
+                    await fileBufferingWriteStream.DrainBufferAsync(response.BodyWriter);
                 }
             }
             finally

src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs

@@ -403,18 +403,20 @@ public void CanWriteResult_ReturnsExpectedValueForMediaType(
         }
 
         [Fact]
-        public async Task XmlSerializerOutputFormatterDoesntFlushOutputStream()
+        public async Task XmlSerializerOutputFormatterWritesContentLengthResponse()
         {
             // Arrange
             var sampleInput = new DummyClass { SampleInt = 10 };
             var formatter = new XmlSerializerOutputFormatter();
             var outputFormatterContext = GetOutputFormatterContext(sampleInput, sampleInput.GetType());
 
             var response = outputFormatterContext.HttpContext.Response;
-            response.Body = FlushReportingStream.GetThrowingStream();
+            response.Body = Stream.Null;
 
             // Act & Assert
             await formatter.WriteAsync(outputFormatterContext);
+
+            Assert.NotNull(outputFormatterContext.HttpContext.Response.ContentLength);
         }
 
         public static IEnumerable<object[]> TypesForGetSupportedContentTypes

src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs

@@ -155,7 +155,7 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co
                 if (fileBufferingWriteStream != null)
                 {
                     response.ContentLength = fileBufferingWriteStream.Length;
-                    await fileBufferingWriteStream.DrainBufferAsync(response.Body);
+                    await fileBufferingWriteStream.DrainBufferAsync(response.BodyWriter);
                 }
             }
             finally

src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs

@@ -305,6 +305,7 @@ public async Task WriteToStreamAsync_LargePayload_DoesNotPerformSynchronousWrite
             var stream = new Mock<Stream> { CallBase = true };
             stream.Setup(v => v.WriteAsync(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<CancellationToken>()))
                 .Returns(Task.CompletedTask);
+            stream.Setup(v => v.FlushAsync(It.IsAny<CancellationToken>())).Returns(Task.CompletedTask);
             stream.SetupGet(s => s.CanWrite).Returns(true);
 
             var formatter = new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool<char>.Shared, new MvcOptions());
@@ -322,6 +323,7 @@ public async Task WriteToStreamAsync_LargePayload_DoesNotPerformSynchronousWrite
 
             stream.Verify(v => v.Write(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>()), Times.Never());
             stream.Verify(v => v.Flush(), Times.Never());
+            Assert.NotNull(outputFormatterContext.HttpContext.Response.ContentLength);
         }
 
         private class TestableJsonOutputFormatter : NewtonsoftJsonOutputFormatter