From 692569d8c71d6b25cd0edbac2bb7aedb9b9bd3ef Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 29 Mar 2019 12:13:48 -0700 Subject: [PATCH 1/7] Buffer writes from sources of synchronous writes Fixes https://github.com/aspnet/AspNetCore/issues/6397 --- src/Http/Http/src/Internal/BufferingHelper.cs | 35 +- .../Http/src/Microsoft.AspNetCore.Http.csproj | 1 + .../src/AspNetCoreTempDirectory.cs | 37 +++ .../src/FileBufferingWriteStream.cs | 308 ++++++++++++++++++ .../src/HttpResponseStreamWriter.cs | 19 ++ .../test/AspNetCoreTempDirectoryTests.cs} | 8 +- .../test/FileBufferingWriteTests.cs | 291 +++++++++++++++++ src/Mvc/Mvc.Core/src/MvcOptions.cs | 8 +- .../test/CreatedAtActionResultTests.cs | 4 +- .../test/CreatedAtRouteResultTests.cs | 7 +- src/Mvc/Mvc.Core/test/CreatedResultTests.cs | 5 +- .../test/Formatters/FormatFilterTest.cs | 7 +- .../Formatters/JsonOutputFormatterTestBase.cs | 4 +- .../test/HttpNotFoundObjectResultTest.cs | 7 +- .../Mvc.Core/test/HttpOkObjectResultTest.cs | 7 +- ...mlDataContractSerializerOutputFormatter.cs | 42 ++- .../src/XmlSerializerOutputFormatter.cs | 42 ++- ...taContractSerializerOutputFormatterTest.cs | 5 + .../test/XmlSerializerOutputFormatterTest.cs | 5 + .../NewtonsoftJsonMvcCoreBuilderExtensions.cs | 6 - .../NewtonsoftJsonMvcOptionsSetup.cs | 2 +- .../src/JsonResultExecutor.cs | 65 ++-- .../src/NewtonsoftJsonOutputFormatter.cs | 50 ++- .../test/JsonResultExecutorTest.cs | 66 ++-- .../Mvc.NewtonsoftJson/test/JsonResultTest.cs | 1 + .../test/NewtonsoftJsonOutputFormatterTest.cs | 42 ++- .../src/ViewComponentResultExecutor.cs | 98 ++++-- .../test/ViewComponentResultTest.cs | 3 +- .../ContentNegotiation/NormalController.cs | 2 +- .../Controllers/JsonFormatterController.cs | 2 +- 30 files changed, 969 insertions(+), 210 deletions(-) create mode 100644 src/Http/WebUtilities/src/AspNetCoreTempDirectory.cs create mode 100644 src/Http/WebUtilities/src/FileBufferingWriteStream.cs rename src/Http/{Http/test/Internal/BufferingHelperTests.cs => WebUtilities/test/AspNetCoreTempDirectoryTests.cs} (72%) create mode 100644 src/Http/WebUtilities/test/FileBufferingWriteTests.cs diff --git a/src/Http/Http/src/Internal/BufferingHelper.cs b/src/Http/Http/src/Internal/BufferingHelper.cs index b912f37116b2..d3ae6e42c83b 100644 --- a/src/Http/Http/src/Internal/BufferingHelper.cs +++ b/src/Http/Http/src/Internal/BufferingHelper.cs @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.IO; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.WebUtilities; namespace Microsoft.AspNetCore.Http.Internal @@ -11,33 +11,6 @@ public static class BufferingHelper { internal const int DefaultBufferThreshold = 1024 * 30; - private readonly static Func _getTempDirectory = () => TempDirectory; - - private static string _tempDirectory; - - public static string TempDirectory - { - get - { - if (_tempDirectory == null) - { - // Look for folders in the following order. - var temp = Environment.GetEnvironmentVariable("ASPNETCORE_TEMP") ?? // ASPNETCORE_TEMP - User set temporary location. - Path.GetTempPath(); // Fall back. - - if (!Directory.Exists(temp)) - { - // TODO: ??? - throw new DirectoryNotFoundException(temp); - } - - _tempDirectory = temp; - } - - return _tempDirectory; - } - } - public static HttpRequest EnableRewind(this HttpRequest request, int bufferThreshold = DefaultBufferThreshold, long? bufferLimit = null) { if (request == null) @@ -48,7 +21,7 @@ public static HttpRequest EnableRewind(this HttpRequest request, int bufferThres var body = request.Body; if (!body.CanSeek) { - var fileStream = new FileBufferingReadStream(body, bufferThreshold, bufferLimit, _getTempDirectory); + var fileStream = new FileBufferingReadStream(body, bufferThreshold, bufferLimit, AspNetCoreTempDirectory.TempDirectoryFactory); request.Body = fileStream; request.HttpContext.Response.RegisterForDispose(fileStream); } @@ -70,11 +43,11 @@ public static MultipartSection EnableRewind(this MultipartSection section, Actio var body = section.Body; if (!body.CanSeek) { - var fileStream = new FileBufferingReadStream(body, bufferThreshold, bufferLimit, _getTempDirectory); + var fileStream = new FileBufferingReadStream(body, bufferThreshold, bufferLimit, AspNetCoreTempDirectory.TempDirectoryFactory); section.Body = fileStream; registerForDispose(fileStream); } return section; } } -} \ No newline at end of file +} diff --git a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj index fd9d84712a62..50d2ff7281a9 100644 --- a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj +++ b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj @@ -13,6 +13,7 @@ + diff --git a/src/Http/WebUtilities/src/AspNetCoreTempDirectory.cs b/src/Http/WebUtilities/src/AspNetCoreTempDirectory.cs new file mode 100644 index 000000000000..868d4efd05e9 --- /dev/null +++ b/src/Http/WebUtilities/src/AspNetCoreTempDirectory.cs @@ -0,0 +1,37 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.IO; + +namespace Microsoft.AspNetCore.Internal +{ + internal static class AspNetCoreTempDirectory + { + private static string _tempDirectory; + + public static string TempDirectory + { + get + { + if (_tempDirectory == null) + { + // Look for folders in the following order. + var temp = Environment.GetEnvironmentVariable("ASPNETCORE_TEMP") ?? // ASPNETCORE_TEMP - User set temporary location. + Path.GetTempPath(); // Fall back. + + if (!Directory.Exists(temp)) + { + throw new DirectoryNotFoundException(temp); + } + + _tempDirectory = temp; + } + + return _tempDirectory; + } + } + + public static Func TempDirectoryFactory => () => TempDirectory; + } +} diff --git a/src/Http/WebUtilities/src/FileBufferingWriteStream.cs b/src/Http/WebUtilities/src/FileBufferingWriteStream.cs new file mode 100644 index 000000000000..09128d31059f --- /dev/null +++ b/src/Http/WebUtilities/src/FileBufferingWriteStream.cs @@ -0,0 +1,308 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Buffers; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Internal; + +namespace Microsoft.AspNetCore.WebUtilities +{ + /// + /// A that wraps another stream and buffers content to be written. + /// + /// is intended to provide a workaround for APIs that need to perform + /// synchronous writes to the HTTP Response Stream while contending with a server that is configured to disallow synchronous writes. + /// Synchronous writes are buffered to a memory backed stream up to the specified threshold, after which further writes are spooled to + /// a temporary file on disk. + /// + /// + /// The performs opportunistic writes to the wrapping stream + /// when asychronous operation such as or + /// are performed. + /// + /// + public sealed class FileBufferingWriteStream : Stream + { + private const int MaxRentedBufferSize = 1024 * 1024; // 1MB + private const int DefaultMemoryThreshold = 30 * 1024; // 30k + + private readonly Stream _writeStream; + private readonly int _memoryThreshold; + private readonly long? _bufferLimit; + private readonly Func _tempFileDirectoryAccessor; + private readonly ArrayPool _bytePool; + private readonly byte[] _rentedBuffer; + + /// + /// Initializes a new instance of . + /// + /// The to write buffered contents to. + /// + /// The maximum amount of memory in bytes to allocate before switching to a file on disk. + /// Defaults to 30kb. + /// + /// + /// The maximum amouont of bytes that the is allowed to buffer. + /// + /// Provides the location of the directory to write buffered contents to. + /// When unspecified, uses the value specified by the environment variable ASPNETCORE_TEMP if available, otherwise + /// uses the value returned by . + /// + public FileBufferingWriteStream( + Stream writeStream, + int memoryThreshold = DefaultMemoryThreshold, + long? bufferLimit = null, + Func tempFileDirectoryAccessor = null) + : this(writeStream, memoryThreshold, bufferLimit, tempFileDirectoryAccessor, ArrayPool.Shared) + { + + } + + internal FileBufferingWriteStream( + Stream writeStream, + int memoryThreshold, + long? bufferLimit, + Func tempFileDirectoryAccessor, + ArrayPool bytePool) + { + _writeStream = writeStream ?? throw new ArgumentNullException(nameof(writeStream)); + + if (memoryThreshold < 0) + { + throw new ArgumentOutOfRangeException(nameof(memoryThreshold)); + } + + if (bufferLimit != null && bufferLimit < memoryThreshold) + { + // We would expect a limit at least as much as memoryThreshold + throw new ArgumentOutOfRangeException(nameof(bufferLimit), $"{nameof(bufferLimit)} must be larger than {nameof(memoryThreshold)}."); + } + + _memoryThreshold = memoryThreshold; + _bufferLimit = bufferLimit; + _tempFileDirectoryAccessor = tempFileDirectoryAccessor ?? AspNetCoreTempDirectory.TempDirectoryFactory; + _bytePool = bytePool; + + if (memoryThreshold < MaxRentedBufferSize) + { + _rentedBuffer = bytePool.Rent(memoryThreshold); + MemoryStream = new MemoryStream(_rentedBuffer); + MemoryStream.SetLength(0); + } + else + { + MemoryStream = new MemoryStream(); + } + } + + /// + public override bool CanRead => false; + + /// + public override bool CanSeek => false; + + /// + public override bool CanWrite => true; + + /// + public override long Length => throw new NotSupportedException(); + + /// + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + internal long BufferedLength => MemoryStream.Length + (FileStream?.Length ?? 0); + + internal MemoryStream MemoryStream { get; } + + internal FileStream FileStream { get; private set; } + + internal bool Disposed { get; private set; } + + /// + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + /// + public override int Read(byte[] buffer, int offset, int count) + => throw new NotSupportedException(); + + /// + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + => throw new NotSupportedException(); + + /// + public override void Write(byte[] buffer, int offset, int count) + { + ThrowArgumentException(buffer, offset, count); + ThrowIfDisposed(); + + if (_bufferLimit.HasValue && _bufferLimit - BufferedLength < count) + { + DiposeInternal(); + throw new IOException("Buffer limit exceeded."); + } + + var availableMemory = _memoryThreshold - MemoryStream.Position; + if (count <= availableMemory) + { + // Buffer content in the MemoryStream if it has capacity. + MemoryStream.Write(buffer, offset, count); + } + else + { + // If the MemoryStream is incapable of accomodating the content to be written + // spool to disk. + EnsureFileStream(); + CopyContent(MemoryStream, FileStream); + FileStream.Write(buffer, offset, count); + } + } + + /// + public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + ThrowArgumentException(buffer, offset, count); + ThrowIfDisposed(); + + // If we have the opportunity to go async, write the buffered content to the response. + await FlushAsync(cancellationToken); + await _writeStream.WriteAsync(buffer, offset, count, cancellationToken); + } + + /// + public override void SetLength(long value) => throw new NotSupportedException(); + + /// + // In the ordinary case, we expect this to throw if the target is the HttpResponse Body + // and disallows synchronous writes. We do not need to optimize for this. + public override void Flush() + { + if (FileStream != null) + { + CopyContent(FileStream, _writeStream); + } + + CopyContent(MemoryStream, _writeStream); + } + + /// + public override async Task FlushAsync(CancellationToken cancellationToken) + { + if (FileStream != null) + { + await CopyContentAsync(FileStream, _writeStream, cancellationToken); + } + + await CopyContentAsync(MemoryStream, _writeStream, cancellationToken); + } + + /// + protected override void Dispose(bool disposing) + { + if (!Disposed) + { + Disposed = true; + Flush(); + + DiposeInternal(); + } + } + + private void DiposeInternal() + { + Disposed = true; + _bytePool.Return(_rentedBuffer); + MemoryStream.Dispose(); + FileStream?.Dispose(); + } + + /// + public override async ValueTask DisposeAsync() + { + if (!Disposed) + { + Disposed = true; + try + { + await FlushAsync(); + } + finally + { + if (_rentedBuffer != null) + { + _bytePool.Return(_rentedBuffer); + } + await MemoryStream.DisposeAsync(); + await (FileStream?.DisposeAsync() ?? default); + } + } + } + + private void EnsureFileStream() + { + if (FileStream == null) + { + var tempFileDirectory = _tempFileDirectoryAccessor(); + var tempFileName = Path.Combine(tempFileDirectory, "ASPNETCORE_" + Guid.NewGuid() + ".tmp"); + FileStream = new FileStream( + tempFileName, + FileMode.Create, + FileAccess.ReadWrite, + FileShare.Delete, + bufferSize: 1, + FileOptions.Asynchronous | FileOptions.SequentialScan | FileOptions.DeleteOnClose); + } + } + + private void ThrowIfDisposed() + { + if (Disposed) + { + throw new ObjectDisposedException(nameof(FileBufferingReadStream)); + } + } + + private static void ThrowArgumentException(byte[] buffer, int offset, int count) + { + if (buffer == null) + { + throw new ArgumentNullException(nameof(buffer)); + } + + if (offset < 0) + { + throw new ArgumentOutOfRangeException(nameof(offset)); + } + + if (count < 0) + { + throw new ArgumentOutOfRangeException(nameof(count)); + } + + if (buffer.Length - offset < count) + { + throw new ArgumentOutOfRangeException(nameof(offset)); + } + } + + private static void CopyContent(Stream source, Stream destination) + { + source.Position = 0; + source.CopyTo(destination); + source.SetLength(0); + } + + private static async Task CopyContentAsync(Stream source, Stream destination, CancellationToken cancellationToken) + { + source.Position = 0; + await source.CopyToAsync(destination, cancellationToken); + source.SetLength(0); + } + } +} diff --git a/src/Http/WebUtilities/src/HttpResponseStreamWriter.cs b/src/Http/WebUtilities/src/HttpResponseStreamWriter.cs index c5bcdd0aa3c0..a17116ae3e38 100644 --- a/src/Http/WebUtilities/src/HttpResponseStreamWriter.cs +++ b/src/Http/WebUtilities/src/HttpResponseStreamWriter.cs @@ -310,6 +310,25 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + public override async ValueTask DisposeAsync() + { + if (!_disposed) + { + _disposed = true; + try + { + await FlushInternalAsync(flushEncoder: true); + } + finally + { + _bytePool.Return(_byteBuffer); + _charPool.Return(_charBuffer); + } + } + + await base.DisposeAsync(); + } + // Note: our FlushInternal method does NOT flush the underlying stream. This would result in // chunking. private void FlushInternal(bool flushEncoder) diff --git a/src/Http/Http/test/Internal/BufferingHelperTests.cs b/src/Http/WebUtilities/test/AspNetCoreTempDirectoryTests.cs similarity index 72% rename from src/Http/Http/test/Internal/BufferingHelperTests.cs rename to src/Http/WebUtilities/test/AspNetCoreTempDirectoryTests.cs index 9ad48986f501..4e0621ca8567 100644 --- a/src/Http/Http/test/Internal/BufferingHelperTests.cs +++ b/src/Http/WebUtilities/test/AspNetCoreTempDirectoryTests.cs @@ -4,16 +4,16 @@ using System.IO; using Xunit; -namespace Microsoft.AspNetCore.Http.Internal +namespace Microsoft.AspNetCore.Internal { - public class BufferingHelperTests + public class AspNetCoreTempDirectoryTests { [Fact] public void GetTempDirectory_Returns_Valid_Location() { - var tempDirectory = BufferingHelper.TempDirectory; + var tempDirectory = AspNetCoreTempDirectory.TempDirectory; Assert.NotNull(tempDirectory); Assert.True(Directory.Exists(tempDirectory)); } } -} \ No newline at end of file +} diff --git a/src/Http/WebUtilities/test/FileBufferingWriteTests.cs b/src/Http/WebUtilities/test/FileBufferingWriteTests.cs new file mode 100644 index 000000000000..2f4186922739 --- /dev/null +++ b/src/Http/WebUtilities/test/FileBufferingWriteTests.cs @@ -0,0 +1,291 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Buffers; +using System.IO; +using System.Text; +using System.Threading.Tasks; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.WebUtilities.Tests +{ + public class FileBufferingWriteStreamTests : IDisposable + { + private readonly string TempDirectory = Path.Combine(Path.GetTempPath(), "FileBufferingWriteTests", Path.GetRandomFileName()); + + public FileBufferingWriteStreamTests() + { + Directory.CreateDirectory(TempDirectory); + } + + [Fact] + public void Write_BuffersContentToMemory() + { + // Arrange + using var writeStream = new MemoryStream(); + using var bufferingStream = new FileBufferingWriteStream(writeStream, tempFileDirectoryAccessor: () => TempDirectory); + var input = Encoding.UTF8.GetBytes("Hello world"); + + // Act + bufferingStream.Write(input, 0, input.Length); + + // Assert + // We should have written content to the MemoryStream + var memoryStream = bufferingStream.MemoryStream; + Assert.Equal(input, memoryStream.ToArray()); + + // No files should not have been created. + Assert.Null(bufferingStream.FileStream); + + // No content should have been written to the wrapping stream + Assert.Equal(0, writeStream.Length); + } + + [Fact] + public void Write_BuffersContentToDisk_WhenMemoryThresholdIsReached() + { + // Arrange + using var writeStream = new MemoryStream(); + var input = new byte[] { 1, 2, 3, 4, 5 }; + using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + bufferingStream.Write(input, 0, input.Length); + + // Assert + var memoryStream = bufferingStream.MemoryStream; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(input, fileBytes); + + // No content should be in the memory stream + Assert.Equal(0, memoryStream.Length); + + // No content should have been written to the wrapping stream + Assert.Equal(0, writeStream.Length); + } + + [Fact] + public void Write_SpoolsContentFromMemoryToDisk() + { + // Arrange + using var writeStream = new MemoryStream(); + var input = new byte[] { 1, 2, 3, 4, 5, 6, 7 }; + using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 4, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + bufferingStream.Write(input, 0, 3); + bufferingStream.Write(input, 3, 2); + bufferingStream.Write(input, 5, 2); + + // Assert + var memoryStream = bufferingStream.MemoryStream; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(new byte[] { 1, 2, 3, 4, 5 }, fileBytes); + + Assert.Equal(new byte[] { 6, 7 }, memoryStream.ToArray()); + + // No content should have been written to the wrapping stream + Assert.Equal(0, writeStream.Length); + } + + [Fact] + public async Task WriteAsync_CopiesBufferedContentsFromMemoryStream() + { + // Arrange + using var writeStream = new MemoryStream(); + using var bufferingStream = new FileBufferingWriteStream(writeStream, tempFileDirectoryAccessor: () => TempDirectory); + var input = Encoding.UTF8.GetBytes("Hello world"); + + // Act + bufferingStream.Write(input, 0, 5); + await bufferingStream.WriteAsync(input, 5, input.Length - 5); + + // Assert + // We should have written all content to the output. + Assert.Equal(input, writeStream.ToArray()); + } + + [Fact] + public async Task WriteAsync_CopiesBufferedContentsFromFileStream() + { + // Arrange + using var writeStream = new MemoryStream(); + using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 1, tempFileDirectoryAccessor: () => TempDirectory); + var input = Encoding.UTF8.GetBytes("Hello world"); + + // Act + bufferingStream.Write(input, 0, 5); + await bufferingStream.WriteAsync(input, 5, input.Length - 5); + + // Assert + // We should have written all content to the output. + Assert.Equal(0, bufferingStream.BufferedLength); + Assert.Equal(input, writeStream.ToArray()); + } + + [Fact] + public async Task WriteFollowedByWriteAsync() + { + // Arrange + using var writeStream = new MemoryStream(); + using var bufferingStream = new FileBufferingWriteStream(writeStream); + var input = new byte[] { 7, 8, 9 }; + + // Act + bufferingStream.Write(input, 0, 1); + await bufferingStream.WriteAsync(input, 1, 1); + bufferingStream.Write(input, 2, 1); + + // Assert + Assert.Equal(new byte[] { 7, 8 }, writeStream.ToArray()); + Assert.Equal(1, bufferingStream.BufferedLength); + Assert.Equal(new byte[] { 9 }, bufferingStream.MemoryStream.ToArray()); + } + + [Fact] + public void Dispose_FlushesBufferedContent() + { + // Arrange + using var writeStream = new MemoryStream(); + using var bufferingStream = new FileBufferingWriteStream(writeStream); + var input = new byte[] { 1, 2, 34 }; + + // Act + bufferingStream.Write(input, 0, input.Length); + bufferingStream.Dispose(); + + // Assert + Assert.Equal(input, writeStream.ToArray()); + } + + [Fact] + public void Dispose_ReturnsBuffer() + { + // Arrange + using var writeStream = new MemoryStream(); + var arrayPool = new Mock>(); + var bytes = new byte[10]; + arrayPool.Setup(p => p.Rent(It.IsAny())).Returns(bytes); + using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, null, tempFileDirectoryAccessor: () => TempDirectory, bytePool: arrayPool.Object); + + // Act + bufferingStream.Dispose(); + bufferingStream.Dispose(); // Double disposing shouldn't be a problem + + // Assert + Assert.True(bufferingStream.Disposed); + arrayPool.Verify(v => v.Return(bytes, false), Times.Once()); + } + + [Fact] + public async Task DisposeAsync_FlushesBufferedContent() + { + // Arrange + using var writeStream = new MemoryStream(); + using var bufferingStream = new FileBufferingWriteStream(writeStream); + var input = new byte[] { 1, 2, 34 }; + + // Act + bufferingStream.Write(input, 0, input.Length); + await bufferingStream.DisposeAsync(); + + // Assert + Assert.Equal(input, writeStream.ToArray()); + } + + [Fact] + public async Task DisposeAsync_ReturnsBuffer() + { + // Arrange + using var writeStream = new MemoryStream(); + var arrayPool = new Mock>(); + var bytes = new byte[10]; + arrayPool.Setup(p => p.Rent(It.IsAny())).Returns(bytes); + using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, null, tempFileDirectoryAccessor: () => TempDirectory, bytePool: arrayPool.Object); + + // Act + await bufferingStream.DisposeAsync(); + await bufferingStream.DisposeAsync(); // Double disposing shouldn't be a problem + + // Assert + arrayPool.Verify(v => v.Return(bytes, false), Times.Once()); + } + + [Fact] + public void Write_Throws_IfBufferLimitIsReached() + { + // Arrange + using var writeStream = new MemoryStream(); + var input = new byte[6]; + var arrayPool = new Mock>(); + arrayPool.Setup(p => p.Rent(It.IsAny())).Returns(new byte[10]); + + using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory, bytePool: arrayPool.Object); + + // Act + bufferingStream.Write(input, 0, input.Length); + var exception = Assert.Throws(() => bufferingStream.Write(input, 0, input.Length)); + Assert.Equal("Buffer limit exceeded.", exception.Message); + + // Verify we return the buffer. + arrayPool.Verify(v => v.Return(It.IsAny(), false), Times.Once()); + } + + [Fact] + public void Write_CopiesContentOnFlush() + { + // Arrange + using var writeStream = new MemoryStream(); + var input = new byte[6]; + var arrayPool = new Mock>(); + arrayPool.Setup(p => p.Rent(It.IsAny())).Returns(new byte[10]); + + using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory, bytePool: arrayPool.Object); + + // Act + bufferingStream.Write(input, 0, input.Length); + var exception = Assert.Throws(() => bufferingStream.Write(input, 0, input.Length)); + Assert.Equal("Buffer limit exceeded.", exception.Message); + + // Verify we return the buffer. + arrayPool.Verify(v => v.Return(It.IsAny(), false), Times.Once()); + } + + public void Dispose() + { + try + { + Directory.Delete(TempDirectory, recursive: true); + } + catch + { + } + } + + private static byte[] ReadFileContent(FileStream fileStream) + { + fileStream.Position = 0; + var count = fileStream.Length; + var bytes = new ArraySegment(new byte[fileStream.Length]); + while (count > 0) + { + var read = fileStream.Read(bytes.AsSpan()); + Assert.False(read == 0, "Should not EOF before we've read the file."); + bytes = bytes.Slice(read); + count -= read; + } + + return bytes.Array; + } + } +} diff --git a/src/Mvc/Mvc.Core/src/MvcOptions.cs b/src/Mvc/Mvc.Core/src/MvcOptions.cs index c5d2ea5d161b..3b8c53a539bf 100644 --- a/src/Mvc/Mvc.Core/src/MvcOptions.cs +++ b/src/Mvc/Mvc.Core/src/MvcOptions.cs @@ -105,7 +105,13 @@ public MvcOptions() /// /// Gets or sets the flag to buffer the request body in input formatters. Default is false. /// - public bool SuppressInputFormatterBuffering { get; set; } = false; + public bool SuppressInputFormatterBuffering { get; set; } + + /// + /// Gets or sets the flag that determines if buffering is disabled for output formatters that + /// synchronously write to the HTTP response body. + /// + public bool SuppressOutputFormatterBuffering { get; set; } /// /// Gets or sets the maximum number of validation errors that are allowed by this application before further diff --git a/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs index 01e7362bc2bb..0088dadf499f 100644 --- a/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs @@ -91,9 +91,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs index 7494544d3142..829992317681 100644 --- a/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; using System.Collections.Generic; using System.IO; using System.Threading.Tasks; @@ -10,14 +9,12 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; -using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNetCore.Mvc @@ -107,9 +104,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Core/test/CreatedResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedResultTests.cs index f6bcf13473bb..8d984ec7beaa 100644 --- a/src/Mvc/Mvc.Core/test/CreatedResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedResultTests.cs @@ -9,7 +9,6 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; @@ -93,9 +92,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs index 0560d51d2876..bb8da19ae2a5 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs @@ -2,13 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; using System.Collections.Generic; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging.Abstractions; @@ -16,7 +14,6 @@ using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Moq; -using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNetCore.Mvc.Formatters @@ -468,9 +465,7 @@ private void Initialize( // Set up default output formatters. MvcOptions.OutputFormatters.Add(new HttpNoContentOutputFormatter()); MvcOptions.OutputFormatters.Add(new StringOutputFormatter()); - MvcOptions.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + MvcOptions.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); // Set up default mapping for json extensions to content type MvcOptions.FormatterMappings.SetMediaTypeMappingForFormat( diff --git a/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs b/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs index 0039a4a9ade1..0467d0e51cc9 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs @@ -100,7 +100,7 @@ public async Task ErrorDuringSerialization_DoesNotCloseTheBrackets() protected static ActionContext GetActionContext( MediaTypeHeaderValue contentType, - MemoryStream responseStream = null) + Stream responseStream = null) { var httpContext = new DefaultHttpContext(); httpContext.Request.ContentType = contentType.ToString(); @@ -115,7 +115,7 @@ protected static OutputFormatterWriteContext GetOutputFormatterContext( object outputValue, Type outputType, string contentType = "application/xml; charset=utf-8", - MemoryStream responseStream = null) + Stream responseStream = null) { var mediaTypeHeaderValue = MediaTypeHeaderValue.Parse(contentType); diff --git a/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs b/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs index b6a64d8b537d..58d6abb3f17c 100644 --- a/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs @@ -2,17 +2,14 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; using System.IO; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; -using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNetCore.Mvc @@ -72,9 +69,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs b/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs index b8642a38f453..0c29f4d541f1 100644 --- a/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs @@ -2,18 +2,15 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; -using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNetCore.Mvc @@ -73,9 +70,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs index 48e7a0d15b22..abb389aa1a20 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs @@ -9,8 +9,12 @@ using System.Text; using System.Threading.Tasks; using System.Xml; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters.Xml; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters { @@ -23,6 +27,7 @@ public class XmlDataContractSerializerOutputFormatter : TextOutputFormatter private readonly ConcurrentDictionary _serializerCache = new ConcurrentDictionary(); private readonly ILogger _logger; private DataContractSerializerSettings _serializerSettings; + private MvcOptions _mvcOptions; /// /// Initializes a new instance of @@ -254,25 +259,38 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var dataContractSerializer = GetCachedSerializer(wrappingType); - // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 - var syncIOFeature = context.HttpContext.Features.Get(); - if (syncIOFeature != null) + var responseStream = GetResponseStream(context.HttpContext); + + try { - syncIOFeature.AllowSynchronousIO = true; + await using (var textWriter = context.WriterFactory(responseStream, writerSettings.Encoding)) + { + using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) + { + dataContractSerializer.WriteObject(xmlWriter, value); + } + } } - - using (var textWriter = context.WriterFactory(context.HttpContext.Response.Body, writerSettings.Encoding)) + finally { - using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) + if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) { - dataContractSerializer.WriteObject(xmlWriter, value); + await fileBufferingWriteStream.DisposeAsync(); } + } + } + + private Stream GetResponseStream(HttpContext httpContext) + { + _mvcOptions ??= httpContext.RequestServices.GetRequiredService>().Value; - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await textWriter.FlushAsync(); + var responseStream = httpContext.Response.Body; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + responseStream = new FileBufferingWriteStream(responseStream); } + + return responseStream; } /// diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs index da1812e6e954..6d11ce5701aa 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs @@ -9,8 +9,12 @@ using System.Threading.Tasks; using System.Xml; using System.Xml.Serialization; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters.Xml; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters { @@ -22,6 +26,7 @@ public class XmlSerializerOutputFormatter : TextOutputFormatter { private readonly ConcurrentDictionary _serializerCache = new ConcurrentDictionary(); private readonly ILogger _logger; + private MvcOptions _mvcOptions; /// /// Initializes a new instance of @@ -230,25 +235,38 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var xmlSerializer = GetCachedSerializer(wrappingType); - // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 - var syncIOFeature = context.HttpContext.Features.Get(); - if (syncIOFeature != null) + var responseStream = GetResponseStream(context.HttpContext); + + try { - syncIOFeature.AllowSynchronousIO = true; + await using (var textWriter = context.WriterFactory(responseStream, selectedEncoding)) + { + using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) + { + Serialize(xmlSerializer, xmlWriter, value); + } + } } - - using (var textWriter = context.WriterFactory(context.HttpContext.Response.Body, writerSettings.Encoding)) + finally { - using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) + if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) { - Serialize(xmlSerializer, xmlWriter, value); + await fileBufferingWriteStream.DisposeAsync(); } + } + } + + private Stream GetResponseStream(HttpContext httpContext) + { + _mvcOptions ??= httpContext.RequestServices.GetRequiredService>().Value; - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await textWriter.FlushAsync(); + var responseStream = httpContext.Response.Body; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + responseStream = new FileBufferingWriteStream(responseStream); } + + return responseStream; } /// diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerOutputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerOutputFormatterTest.cs index fd33afc14805..d94970348f68 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerOutputFormatterTest.cs @@ -10,8 +10,10 @@ using System.Xml; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Testing.xunit; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Xunit; @@ -735,6 +737,9 @@ private static HttpContext GetHttpContext(string contentType) request.Headers["Accept-Charset"] = MediaTypeHeaderValue.Parse(contentType).Charset.ToString(); request.ContentType = contentType; httpContext.Response.Body = new MemoryStream(); + httpContext.RequestServices = new ServiceCollection() + .AddSingleton(Options.Create(new MvcOptions())) + .BuildServiceProvider(); return httpContext; } diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs index 34bb5e0298fe..fff6d5c062fc 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs @@ -10,8 +10,10 @@ using System.Xml; using System.Xml.Serialization; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Xunit; @@ -520,6 +522,9 @@ private static HttpContext GetHttpContext(string contentType) request.Headers["Accept-Charset"] = MediaTypeHeaderValue.Parse(contentType).Charset.ToString(); request.ContentType = contentType; httpContext.Response.Body = new MemoryStream(); + httpContext.RequestServices = new ServiceCollection() + .AddSingleton(Options.Create(new MvcOptions())) + .BuildServiceProvider(); return httpContext; } diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs index 225385317428..d39ce087cd16 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs @@ -102,12 +102,6 @@ internal static void AddServicesCore(IServiceCollection services) } services.TryAddSingleton(); - services.TryAdd(ServiceDescriptor.Singleton(serviceProvider => - { - var options = serviceProvider.GetRequiredService>().Value; - var charPool = serviceProvider.GetRequiredService>(); - return new NewtonsoftJsonOutputFormatter(options.SerializerSettings, charPool); - })); } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcOptionsSetup.cs b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcOptionsSetup.cs index c466d8d11416..fca499d1fd18 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcOptionsSetup.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcOptionsSetup.cs @@ -60,7 +60,7 @@ public NewtonsoftJsonMvcOptionsSetup( public void Configure(MvcOptions options) { options.OutputFormatters.RemoveType(); - options.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter(_jsonOptions.SerializerSettings, _charPool)); + options.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter(_jsonOptions.SerializerSettings, _charPool, options)); options.InputFormatters.RemoveType(); // Register JsonPatchInputFormatter before JsonInputFormatter, otherwise diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs index b4ccaa171ff7..ec49d955af2f 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs @@ -3,10 +3,13 @@ using System; using System.Buffers; +using System.IO; using System.Text; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Net.Http.Headers; @@ -26,7 +29,8 @@ internal class JsonResultExecutor : IActionResultExecutor private readonly IHttpResponseStreamWriterFactory _writerFactory; private readonly ILogger _logger; - private readonly MvcNewtonsoftJsonOptions _options; + private readonly MvcOptions _mvcOptions; + private readonly MvcNewtonsoftJsonOptions _jsonOptions; private readonly IArrayPool _charPool; /// @@ -34,12 +38,14 @@ internal class JsonResultExecutor : IActionResultExecutor /// /// The . /// The . - /// The . + /// Accessor to . + /// Accessor to . /// The for creating buffers. public JsonResultExecutor( IHttpResponseStreamWriterFactory writerFactory, ILogger logger, - IOptions options, + IOptions mvcOptions, + IOptions jsonOptions, ArrayPool charPool) { if (writerFactory == null) @@ -52,9 +58,9 @@ public JsonResultExecutor( throw new ArgumentNullException(nameof(logger)); } - if (options == null) + if (jsonOptions == null) { - throw new ArgumentNullException(nameof(options)); + throw new ArgumentNullException(nameof(jsonOptions)); } if (charPool == null) @@ -64,7 +70,8 @@ public JsonResultExecutor( _writerFactory = writerFactory; _logger = logger; - _options = options.Value; + _mvcOptions = mvcOptions?.Value ?? throw new ArgumentNullException(nameof(mvcOptions)); + _jsonOptions = jsonOptions.Value; _charPool = new JsonArrayPool(charPool); } @@ -105,22 +112,42 @@ public async Task ExecuteAsync(ActionContext context, JsonResult result) } _logger.JsonResultExecuting(result.Value); - using (var writer = _writerFactory.CreateWriter(response.Body, resolvedContentTypeEncoding)) + + var responseStream = GetResponseStream(response); + + try { - using (var jsonWriter = new JsonTextWriter(writer)) + await using (var writer = _writerFactory.CreateWriter(responseStream, resolvedContentTypeEncoding)) { - jsonWriter.ArrayPool = _charPool; - jsonWriter.CloseOutput = false; - jsonWriter.AutoCompleteOnClose = false; - - var jsonSerializer = JsonSerializer.Create(jsonSerializerSettings); - jsonSerializer.Serialize(jsonWriter, result.Value); + using (var jsonWriter = new JsonTextWriter(writer)) + { + jsonWriter.ArrayPool = _charPool; + jsonWriter.CloseOutput = false; + jsonWriter.AutoCompleteOnClose = false; + + var jsonSerializer = JsonSerializer.Create(jsonSerializerSettings); + jsonSerializer.Serialize(jsonWriter, result.Value); + } + } + } + finally + { + if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) + { + await fileBufferingWriteStream.DisposeAsync(); } + } + } - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous write). - await writer.FlushAsync(); + private Stream GetResponseStream(HttpResponse response) + { + var responseStream = response.Body; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + responseStream = new FileBufferingWriteStream(responseStream); } + + return responseStream; } private JsonSerializerSettings GetSerializerSettings(JsonResult result) @@ -128,9 +155,9 @@ private JsonSerializerSettings GetSerializerSettings(JsonResult result) var serializerSettings = result.SerializerSettings; if (serializerSettings == null) { - return _options.SerializerSettings; + return _jsonOptions.SerializerSettings; } - else + else { if (!(serializerSettings is JsonSerializerSettings settingsFromResult)) { diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs index 934cf824ff2e..c37f9f107836 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs @@ -6,7 +6,9 @@ using System.IO; using System.Text; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.NewtonsoftJson; +using Microsoft.AspNetCore.WebUtilities; using Newtonsoft.Json; namespace Microsoft.AspNetCore.Mvc.Formatters @@ -17,6 +19,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters public class NewtonsoftJsonOutputFormatter : TextOutputFormatter { private readonly IArrayPool _charPool; + private readonly MvcOptions _mvcOptions; // Perf: JsonSerializers are relatively expensive to create, and are thread safe. We cache // the serializer and invalidate it when the settings change. @@ -31,7 +34,11 @@ public class NewtonsoftJsonOutputFormatter : TextOutputFormatter /// initially returned. /// /// The . - public NewtonsoftJsonOutputFormatter(JsonSerializerSettings serializerSettings, ArrayPool charPool) + /// The . + public NewtonsoftJsonOutputFormatter( + JsonSerializerSettings serializerSettings, + ArrayPool charPool, + MvcOptions mvcOptions) { if (serializerSettings == null) { @@ -45,6 +52,7 @@ public NewtonsoftJsonOutputFormatter(JsonSerializerSettings serializerSettings, SerializerSettings = serializerSettings; _charPool = new JsonArrayPool(charPool); + _mvcOptions = mvcOptions ?? throw new ArgumentNullException(nameof(mvcOptions)); SupportedEncodings.Add(Encoding.UTF8); SupportedEncodings.Add(Encoding.Unicode); @@ -123,27 +131,39 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co throw new ArgumentNullException(nameof(selectedEncoding)); } - // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 - var syncIOFeature = context.HttpContext.Features.Get(); - if (syncIOFeature != null) + var response = context.HttpContext.Response; + + var responseStream = GetResponseStream(response); + + try { - syncIOFeature.AllowSynchronousIO = true; + await using (var writer = context.WriterFactory(responseStream, selectedEncoding)) + { + using (var jsonWriter = CreateJsonWriter(writer)) + { + var jsonSerializer = CreateJsonSerializer(context); + jsonSerializer.Serialize(jsonWriter, context.Object); + } + } } - - var response = context.HttpContext.Response; - using (var writer = context.WriterFactory(response.Body, selectedEncoding)) + finally { - using (var jsonWriter = CreateJsonWriter(writer)) + if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) { - var jsonSerializer = CreateJsonSerializer(context); - jsonSerializer.Serialize(jsonWriter, context.Object); + await fileBufferingWriteStream.DisposeAsync(); } + } + } - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await writer.FlushAsync(); + private Stream GetResponseStream(HttpResponse response) + { + var responseStream = response.Body; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + responseStream = new FileBufferingWriteStream(responseStream); } + + return responseStream; } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs index 628916dd25f0..024ee7620c57 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs @@ -3,7 +3,9 @@ using System; using System.Buffers; +using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -220,63 +222,29 @@ public async Task ExecuteAsync_NullResult_LogsNull() } [Fact] - public async Task ExecuteAsync_WritesToTheResponseStream_WhenContentIsLargerThanBuffer() + public async Task ExecuteAsync_LargePayload_DoesNotPerformSynchronousWrites() { // Arrange - var writeLength = 2 * TestHttpResponseStreamWriterFactory.DefaultBufferSize + 4; - var text = new string('a', writeLength); - var expectedWriteCallCount = Math.Ceiling((double)writeLength / TestHttpResponseStreamWriterFactory.DefaultBufferSize); + var model = Enumerable.Range(0, 1000).Select(p => new TestModel { Property = new string('a', 5000)}); - var stream = new Mock(); + var stream = new Mock { CallBase = true }; + stream.Setup(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); stream.SetupGet(s => s.CanWrite).Returns(true); - var httpContext = new DefaultHttpContext(); - httpContext.Response.Body = stream.Object; - var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var context = GetActionContext(); + context.HttpContext.Response.Body = stream.Object; - var result = new JsonResult(text); var executor = CreateExecutor(); + var result = new JsonResult(model); // Act - await executor.ExecuteAsync(actionContext, result); + await executor.ExecuteAsync(context, result); // Assert - // HttpResponseStreamWriter buffers content up to the buffer size (16k). When writes exceed the buffer size, it'll perform a synchronous - // write to the response stream. - stream.Verify(s => s.Write(It.IsAny(), It.IsAny(), TestHttpResponseStreamWriterFactory.DefaultBufferSize), Times.Exactly(2)); + stream.Verify(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.AtLeastOnce()); - // Remainder buffered content is written asynchronously as part of the FlushAsync. - stream.Verify(s => s.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); - - // Dispose does not call Flush - stream.Verify(s => s.Flush(), Times.Never()); - } - - [Theory] - [InlineData(5)] - [InlineData(TestHttpResponseStreamWriterFactory.DefaultBufferSize - 30)] - public async Task ExecuteAsync_DoesNotWriteSynchronouslyToTheResponseBody_WhenContentIsSmallerThanBufferSize(int writeLength) - { - // Arrange - var text = new string('a', writeLength); - - var stream = new Mock(); - stream.SetupGet(s => s.CanWrite).Returns(true); - var httpContext = new DefaultHttpContext(); - httpContext.Response.Body = stream.Object; - var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); - - var result = new JsonResult(text); - var executor = CreateExecutor(); - - // Act - await executor.ExecuteAsync(actionContext, result); - - // Assert - // HttpResponseStreamWriter buffers content up to the buffer size (16k) and will asynchronously write content to the response as part - // of the FlushAsync call if the content written to it is smaller than the buffer size. - // This test verifies that no synchronous writes are performed in this scenario. - stream.Verify(s => s.Flush(), Times.Never()); - stream.Verify(s => s.Write(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + stream.Verify(v => v.Write(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + stream.Verify(v => v.Flush(), Times.Never()); } private static JsonResultExecutor CreateExecutor(ILogger logger = null) @@ -284,6 +252,7 @@ private static JsonResultExecutor CreateExecutor(ILogger log return new JsonResultExecutor( new TestHttpResponseStreamWriterFactory(), logger ?? NullLogger.Instance, + Options.Create(new MvcOptions()), Options.Create(new MvcNewtonsoftJsonOptions()), ArrayPool.Shared); } @@ -337,5 +306,10 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except MostRecentMessage = formatter(state, exception); } } + + private class TestModel + { + public string Property { get; set; } + } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs index dca9a521643a..bc1c0869ee10 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs @@ -47,6 +47,7 @@ private static HttpContext GetHttpContext() var executor = new JsonResultExecutor( new TestHttpResponseStreamWriterFactory(), NullLogger.Instance, + Options.Create(new MvcOptions()), Options.Create(new MvcNewtonsoftJsonOptions()), ArrayPool.Shared); diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs index e5cce46201ca..a04c79c63821 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs @@ -5,8 +5,11 @@ using System.Buffers; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; +using System.Threading; using System.Threading.Tasks; +using Moq; using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Newtonsoft.Json.Serialization; @@ -18,7 +21,7 @@ public class NewtonsoftJsonOutputFormatterTest : JsonOutputFormatterTestBase { protected override TextOutputFormatter GetOutputFormatter() { - return new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared); + return new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared, new MvcOptions()); } [Fact] @@ -56,7 +59,7 @@ public async Task ChangesTo_SerializerSettings_AffectSerialization() Formatting = Formatting.Indented, }; var expectedOutput = JsonConvert.SerializeObject(person, settings); - var jsonFormatter = new NewtonsoftJsonOutputFormatter(settings, ArrayPool.Shared); + var jsonFormatter = new NewtonsoftJsonOutputFormatter(settings, ArrayPool.Shared, new MvcOptions()); // Act await jsonFormatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.UTF8); @@ -274,8 +277,7 @@ public async Task WriteToStreamAsync_RoundTripsJToken() { // Arrange var beforeMessage = "Hello World"; - var formatter = new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared); - var before = new JValue(beforeMessage); + var formatter = new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared, new MvcOptions()); var memStream = new MemoryStream(); var outputFormatterContext = GetOutputFormatterContext( beforeMessage, @@ -294,10 +296,38 @@ public async Task WriteToStreamAsync_RoundTripsJToken() Assert.Equal(beforeMessage, afterMessage); } + [Fact] + public async Task WriteToStreamAsync_LargePayload_DoesNotPerformSynchronousWrites() + { + // Arrange + var model = Enumerable.Range(0, 1000).Select(p => new User { FullName = new string('a', 5000) }); + + var stream = new Mock { CallBase = true }; + stream.Setup(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + stream.SetupGet(s => s.CanWrite).Returns(true); + + var formatter = new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared, new MvcOptions()); + var outputFormatterContext = GetOutputFormatterContext( + model, + typeof(string), + "application/json; charset=utf-8", + stream.Object); + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.UTF8); + + // Assert + stream.Verify(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.AtLeastOnce()); + + stream.Verify(v => v.Write(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + stream.Verify(v => v.Flush(), Times.Never()); + } + private class TestableJsonOutputFormatter : NewtonsoftJsonOutputFormatter { public TestableJsonOutputFormatter(JsonSerializerSettings serializerSettings) - : base(serializerSettings, ArrayPool.Shared) + : base(serializerSettings, ArrayPool.Shared, new MvcOptions()) { } @@ -331,5 +361,5 @@ private class UserWithJsonObject public string FullName { get; set; } } - } + } } \ No newline at end of file diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs index 0d839e7b6840..95e26022a8c0 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs @@ -2,9 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO; using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNetCore.Html; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -17,6 +19,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures { + /// + /// An for . + /// public class ViewComponentResultExecutor : IActionResultExecutor { private readonly HtmlEncoder _htmlEncoder; @@ -24,13 +29,46 @@ public class ViewComponentResultExecutor : IActionResultExecutor _logger; private readonly IModelMetadataProvider _modelMetadataProvider; private readonly ITempDataDictionaryFactory _tempDataDictionaryFactory; - + private readonly MvcOptions _mvcOptions; + private readonly IHttpResponseStreamWriterFactory _writerFactory; + + /// + /// Initializes a new instance of . + /// + /// Accessor for . + /// The . + /// The . + /// The . + /// The . + [Obsolete("This constructor is obsolete and weill be removed in a future version.")] public ViewComponentResultExecutor( IOptions mvcHelperOptions, ILoggerFactory loggerFactory, HtmlEncoder htmlEncoder, IModelMetadataProvider modelMetadataProvider, ITempDataDictionaryFactory tempDataDictionaryFactory) + : this(mvcHelperOptions, loggerFactory, htmlEncoder, modelMetadataProvider, tempDataDictionaryFactory, null, null) + { + } + + /// + /// Initializes a new instance of . + /// + /// Accessor to . + /// The . + /// The . + /// The . + /// The . + /// Accessor to . + /// The . + public ViewComponentResultExecutor( + IOptions mvcHelperOptions, + ILoggerFactory loggerFactory, + HtmlEncoder htmlEncoder, + IModelMetadataProvider modelMetadataProvider, + ITempDataDictionaryFactory tempDataDictionaryFactory, + IOptions mvcOptions, + IHttpResponseStreamWriterFactory writerFactory) { if (mvcHelperOptions == null) { @@ -62,6 +100,8 @@ public ViewComponentResultExecutor( _htmlEncoder = htmlEncoder; _modelMetadataProvider = modelMetadataProvider; _tempDataDictionaryFactory = tempDataDictionaryFactory; + _mvcOptions = mvcOptions?.Value ?? throw new ArgumentNullException(nameof(mvcOptions)); + _writerFactory = writerFactory ?? throw new ArgumentNullException(nameof(writerFactory)); } /// @@ -105,34 +145,50 @@ public virtual async Task ExecuteAsync(ActionContext context, ViewComponentResul response.StatusCode = result.StatusCode.Value; } - // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 - var syncIOFeature = context.HttpContext.Features.Get(); - if (syncIOFeature != null) - { - syncIOFeature.AllowSynchronousIO = true; - } + var responseStream = GetResponseStream(response); - using (var writer = new HttpResponseStreamWriter(response.Body, resolvedContentTypeEncoding)) + try { - var viewContext = new ViewContext( - context, - NullView.Instance, - viewData, - tempData, - writer, - _htmlHelperOptions); + await using (var writer = _writerFactory.CreateWriter(responseStream, resolvedContentTypeEncoding)) + { + var viewContext = new ViewContext( + context, + NullView.Instance, + viewData, + tempData, + writer, + _htmlHelperOptions); - OnExecuting(viewContext); + OnExecuting(viewContext); - // IViewComponentHelper is stateful, we want to make sure to retrieve it every time we need it. - var viewComponentHelper = context.HttpContext.RequestServices.GetRequiredService(); - (viewComponentHelper as IViewContextAware)?.Contextualize(viewContext); + // IViewComponentHelper is stateful, we want to make sure to retrieve it every time we need it. + var viewComponentHelper = context.HttpContext.RequestServices.GetRequiredService(); + (viewComponentHelper as IViewContextAware)?.Contextualize(viewContext); - var viewComponentResult = await GetViewComponentResult(viewComponentHelper, _logger, result); - viewComponentResult.WriteTo(writer, _htmlEncoder); + var viewComponentResult = await GetViewComponentResult(viewComponentHelper, _logger, result); + viewComponentResult.WriteTo(writer, _htmlEncoder); + } + } + finally + { + if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) + { + await fileBufferingWriteStream.DisposeAsync(); + } } } + private Stream GetResponseStream(HttpResponse response) + { + var responseStream = response.Body; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + responseStream = new FileBufferingWriteStream(responseStream); + } + + return responseStream; + } + private void OnExecuting(ViewContext viewContext) { var viewDataValuesProvider = viewContext.HttpContext.Features.Get(); diff --git a/src/Mvc/Mvc.ViewFeatures/test/ViewComponentResultTest.cs b/src/Mvc/Mvc.ViewFeatures/test/ViewComponentResultTest.cs index 2c65acccb2ce..c8da99449b60 100644 --- a/src/Mvc/Mvc.ViewFeatures/test/ViewComponentResultTest.cs +++ b/src/Mvc/Mvc.ViewFeatures/test/ViewComponentResultTest.cs @@ -573,7 +573,6 @@ private IServiceCollection CreateServices( HttpContext context, params ViewComponentDescriptor[] descriptors) { - var httpContext = new DefaultHttpContext(); var diagnosticSource = new DiagnosticListener("Microsoft.AspNetCore"); if (diagnosticListener != null) { @@ -583,6 +582,7 @@ private IServiceCollection CreateServices( var services = new ServiceCollection(); services.AddSingleton(diagnosticSource); services.AddSingleton(); + services.AddSingleton(Options.Create(new MvcOptions())); services.AddSingleton(Options.Create(new MvcViewOptions())); services.AddTransient(); services.AddSingleton(); @@ -600,6 +600,7 @@ private IServiceCollection CreateServices( services.AddSingleton(); services.AddSingleton(); services.AddSingleton, ViewComponentResultExecutor>(); + services.AddSingleton(); return services; } diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/NormalController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/NormalController.cs index 19150c264feb..a86ff9919ab3 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/NormalController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/NormalController.cs @@ -25,7 +25,7 @@ static NormalController() public NormalController(ArrayPool charPool) { - _indentingFormatter = new NewtonsoftJsonOutputFormatter(_indentedSettings, charPool); + _indentingFormatter = new NewtonsoftJsonOutputFormatter(_indentedSettings, charPool, new MvcOptions()); } public override void OnActionExecuted(ActionExecutedContext context) diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs index 48e29ca041d7..0ebaf0c3242c 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs @@ -23,7 +23,7 @@ static JsonFormatterController() public JsonFormatterController(ArrayPool charPool) { - _indentingFormatter = new NewtonsoftJsonOutputFormatter(_indentedSettings, charPool); + _indentingFormatter = new NewtonsoftJsonOutputFormatter(_indentedSettings, charPool, new MvcOptions()); } public IActionResult ReturnsIndentedJson() From fa081f6ed56867ab61fbcd19486ddc6a0bc6e8d6 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 15 Apr 2019 11:10:42 -0700 Subject: [PATCH 2/7] Remove pass thru behavior * Require calling CopyTo to copy content to response stream * Use PagedByteBuffer to buffer content in memory --- .../src/FileBufferingWriteStream.cs | 178 ++++---- src/Http/WebUtilities/src/PagedByteBuffer.cs | 140 ++++++ .../test/FileBufferingWriteTests.cs | 404 +++++++++++++----- .../WebUtilities/test/FormPipeReaderTests.cs | 2 +- .../test/HttpRequestStreamReaderTest.cs | 2 +- .../test/HttpResponseStreamWriterTest.cs | 2 +- .../WebUtilities/test/PagedByteBufferTest.cs | 244 +++++++++++ ...mlDataContractSerializerOutputFormatter.cs | 38 +- .../src/XmlSerializerOutputFormatter.cs | 38 +- .../src/JsonResultExecutor.cs | 47 +- .../src/NewtonsoftJsonOutputFormatter.cs | 30 +- .../test/JsonResultExecutorTest.cs | 5 +- .../src/ViewComponentResultExecutor.cs | 98 +---- .../test/ViewComponentResultTest.cs | 3 +- 14 files changed, 872 insertions(+), 359 deletions(-) create mode 100644 src/Http/WebUtilities/src/PagedByteBuffer.cs create mode 100644 src/Http/WebUtilities/test/PagedByteBufferTest.cs diff --git a/src/Http/WebUtilities/src/FileBufferingWriteStream.cs b/src/Http/WebUtilities/src/FileBufferingWriteStream.cs index 09128d31059f..c42f64871141 100644 --- a/src/Http/WebUtilities/src/FileBufferingWriteStream.cs +++ b/src/Http/WebUtilities/src/FileBufferingWriteStream.cs @@ -3,6 +3,7 @@ using System; using System.Buffers; +using System.Diagnostics; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -19,30 +20,23 @@ namespace Microsoft.AspNetCore.WebUtilities /// a temporary file on disk. /// /// - /// The performs opportunistic writes to the wrapping stream - /// when asychronous operation such as or - /// are performed. + /// Consumers of this API can invoke to copy the results to the HTTP Response Stream. /// /// public sealed class FileBufferingWriteStream : Stream { - private const int MaxRentedBufferSize = 1024 * 1024; // 1MB - private const int DefaultMemoryThreshold = 30 * 1024; // 30k + private const int DefaultMemoryThreshold = 32 * 1024; // 32k - private readonly Stream _writeStream; private readonly int _memoryThreshold; private readonly long? _bufferLimit; private readonly Func _tempFileDirectoryAccessor; - private readonly ArrayPool _bytePool; - private readonly byte[] _rentedBuffer; /// /// Initializes a new instance of . /// - /// The to write buffered contents to. /// /// The maximum amount of memory in bytes to allocate before switching to a file on disk. - /// Defaults to 30kb. + /// Defaults to 32kb. /// /// /// The maximum amouont of bytes that the is allowed to buffer. @@ -52,24 +46,10 @@ public sealed class FileBufferingWriteStream : Stream /// uses the value returned by . /// public FileBufferingWriteStream( - Stream writeStream, int memoryThreshold = DefaultMemoryThreshold, long? bufferLimit = null, Func tempFileDirectoryAccessor = null) - : this(writeStream, memoryThreshold, bufferLimit, tempFileDirectoryAccessor, ArrayPool.Shared) { - - } - - internal FileBufferingWriteStream( - Stream writeStream, - int memoryThreshold, - long? bufferLimit, - Func tempFileDirectoryAccessor, - ArrayPool bytePool) - { - _writeStream = writeStream ?? throw new ArgumentNullException(nameof(writeStream)); - if (memoryThreshold < 0) { throw new ArgumentOutOfRangeException(nameof(memoryThreshold)); @@ -84,18 +64,7 @@ internal FileBufferingWriteStream( _memoryThreshold = memoryThreshold; _bufferLimit = bufferLimit; _tempFileDirectoryAccessor = tempFileDirectoryAccessor ?? AspNetCoreTempDirectory.TempDirectoryFactory; - _bytePool = bytePool; - - if (memoryThreshold < MaxRentedBufferSize) - { - _rentedBuffer = bytePool.Rent(memoryThreshold); - MemoryStream = new MemoryStream(_rentedBuffer); - MemoryStream.SetLength(0); - } - else - { - MemoryStream = new MemoryStream(); - } + PagedByteBuffer = new PagedByteBuffer(ArrayPool.Shared); } /// @@ -117,9 +86,9 @@ public override long Position set => throw new NotSupportedException(); } - internal long BufferedLength => MemoryStream.Length + (FileStream?.Length ?? 0); + internal long BufferedLength => PagedByteBuffer.Length + (FileStream?.Length ?? 0); - internal MemoryStream MemoryStream { get; } + internal PagedByteBuffer PagedByteBuffer { get; } internal FileStream FileStream { get; private set; } @@ -144,22 +113,27 @@ public override void Write(byte[] buffer, int offset, int count) if (_bufferLimit.HasValue && _bufferLimit - BufferedLength < count) { - DiposeInternal(); + Dispose(); throw new IOException("Buffer limit exceeded."); } - var availableMemory = _memoryThreshold - MemoryStream.Position; - if (count <= availableMemory) + // Allow buffering in memory if we're below the memory threshold once the current buffer is written. + var allowMemoryBuffer = (_memoryThreshold - count) >= PagedByteBuffer.Length; + if (allowMemoryBuffer) { // Buffer content in the MemoryStream if it has capacity. - MemoryStream.Write(buffer, offset, count); + PagedByteBuffer.Add(buffer, offset, count); + Debug.Assert(PagedByteBuffer.Length <= _memoryThreshold); } else { // If the MemoryStream is incapable of accomodating the content to be written // spool to disk. EnsureFileStream(); - CopyContent(MemoryStream, FileStream); + + // Spool memory content to disk and clear in memory buffers. We no longer need to hold on to it + PagedByteBuffer.CopyTo(FileStream, clearBuffers: true); + FileStream.Write(buffer, offset, count); } } @@ -170,36 +144,86 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc ThrowArgumentException(buffer, offset, count); ThrowIfDisposed(); - // If we have the opportunity to go async, write the buffered content to the response. - await FlushAsync(cancellationToken); - await _writeStream.WriteAsync(buffer, offset, count, cancellationToken); + if (_bufferLimit.HasValue && _bufferLimit - BufferedLength < count) + { + Dispose(); + throw new IOException("Buffer limit exceeded."); + } + + // Allow buffering in memory if we're below the memory threshold once the current buffer is written. + var allowMemoryBuffer = (_memoryThreshold - count) >= PagedByteBuffer.Length; + if (allowMemoryBuffer) + { + // Buffer content in the MemoryStream if it has capacity. + PagedByteBuffer.Add(buffer, offset, count); + Debug.Assert(PagedByteBuffer.Length <= _memoryThreshold); + } + else + { + // If the MemoryStream is incapable of accomodating the content to be written + // spool to disk. + EnsureFileStream(); + + // Spool memory content to disk and clear in memory buffers. We no longer need to hold on to it + await PagedByteBuffer.CopyToAsync(FileStream, clearBuffers: true, cancellationToken); + + await FileStream.WriteAsync(buffer, offset, count, cancellationToken); + } + } + + public override void Flush() + { + // Do nothing. } /// public override void SetLength(long value) => throw new NotSupportedException(); - /// - // In the ordinary case, we expect this to throw if the target is the HttpResponse Body - // and disallows synchronous writes. We do not need to optimize for this. - public override void Flush() + /// + /// Copies buffered content to . + /// + /// The to copy to. + /// The size of the buffer. + /// The . + /// A that represents the asynchronous copy operation. + /// + /// Users of this API do not need to reset the of this instance, prior to copying content. + /// + public override async Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { + // 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) { - CopyContent(FileStream, _writeStream); + FileStream.Position = 0; + await FileStream.CopyToAsync(destination, bufferSize, cancellationToken); } - CopyContent(MemoryStream, _writeStream); + // Copy memory content, but do not clear the buffers. We want multiple invocations of CopyTo \ CopyToAsync + // on this instance to behave the same. + await PagedByteBuffer.CopyToAsync(destination, clearBuffers: false, cancellationToken); } - /// - public override async Task FlushAsync(CancellationToken cancellationToken) + /// + /// Copies buffered content to . + /// + /// The to copy to. + /// The size of the buffer. + /// + /// Users of this API do not need to reset the of this instance, prior to copying content. + /// + public override void CopyTo(Stream destination, int bufferSize) { + // See comments under CopyToAsync for an explanation for the order of execution. if (FileStream != null) { - await CopyContentAsync(FileStream, _writeStream, cancellationToken); + FileStream.Position = 0; + FileStream.CopyTo(destination, bufferSize); } - await CopyContentAsync(MemoryStream, _writeStream, cancellationToken); + // Copy memory content, but do not clear the buffers. We want multiple invocations of CopyTo \ CopyToAsync + // on this instance to behave the same. + PagedByteBuffer.CopyTo(destination, clearBuffers: false); } /// @@ -208,39 +232,21 @@ protected override void Dispose(bool disposing) if (!Disposed) { Disposed = true; - Flush(); - DiposeInternal(); + PagedByteBuffer.Dispose(); + FileStream?.Dispose(); } } - private void DiposeInternal() - { - Disposed = true; - _bytePool.Return(_rentedBuffer); - MemoryStream.Dispose(); - FileStream?.Dispose(); - } - /// public override async ValueTask DisposeAsync() { if (!Disposed) { Disposed = true; - try - { - await FlushAsync(); - } - finally - { - if (_rentedBuffer != null) - { - _bytePool.Return(_rentedBuffer); - } - await MemoryStream.DisposeAsync(); - await (FileStream?.DisposeAsync() ?? default); - } + + PagedByteBuffer.Dispose(); + await (FileStream?.DisposeAsync() ?? default); } } @@ -290,19 +296,5 @@ private static void ThrowArgumentException(byte[] buffer, int offset, int count) throw new ArgumentOutOfRangeException(nameof(offset)); } } - - private static void CopyContent(Stream source, Stream destination) - { - source.Position = 0; - source.CopyTo(destination); - source.SetLength(0); - } - - private static async Task CopyContentAsync(Stream source, Stream destination, CancellationToken cancellationToken) - { - source.Position = 0; - await source.CopyToAsync(destination, cancellationToken); - source.SetLength(0); - } } } diff --git a/src/Http/WebUtilities/src/PagedByteBuffer.cs b/src/Http/WebUtilities/src/PagedByteBuffer.cs new file mode 100644 index 000000000000..f1e134238da5 --- /dev/null +++ b/src/Http/WebUtilities/src/PagedByteBuffer.cs @@ -0,0 +1,140 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Buffers; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.WebUtilities +{ + internal sealed class PagedByteBuffer : IDisposable + { + internal const int PageSize = 1024; + private readonly ArrayPool _arrayPool; + private byte[] _currentPage; + private int _currentPageIndex; + + public PagedByteBuffer(ArrayPool arrayPool) + { + _arrayPool = arrayPool; + Pages = new List(); + } + + public int Length { get; private set; } + + internal bool Disposed { get; private set; } + + internal List Pages { get; } + + private byte[] CurrentPage + { + get + { + if (_currentPage == null || _currentPageIndex == _currentPage.Length) + { + _currentPage = _arrayPool.Rent(PageSize); + Pages.Add(_currentPage); + _currentPageIndex = 0; + } + + return _currentPage; + } + } + + public void Add(byte[] buffer, int offset, int count) + { + ThrowIfDisposed(); + + while (count > 0) + { + var currentPage = CurrentPage; + var copyLength = Math.Min(count, currentPage.Length - _currentPageIndex); + + Buffer.BlockCopy( + buffer, + offset, + currentPage, + _currentPageIndex, + copyLength); + + Length += copyLength; + _currentPageIndex += copyLength; + offset += copyLength; + count -= copyLength; + } + } + + public void CopyTo(Stream stream, bool clearBuffers) + { + ThrowIfDisposed(); + + for (var i = 0; i < Pages.Count; i++) + { + var page = Pages[i]; + var length = (i == Pages.Count - 1) ? + _currentPageIndex : + page.Length; + + stream.Write(page, 0, length); + } + + if (clearBuffers) + { + ClearBuffers(); + } + } + + public async Task CopyToAsync(Stream stream, bool clearBuffers, 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 stream.WriteAsync(page, 0, length, cancellationToken); + } + + if (clearBuffers) + { + ClearBuffers(); + } + } + + public void Dispose() + { + if (!Disposed) + { + Disposed = true; + ClearBuffers(); + } + } + + private void ClearBuffers() + { + for (var i = 0; i < Pages.Count; i++) + { + _arrayPool.Return(Pages[i]); + } + + Pages.Clear(); + _currentPage = null; + Length = 0; + _currentPageIndex = 0; + } + + private void ThrowIfDisposed() + { + if (Disposed) + { + throw new ObjectDisposedException(nameof(PagedByteBuffer)); + } + } + } +} diff --git a/src/Http/WebUtilities/test/FileBufferingWriteTests.cs b/src/Http/WebUtilities/test/FileBufferingWriteTests.cs index 2f4186922739..86bae92bb2d0 100644 --- a/src/Http/WebUtilities/test/FileBufferingWriteTests.cs +++ b/src/Http/WebUtilities/test/FileBufferingWriteTests.cs @@ -4,12 +4,12 @@ using System; using System.Buffers; using System.IO; +using System.Linq; using System.Text; using System.Threading.Tasks; -using Moq; using Xunit; -namespace Microsoft.AspNetCore.WebUtilities.Tests +namespace Microsoft.AspNetCore.WebUtilities { public class FileBufferingWriteStreamTests : IDisposable { @@ -24,38 +24,56 @@ public FileBufferingWriteStreamTests() public void Write_BuffersContentToMemory() { // Arrange - using var writeStream = new MemoryStream(); - using var bufferingStream = new FileBufferingWriteStream(writeStream, tempFileDirectoryAccessor: () => TempDirectory); + using var bufferingStream = new FileBufferingWriteStream(tempFileDirectoryAccessor: () => TempDirectory); var input = Encoding.UTF8.GetBytes("Hello world"); // Act bufferingStream.Write(input, 0, input.Length); // Assert - // We should have written content to the MemoryStream - var memoryStream = bufferingStream.MemoryStream; - Assert.Equal(input, memoryStream.ToArray()); + // We should have written content to memory + var pagedByteBuffer = bufferingStream.PagedByteBuffer; + Assert.Equal(input, ReadBufferedContent(pagedByteBuffer)); // No files should not have been created. Assert.Null(bufferingStream.FileStream); + } + + [Fact] + public void Write_BeforeMemoryThresholdIsReached_WritesToMemory() + { + // Arrange + var input = new byte[] { 1, 2, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); - // No content should have been written to the wrapping stream - Assert.Equal(0, writeStream.Length); + // Act + bufferingStream.Write(input, 0, 2); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.Null(fileStream); + + // No content should be in the memory stream + Assert.Equal(2, pageBuffer.Length); + Assert.Equal(input, ReadBufferedContent(pageBuffer)); } [Fact] public void Write_BuffersContentToDisk_WhenMemoryThresholdIsReached() { // Arrange - using var writeStream = new MemoryStream(); - var input = new byte[] { 1, 2, 3, 4, 5 }; - using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + var input = new byte[] { 1, 2, 3, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, 2); // Act - bufferingStream.Write(input, 0, input.Length); + bufferingStream.Write(input, 2, 1); // Assert - var memoryStream = bufferingStream.MemoryStream; + var pageBuffer = bufferingStream.PagedByteBuffer; var fileStream = bufferingStream.FileStream; // File should have been created. @@ -64,173 +82,186 @@ public void Write_BuffersContentToDisk_WhenMemoryThresholdIsReached() Assert.Equal(input, fileBytes); // No content should be in the memory stream - Assert.Equal(0, memoryStream.Length); - - // No content should have been written to the wrapping stream - Assert.Equal(0, writeStream.Length); + Assert.Equal(0, pageBuffer.Length); } [Fact] - public void Write_SpoolsContentFromMemoryToDisk() + public void Write_BuffersContentToDisk_WhenWriteWillOverflowMemoryThreshold() { // Arrange - using var writeStream = new MemoryStream(); - var input = new byte[] { 1, 2, 3, 4, 5, 6, 7 }; - using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 4, tempFileDirectoryAccessor: () => TempDirectory); + var input = new byte[] { 1, 2, 3, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); // Act - bufferingStream.Write(input, 0, 3); - bufferingStream.Write(input, 3, 2); - bufferingStream.Write(input, 5, 2); + bufferingStream.Write(input, 0, input.Length); // Assert - var memoryStream = bufferingStream.MemoryStream; + var pageBuffer = bufferingStream.PagedByteBuffer; var fileStream = bufferingStream.FileStream; // File should have been created. Assert.NotNull(fileStream); var fileBytes = ReadFileContent(fileStream); - Assert.Equal(new byte[] { 1, 2, 3, 4, 5 }, fileBytes); - - Assert.Equal(new byte[] { 6, 7 }, memoryStream.ToArray()); + Assert.Equal(input, fileBytes); - // No content should have been written to the wrapping stream - Assert.Equal(0, writeStream.Length); + // No content should be in the memory stream + Assert.Equal(0, pageBuffer.Length); } [Fact] - public async Task WriteAsync_CopiesBufferedContentsFromMemoryStream() + public void Write_AfterMemoryThresholdIsReached_BuffersToMemory() { // Arrange - using var writeStream = new MemoryStream(); - using var bufferingStream = new FileBufferingWriteStream(writeStream, tempFileDirectoryAccessor: () => TempDirectory); - var input = Encoding.UTF8.GetBytes("Hello world"); + var input = new byte[] { 1, 2, 3, 4, 5, 6, 7 }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 4, tempFileDirectoryAccessor: () => TempDirectory); // Act bufferingStream.Write(input, 0, 5); - await bufferingStream.WriteAsync(input, 5, input.Length - 5); + bufferingStream.Write(input, 5, 2); // Assert - // We should have written all content to the output. - Assert.Equal(input, writeStream.ToArray()); + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(new byte[] { 1, 2, 3, 4, 5, }, fileBytes); + + Assert.Equal(new byte[] { 6, 7 }, ReadBufferedContent(pageBuffer)); } [Fact] - public async Task WriteAsync_CopiesBufferedContentsFromFileStream() + public async Task WriteAsync_BuffersContentToMemory() { // Arrange - using var writeStream = new MemoryStream(); - using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 1, tempFileDirectoryAccessor: () => TempDirectory); + using var bufferingStream = new FileBufferingWriteStream(tempFileDirectoryAccessor: () => TempDirectory); var input = Encoding.UTF8.GetBytes("Hello world"); // Act - bufferingStream.Write(input, 0, 5); - await bufferingStream.WriteAsync(input, 5, input.Length - 5); + await bufferingStream.WriteAsync(input, 0, input.Length); // Assert - // We should have written all content to the output. - Assert.Equal(0, bufferingStream.BufferedLength); - Assert.Equal(input, writeStream.ToArray()); + // We should have written content to memory + var pagedByteBuffer = bufferingStream.PagedByteBuffer; + Assert.Equal(input, ReadBufferedContent(pagedByteBuffer)); + + // No files should not have been created. + Assert.Null(bufferingStream.FileStream); } [Fact] - public async Task WriteFollowedByWriteAsync() + public async Task WriteAsync_BeforeMemoryThresholdIsReached_WritesToMemory() { // Arrange - using var writeStream = new MemoryStream(); - using var bufferingStream = new FileBufferingWriteStream(writeStream); - var input = new byte[] { 7, 8, 9 }; + var input = new byte[] { 1, 2, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); // Act - bufferingStream.Write(input, 0, 1); - await bufferingStream.WriteAsync(input, 1, 1); - bufferingStream.Write(input, 2, 1); + await bufferingStream.WriteAsync(input, 0, 2); // Assert - Assert.Equal(new byte[] { 7, 8 }, writeStream.ToArray()); - Assert.Equal(1, bufferingStream.BufferedLength); - Assert.Equal(new byte[] { 9 }, bufferingStream.MemoryStream.ToArray()); + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.Null(fileStream); + + // No content should be in the memory stream + Assert.Equal(2, pageBuffer.Length); + Assert.Equal(input, ReadBufferedContent(pageBuffer)); } [Fact] - public void Dispose_FlushesBufferedContent() + public async Task WriteAsync_BuffersContentToDisk_WhenMemoryThresholdIsReached() { // Arrange - using var writeStream = new MemoryStream(); - using var bufferingStream = new FileBufferingWriteStream(writeStream); - var input = new byte[] { 1, 2, 34 }; + var input = new byte[] { 1, 2, 3, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, 2); // Act - bufferingStream.Write(input, 0, input.Length); - bufferingStream.Dispose(); + await bufferingStream.WriteAsync(input, 2, 1); // Assert - Assert.Equal(input, writeStream.ToArray()); + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(input, fileBytes); + + // No content should be in the memory stream + Assert.Equal(0, pageBuffer.Length); } [Fact] - public void Dispose_ReturnsBuffer() + public async Task WriteAsync_BuffersContentToDisk_WhenWriteWillOverflowMemoryThreshold() { // Arrange - using var writeStream = new MemoryStream(); - var arrayPool = new Mock>(); - var bytes = new byte[10]; - arrayPool.Setup(p => p.Rent(It.IsAny())).Returns(bytes); - using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, null, tempFileDirectoryAccessor: () => TempDirectory, bytePool: arrayPool.Object); + var input = new byte[] { 1, 2, 3, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); // Act - bufferingStream.Dispose(); - bufferingStream.Dispose(); // Double disposing shouldn't be a problem + await bufferingStream.WriteAsync(input, 0, input.Length); // Assert - Assert.True(bufferingStream.Disposed); - arrayPool.Verify(v => v.Return(bytes, false), Times.Once()); + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(input, fileBytes); + + // No content should be in the memory stream + Assert.Equal(0, pageBuffer.Length); } [Fact] - public async Task DisposeAsync_FlushesBufferedContent() + public async Task WriteAsync_AfterMemoryThresholdIsReached_BuffersToMemory() { // Arrange - using var writeStream = new MemoryStream(); - using var bufferingStream = new FileBufferingWriteStream(writeStream); - var input = new byte[] { 1, 2, 34 }; + var input = new byte[] { 1, 2, 3, 4, 5, 6, 7 }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 4, tempFileDirectoryAccessor: () => TempDirectory); // Act - bufferingStream.Write(input, 0, input.Length); - await bufferingStream.DisposeAsync(); + await bufferingStream.WriteAsync(input, 0, 5); + await bufferingStream.WriteAsync(input, 5, 2); // Assert - Assert.Equal(input, writeStream.ToArray()); + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + + Assert.Equal(new byte[] { 1, 2, 3, 4, 5, }, fileBytes); + Assert.Equal(new byte[] { 6, 7 }, ReadBufferedContent(pageBuffer)); } [Fact] - public async Task DisposeAsync_ReturnsBuffer() + public void Write_Throws_IfSingleWriteExceedsBufferLimit() { // Arrange - using var writeStream = new MemoryStream(); - var arrayPool = new Mock>(); - var bytes = new byte[10]; - arrayPool.Setup(p => p.Rent(It.IsAny())).Returns(bytes); - using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, null, tempFileDirectoryAccessor: () => TempDirectory, bytePool: arrayPool.Object); + var input = new byte[20]; + var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); // Act - await bufferingStream.DisposeAsync(); - await bufferingStream.DisposeAsync(); // Double disposing shouldn't be a problem + var exception = Assert.Throws(() => bufferingStream.Write(input, 0, input.Length)); + Assert.Equal("Buffer limit exceeded.", exception.Message); - // Assert - arrayPool.Verify(v => v.Return(bytes, false), Times.Once()); + Assert.True(bufferingStream.Disposed); } [Fact] - public void Write_Throws_IfBufferLimitIsReached() + public void Write_Throws_IfWriteCumulativeWritesExceedsBuffersLimit() { // Arrange - using var writeStream = new MemoryStream(); var input = new byte[6]; - var arrayPool = new Mock>(); - arrayPool.Setup(p => p.Rent(It.IsAny())).Returns(new byte[10]); - - using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory, bytePool: arrayPool.Object); + var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); // Act bufferingStream.Write(input, 0, input.Length); @@ -238,27 +269,167 @@ public void Write_Throws_IfBufferLimitIsReached() Assert.Equal("Buffer limit exceeded.", exception.Message); // Verify we return the buffer. - arrayPool.Verify(v => v.Return(It.IsAny(), false), Times.Once()); + Assert.True(bufferingStream.Disposed); } [Fact] - public void Write_CopiesContentOnFlush() + public void Write_DoesNotThrow_IfBufferLimitIsReached() { // Arrange - using var writeStream = new MemoryStream(); - var input = new byte[6]; - var arrayPool = new Mock>(); - arrayPool.Setup(p => p.Rent(It.IsAny())).Returns(new byte[10]); - - using var bufferingStream = new FileBufferingWriteStream(writeStream, memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory, bytePool: arrayPool.Object); + var input = new byte[5]; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); // Act bufferingStream.Write(input, 0, input.Length); - var exception = Assert.Throws(() => bufferingStream.Write(input, 0, input.Length)); + bufferingStream.Write(input, 0, input.Length); // Should get to exactly the buffer limit, which is fine + + // If we got here, the test succeeded. + } + + [Fact] + public async Task WriteAsync_Throws_IfSingleWriteExceedsBufferLimit() + { + // Arrange + var input = new byte[20]; + var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + var exception = await Assert.ThrowsAsync(() => bufferingStream.WriteAsync(input, 0, input.Length)); + Assert.Equal("Buffer limit exceeded.", exception.Message); + + Assert.True(bufferingStream.Disposed); + } + + [Fact] + public async Task WriteAsync_Throws_IfWriteCumulativeWritesExceedsBuffersLimit() + { + // Arrange + var input = new byte[6]; + var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + await bufferingStream.WriteAsync(input, 0, input.Length); + var exception = await Assert.ThrowsAsync(() => bufferingStream.WriteAsync(input, 0, input.Length)); Assert.Equal("Buffer limit exceeded.", exception.Message); // Verify we return the buffer. - arrayPool.Verify(v => v.Return(It.IsAny(), false), Times.Once()); + Assert.True(bufferingStream.Disposed); + } + + [Fact] + public async Task WriteAsync_DoesNotThrow_IfBufferLimitIsReached() + { + // Arrange + var input = new byte[5]; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + await bufferingStream.WriteAsync(input, 0, input.Length); + await bufferingStream.WriteAsync(input, 0, input.Length); // Should get to exactly the buffer limit, which is fine + + // If we got here, the test succeeded. + } + + [Fact] + public void CopyTo_CopiesContentFromMemoryStream() + { + // Arrange + var input = new byte[] { 1, 2, 3, 4, 5 }; + using var bufferingStream = new FileBufferingWriteStream(tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, input.Length); + var memoryStream = new MemoryStream(); + + // Act + bufferingStream.CopyTo(memoryStream); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + [Fact] + public void CopyTo_WithContentInDisk_CopiesContentFromMemoryStream() + { + // Arrange + var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 21, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, input.Length); + var memoryStream = new MemoryStream(); + + // Act + bufferingStream.CopyTo(memoryStream); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + [Fact] + public void CopyTo_InvokedMultipleTimes_Works() + { + // Arrange + var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 21, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, input.Length); + var memoryStream1 = new MemoryStream(); + var memoryStream2 = new MemoryStream(); + + // Act + bufferingStream.CopyTo(memoryStream1); + bufferingStream.CopyTo(memoryStream2); + + // Assert + Assert.Equal(input, memoryStream1.ToArray()); + Assert.Equal(input, memoryStream2.ToArray()); + } + + [Fact] + public async Task CopyToAsync_CopiesContentFromMemoryStream() + { + // Arrange + var input = new byte[] { 1, 2, 3, 4, 5 }; + using var bufferingStream = new FileBufferingWriteStream(tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, input.Length); + var memoryStream = new MemoryStream(); + + // Act + await bufferingStream.CopyToAsync(memoryStream); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + [Fact] + public async Task CopyToAsync_WithContentInDisk_CopiesContentFromMemoryStream() + { + // Arrange + var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 21, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, input.Length); + var memoryStream = new MemoryStream(); + + // Act + await bufferingStream.CopyToAsync(memoryStream); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + [Fact] + public async Task CopyToAsync_InvokedMultipleTimes_Works() + { + // Arrange + var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 21, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, input.Length); + var memoryStream1 = new MemoryStream(); + var memoryStream2 = new MemoryStream(); + + // Act + await bufferingStream.CopyToAsync(memoryStream1); + await bufferingStream.CopyToAsync(memoryStream2); + + // Assert + Assert.Equal(input, memoryStream1.ToArray()); + Assert.Equal(input, memoryStream2.ToArray()); } public void Dispose() @@ -275,17 +446,18 @@ public void Dispose() private static byte[] ReadFileContent(FileStream fileStream) { fileStream.Position = 0; - var count = fileStream.Length; - var bytes = new ArraySegment(new byte[fileStream.Length]); - while (count > 0) - { - var read = fileStream.Read(bytes.AsSpan()); - Assert.False(read == 0, "Should not EOF before we've read the file."); - bytes = bytes.Slice(read); - count -= read; - } + using var memoryStream = new MemoryStream(); + fileStream.CopyTo(memoryStream); + + return memoryStream.ToArray(); + } + + private static byte[] ReadBufferedContent(PagedByteBuffer buffer) + { + using var memoryStream = new MemoryStream(); + buffer.CopyTo(memoryStream, clearBuffers: false); - return bytes.Array; + return memoryStream.ToArray(); } } } diff --git a/src/Http/WebUtilities/test/FormPipeReaderTests.cs b/src/Http/WebUtilities/test/FormPipeReaderTests.cs index a95a23db6344..23c3be73fd4f 100644 --- a/src/Http/WebUtilities/test/FormPipeReaderTests.cs +++ b/src/Http/WebUtilities/test/FormPipeReaderTests.cs @@ -10,7 +10,7 @@ using Microsoft.Extensions.Primitives; using Xunit; -namespace Microsoft.AspNetCore.WebUtilities.Test +namespace Microsoft.AspNetCore.WebUtilities { public class FormPipeReaderTests { diff --git a/src/Http/WebUtilities/test/HttpRequestStreamReaderTest.cs b/src/Http/WebUtilities/test/HttpRequestStreamReaderTest.cs index 062342fa4cda..6cab20bccc7f 100644 --- a/src/Http/WebUtilities/test/HttpRequestStreamReaderTest.cs +++ b/src/Http/WebUtilities/test/HttpRequestStreamReaderTest.cs @@ -11,7 +11,7 @@ using Xunit; -namespace Microsoft.AspNetCore.WebUtilities.Test +namespace Microsoft.AspNetCore.WebUtilities { public class HttpResponseStreamReaderTest { diff --git a/src/Http/WebUtilities/test/HttpResponseStreamWriterTest.cs b/src/Http/WebUtilities/test/HttpResponseStreamWriterTest.cs index 7847e1384e24..30402a5bbc71 100644 --- a/src/Http/WebUtilities/test/HttpResponseStreamWriterTest.cs +++ b/src/Http/WebUtilities/test/HttpResponseStreamWriterTest.cs @@ -11,7 +11,7 @@ using System.Threading.Tasks; using Xunit; -namespace Microsoft.AspNetCore.WebUtilities.Test +namespace Microsoft.AspNetCore.WebUtilities { public class HttpResponseStreamWriterTest { diff --git a/src/Http/WebUtilities/test/PagedByteBufferTest.cs b/src/Http/WebUtilities/test/PagedByteBufferTest.cs new file mode 100644 index 000000000000..04b1869f1b6a --- /dev/null +++ b/src/Http/WebUtilities/test/PagedByteBufferTest.cs @@ -0,0 +1,244 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Buffers; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.WebUtilities +{ + public class PagedByteBufferTest + { + [Fact] + public void Add_CreatesNewPage() + { + // Arrange + var input = Encoding.UTF8.GetBytes("Hello world"); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + + // Act + buffer.Add(input, 0, input.Length); + + // Assert + Assert.Single(buffer.Pages); + Assert.Equal(input.Length, buffer.Length); + Assert.Equal(input, ReadBufferedContent(buffer)); + } + + [Fact] + public void Add_AppendsToExistingPage() + { + // Arrange + var input1 = Encoding.UTF8.GetBytes("Hello"); + var input2 = Encoding.UTF8.GetBytes("world"); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input1, 0, input1.Length); + + // Act + buffer.Add(input2, 0, input2.Length); + + // Assert + Assert.Single(buffer.Pages); + Assert.Equal(10, buffer.Length); + Assert.Equal(Enumerable.Concat(input1, input2).ToArray(), ReadBufferedContent(buffer)); + } + + [Fact] + public void Add_WithOffsets() + { + // Arrange + var input = new byte[] { 1, 2, 3, 4, 5 }; + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + + // Act + buffer.Add(input, 1, 3); + + // Assert + Assert.Single(buffer.Pages); + Assert.Equal(3, buffer.Length); + Assert.Equal(new byte[] { 2, 3, 4 }, ReadBufferedContent(buffer)); + } + + [Fact] + public void Add_FillsUpBuffer() + { + // Arrange + var input1 = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize - 1).ToArray(); + var input2 = new byte[] { 0xca }; + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input1, 0, input1.Length); + + // Act + buffer.Add(input2, 0, 1); + + // Assert + Assert.Single(buffer.Pages); + Assert.Equal(PagedByteBuffer.PageSize, buffer.Length); + Assert.Equal(Enumerable.Concat(input1, input2).ToArray(), ReadBufferedContent(buffer)); + } + + [Fact] + public void Add_AppendsToMultiplePages() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize + 10).ToArray(); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + + // Act + buffer.Add(input, 0, input.Length); + + // Assert + Assert.Equal(2, buffer.Pages.Count); + Assert.Equal(PagedByteBuffer.PageSize + 10, buffer.Length); + Assert.Equal(input.ToArray(), ReadBufferedContent(buffer)); + } + + [Fact] + public void CopyTo_CopiesContentToStream() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input, 0, input.Length); + var stream = new MemoryStream(); + + // Act + buffer.CopyTo(stream, clearBuffers: false); + + // Assert + Assert.Equal(input, stream.ToArray()); + + // Verify copying it again works. + stream.SetLength(0); + buffer.CopyTo(stream, clearBuffers: false); + + Assert.Equal(input, stream.ToArray()); + } + + [Fact] + public async Task CopyToAsync_CopiesContentToStream() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input, 0, input.Length); + var stream = new MemoryStream(); + + // Act + await buffer.CopyToAsync(stream, clearBuffers: false, default); + + // Assert + Assert.Equal(input, stream.ToArray()); + + // Verify copying it again works. + stream.SetLength(0); + await buffer.CopyToAsync(stream, clearBuffers: false, default); + + Assert.Equal(input, stream.ToArray()); + } + + [Fact] + public async Task CopyToAsync_WithClear_ClearsBuffers() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input, 0, input.Length); + var stream = new MemoryStream(); + + // Act + await buffer.CopyToAsync(stream, clearBuffers: true, default); + + // Assert + Assert.Equal(input, stream.ToArray()); + + // Verify copying it again works. + Assert.Equal(0, buffer.Length); + Assert.False(buffer.Disposed); + Assert.Empty(buffer.Pages); + } + + [Fact] + public void CopyTo_WithClear_ReturnsBuffers() + { + // Arrange + var input = new byte[] { 1, }; + var arrayPool = new Mock>(); + var byteArray = new byte[PagedByteBuffer.PageSize]; + arrayPool.Setup(p => p.Rent(PagedByteBuffer.PageSize)) + .Returns(byteArray); + arrayPool.Setup(p => p.Return(byteArray, false)).Verifiable(); + var memoryStream = new MemoryStream(); + + using (var buffer = new PagedByteBuffer(arrayPool.Object)) + { + // Act + buffer.Add(input, 0, input.Length); + buffer.CopyTo(memoryStream, clearBuffers: true); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + arrayPool.Verify(p => p.Rent(It.IsAny()), Times.Once()); + arrayPool.Verify(p => p.Return(It.IsAny(), It.IsAny()), Times.Once()); + } + + [Fact] + public async Task CopyToAsync_WithClear_ReturnsBuffers() + { + // Arrange + var input = new byte[] { 1, }; + var arrayPool = new Mock>(); + var byteArray = new byte[PagedByteBuffer.PageSize]; + arrayPool.Setup(p => p.Rent(PagedByteBuffer.PageSize)) + .Returns(byteArray); + var memoryStream = new MemoryStream(); + + using (var buffer = new PagedByteBuffer(arrayPool.Object)) + { + // Act + buffer.Add(input, 0, input.Length); + await buffer.CopyToAsync(memoryStream, clearBuffers: true, default); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + arrayPool.Verify(p => p.Rent(It.IsAny()), Times.Once()); + arrayPool.Verify(p => p.Return(It.IsAny(), It.IsAny()), Times.Once()); + } + + [Fact] + public void Dispose_ReturnsBuffers_ExactlyOnce() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); + var arrayPool = new Mock>(); + arrayPool.Setup(p => p.Rent(PagedByteBuffer.PageSize)) + .Returns(new byte[PagedByteBuffer.PageSize]); + + var buffer = new PagedByteBuffer(arrayPool.Object); + + // Act + buffer.Add(input, 0, input.Length); + buffer.Dispose(); + buffer.Dispose(); + + arrayPool.Verify(p => p.Rent(It.IsAny()), Times.Exactly(4)); + arrayPool.Verify(p => p.Return(It.IsAny(), It.IsAny()), Times.Exactly(4)); + } + + private static byte[] ReadBufferedContent(PagedByteBuffer buffer) + { + using var stream = new MemoryStream(); + buffer.CopyTo(stream, clearBuffers: false); + + return stream.ToArray(); + } + } +} diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs index abb389aa1a20..9564017061dc 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs @@ -259,7 +259,18 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var dataContractSerializer = GetCachedSerializer(wrappingType); - var responseStream = GetResponseStream(context.HttpContext); + var httpContext = context.HttpContext; + var response = httpContext.Response; + + _mvcOptions ??= httpContext.RequestServices.GetRequiredService>().Value; + + var responseStream = response.Body; + FileBufferingWriteStream fileBufferingWriteStream = null; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + fileBufferingWriteStream = new FileBufferingWriteStream(); + responseStream = fileBufferingWriteStream; + } try { @@ -269,30 +280,27 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co { dataContractSerializer.WriteObject(xmlWriter, value); } + + // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's + // buffers. This is better than just letting dispose handle it (which would result in a synchronous + // write). + await textWriter.FlushAsync(); + } + + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.CopyToAsync(response.Body); } } finally { - if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) + if (fileBufferingWriteStream != null) { await fileBufferingWriteStream.DisposeAsync(); } } } - private Stream GetResponseStream(HttpContext httpContext) - { - _mvcOptions ??= httpContext.RequestServices.GetRequiredService>().Value; - - var responseStream = httpContext.Response.Body; - if (!_mvcOptions.SuppressOutputFormatterBuffering) - { - responseStream = new FileBufferingWriteStream(responseStream); - } - - return responseStream; - } - /// /// Gets the cached serializer or creates and caches the serializer for the given type. /// diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs index 6d11ce5701aa..a7c7c6b05324 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs @@ -235,7 +235,18 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var xmlSerializer = GetCachedSerializer(wrappingType); - var responseStream = GetResponseStream(context.HttpContext); + var httpContext = context.HttpContext; + var response = httpContext.Response; + + _mvcOptions ??= httpContext.RequestServices.GetRequiredService>().Value; + + var responseStream = response.Body; + FileBufferingWriteStream fileBufferingWriteStream = null; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + fileBufferingWriteStream = new FileBufferingWriteStream(); + responseStream = fileBufferingWriteStream; + } try { @@ -245,30 +256,27 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co { Serialize(xmlSerializer, xmlWriter, value); } + + // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's + // buffers. This is better than just letting dispose handle it (which would result in a synchronous + // write). + await textWriter.FlushAsync(); + } + + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.CopyToAsync(response.Body); } } finally { - if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) + if (fileBufferingWriteStream != null) { await fileBufferingWriteStream.DisposeAsync(); } } } - private Stream GetResponseStream(HttpContext httpContext) - { - _mvcOptions ??= httpContext.RequestServices.GetRequiredService>().Value; - - var responseStream = httpContext.Response.Body; - if (!_mvcOptions.SuppressOutputFormatterBuffering) - { - responseStream = new FileBufferingWriteStream(responseStream); - } - - return responseStream; - } - /// /// Serializes value using the passed in and . /// diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs index ec49d955af2f..dee73bf96b5d 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs @@ -113,43 +113,46 @@ public async Task ExecuteAsync(ActionContext context, JsonResult result) _logger.JsonResultExecuting(result.Value); - var responseStream = GetResponseStream(response); + var responseStream = response.Body; + Stream fileBufferingWriteStream = null; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + fileBufferingWriteStream = new FileBufferingWriteStream(); + responseStream = fileBufferingWriteStream; + } try { await using (var writer = _writerFactory.CreateWriter(responseStream, resolvedContentTypeEncoding)) { - using (var jsonWriter = new JsonTextWriter(writer)) - { - jsonWriter.ArrayPool = _charPool; - jsonWriter.CloseOutput = false; - jsonWriter.AutoCompleteOnClose = false; - - var jsonSerializer = JsonSerializer.Create(jsonSerializerSettings); - jsonSerializer.Serialize(jsonWriter, result.Value); - } + using var jsonWriter = new JsonTextWriter(writer); + jsonWriter.ArrayPool = _charPool; + jsonWriter.CloseOutput = false; + jsonWriter.AutoCompleteOnClose = false; + + var jsonSerializer = JsonSerializer.Create(jsonSerializerSettings); + jsonSerializer.Serialize(jsonWriter, result.Value); + + // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's + // buffers. This is better than just letting dispose handle it (which would result in a synchronous + // write). + await writer.FlushAsync(); + } + + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.CopyToAsync(response.Body); } } finally { - if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) + if (fileBufferingWriteStream != null) { await fileBufferingWriteStream.DisposeAsync(); } } } - private Stream GetResponseStream(HttpResponse response) - { - var responseStream = response.Body; - if (!_mvcOptions.SuppressOutputFormatterBuffering) - { - responseStream = new FileBufferingWriteStream(responseStream); - } - - return responseStream; - } - private JsonSerializerSettings GetSerializerSettings(JsonResult result) { var serializerSettings = result.SerializerSettings; diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs index c37f9f107836..d4076de44b5d 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs @@ -133,7 +133,13 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var response = context.HttpContext.Response; - var responseStream = GetResponseStream(response); + var responseStream = response.Body; + Stream fileBufferingWriteStream = null; + if (!_mvcOptions.SuppressOutputFormatterBuffering) + { + fileBufferingWriteStream = new FileBufferingWriteStream(); + responseStream = fileBufferingWriteStream; + } try { @@ -144,26 +150,24 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var jsonSerializer = CreateJsonSerializer(context); jsonSerializer.Serialize(jsonWriter, context.Object); } + + // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's + // buffers. This is better than just letting dispose handle it (which would result in a synchronous write). + await writer.FlushAsync(); + } + + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.CopyToAsync(response.Body); } } finally { - if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) + if (fileBufferingWriteStream != null) { await fileBufferingWriteStream.DisposeAsync(); } } } - - private Stream GetResponseStream(HttpResponse response) - { - var responseStream = response.Body; - if (!_mvcOptions.SuppressOutputFormatterBuffering) - { - responseStream = new FileBufferingWriteStream(responseStream); - } - - return responseStream; - } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs index 024ee7620c57..72e030ba6c80 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs @@ -163,10 +163,9 @@ public async Task ExecuteAsync_UsesPassedInSerializerSettings() } [Fact] - public async Task ExecuteAsync_ErrorDuringSerialization_DoesNotCloseTheBrackets() + public async Task ExecuteAsync_ErrorDuringSerialization_DoesNotWriteContent() { // Arrange - var expected = Encoding.UTF8.GetBytes("{\"name\":\"Robert\""); var context = GetActionContext(); var result = new JsonResult(new ModelWithSerializationError()); var executor = CreateExecutor(); @@ -184,7 +183,7 @@ public async Task ExecuteAsync_ErrorDuringSerialization_DoesNotCloseTheBrackets( // Assert var written = GetWrittenBytes(context.HttpContext); - Assert.Equal(expected, written); + Assert.Empty(written); } [Fact] diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs index 95e26022a8c0..0d839e7b6840 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs @@ -2,11 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.IO; using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNetCore.Html; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -19,9 +17,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures { - /// - /// An for . - /// public class ViewComponentResultExecutor : IActionResultExecutor { private readonly HtmlEncoder _htmlEncoder; @@ -29,46 +24,13 @@ public class ViewComponentResultExecutor : IActionResultExecutor _logger; private readonly IModelMetadataProvider _modelMetadataProvider; private readonly ITempDataDictionaryFactory _tempDataDictionaryFactory; - private readonly MvcOptions _mvcOptions; - private readonly IHttpResponseStreamWriterFactory _writerFactory; - - /// - /// Initializes a new instance of . - /// - /// Accessor for . - /// The . - /// The . - /// The . - /// The . - [Obsolete("This constructor is obsolete and weill be removed in a future version.")] - public ViewComponentResultExecutor( - IOptions mvcHelperOptions, - ILoggerFactory loggerFactory, - HtmlEncoder htmlEncoder, - IModelMetadataProvider modelMetadataProvider, - ITempDataDictionaryFactory tempDataDictionaryFactory) - : this(mvcHelperOptions, loggerFactory, htmlEncoder, modelMetadataProvider, tempDataDictionaryFactory, null, null) - { - } - /// - /// Initializes a new instance of . - /// - /// Accessor to . - /// The . - /// The . - /// The . - /// The . - /// Accessor to . - /// The . public ViewComponentResultExecutor( IOptions mvcHelperOptions, ILoggerFactory loggerFactory, HtmlEncoder htmlEncoder, IModelMetadataProvider modelMetadataProvider, - ITempDataDictionaryFactory tempDataDictionaryFactory, - IOptions mvcOptions, - IHttpResponseStreamWriterFactory writerFactory) + ITempDataDictionaryFactory tempDataDictionaryFactory) { if (mvcHelperOptions == null) { @@ -100,8 +62,6 @@ public ViewComponentResultExecutor( _htmlEncoder = htmlEncoder; _modelMetadataProvider = modelMetadataProvider; _tempDataDictionaryFactory = tempDataDictionaryFactory; - _mvcOptions = mvcOptions?.Value ?? throw new ArgumentNullException(nameof(mvcOptions)); - _writerFactory = writerFactory ?? throw new ArgumentNullException(nameof(writerFactory)); } /// @@ -145,48 +105,32 @@ public virtual async Task ExecuteAsync(ActionContext context, ViewComponentResul response.StatusCode = result.StatusCode.Value; } - var responseStream = GetResponseStream(response); - - try + // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 + var syncIOFeature = context.HttpContext.Features.Get(); + if (syncIOFeature != null) { - await using (var writer = _writerFactory.CreateWriter(responseStream, resolvedContentTypeEncoding)) - { - var viewContext = new ViewContext( - context, - NullView.Instance, - viewData, - tempData, - writer, - _htmlHelperOptions); + syncIOFeature.AllowSynchronousIO = true; + } - OnExecuting(viewContext); + using (var writer = new HttpResponseStreamWriter(response.Body, resolvedContentTypeEncoding)) + { + var viewContext = new ViewContext( + context, + NullView.Instance, + viewData, + tempData, + writer, + _htmlHelperOptions); - // IViewComponentHelper is stateful, we want to make sure to retrieve it every time we need it. - var viewComponentHelper = context.HttpContext.RequestServices.GetRequiredService(); - (viewComponentHelper as IViewContextAware)?.Contextualize(viewContext); + OnExecuting(viewContext); - var viewComponentResult = await GetViewComponentResult(viewComponentHelper, _logger, result); - viewComponentResult.WriteTo(writer, _htmlEncoder); - } - } - finally - { - if (responseStream is FileBufferingWriteStream fileBufferingWriteStream) - { - await fileBufferingWriteStream.DisposeAsync(); - } - } - } + // IViewComponentHelper is stateful, we want to make sure to retrieve it every time we need it. + var viewComponentHelper = context.HttpContext.RequestServices.GetRequiredService(); + (viewComponentHelper as IViewContextAware)?.Contextualize(viewContext); - private Stream GetResponseStream(HttpResponse response) - { - var responseStream = response.Body; - if (!_mvcOptions.SuppressOutputFormatterBuffering) - { - responseStream = new FileBufferingWriteStream(responseStream); + var viewComponentResult = await GetViewComponentResult(viewComponentHelper, _logger, result); + viewComponentResult.WriteTo(writer, _htmlEncoder); } - - return responseStream; } private void OnExecuting(ViewContext viewContext) diff --git a/src/Mvc/Mvc.ViewFeatures/test/ViewComponentResultTest.cs b/src/Mvc/Mvc.ViewFeatures/test/ViewComponentResultTest.cs index c8da99449b60..2c65acccb2ce 100644 --- a/src/Mvc/Mvc.ViewFeatures/test/ViewComponentResultTest.cs +++ b/src/Mvc/Mvc.ViewFeatures/test/ViewComponentResultTest.cs @@ -573,6 +573,7 @@ private IServiceCollection CreateServices( HttpContext context, params ViewComponentDescriptor[] descriptors) { + var httpContext = new DefaultHttpContext(); var diagnosticSource = new DiagnosticListener("Microsoft.AspNetCore"); if (diagnosticListener != null) { @@ -582,7 +583,6 @@ private IServiceCollection CreateServices( var services = new ServiceCollection(); services.AddSingleton(diagnosticSource); services.AddSingleton(); - services.AddSingleton(Options.Create(new MvcOptions())); services.AddSingleton(Options.Create(new MvcViewOptions())); services.AddTransient(); services.AddSingleton(); @@ -600,7 +600,6 @@ private IServiceCollection CreateServices( services.AddSingleton(); services.AddSingleton(); services.AddSingleton, ViewComponentResultExecutor>(); - services.AddSingleton(); return services; } From f5eae1b9bafa55a82815a0f56850f5e614f19c30 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 15 Apr 2019 20:23:28 -0700 Subject: [PATCH 3/7] Update refs --- ...t.AspNetCore.WebUtilities.netcoreapp3.0.cs | 25 +++++++++++++++++++ ...osoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs | 1 + 2 files changed, 26 insertions(+) diff --git a/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs b/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs index 921c5056231f..6f79c995c389 100644 --- a/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs +++ b/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs @@ -62,6 +62,29 @@ public override void SetLength(long value) { } public override void Write(byte[] buffer, int offset, int count) { } public override System.Threading.Tasks.Task WriteAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } } + public sealed partial class FileBufferingWriteStream : System.IO.Stream + { + public FileBufferingWriteStream(int memoryThreshold = 32768, long? bufferLimit = default(long?), System.Func tempFileDirectoryAccessor = null) { } + public override bool CanRead { get { throw null; } } + public override bool CanSeek { get { throw null; } } + public override bool CanWrite { get { throw null; } } + public override long Length { get { throw null; } } + public override long Position { get { throw null; } set { } } + public override void CopyTo(System.IO.Stream destination, int bufferSize) { } + [System.Diagnostics.DebuggerStepThroughAttribute] + public override System.Threading.Tasks.Task CopyToAsync(System.IO.Stream destination, int bufferSize, System.Threading.CancellationToken cancellationToken) { throw null; } + protected override void Dispose(bool disposing) { } + [System.Diagnostics.DebuggerStepThroughAttribute] + public override System.Threading.Tasks.ValueTask DisposeAsync() { throw null; } + public override void Flush() { } + public override int Read(byte[] buffer, int offset, int count) { throw null; } + public override System.Threading.Tasks.Task ReadAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } + public override long Seek(long offset, System.IO.SeekOrigin origin) { throw null; } + public override void SetLength(long value) { } + public override void Write(byte[] buffer, int offset, int count) { } + [System.Diagnostics.DebuggerStepThroughAttribute] + public override System.Threading.Tasks.Task WriteAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } + } public partial class FileMultipartSection { public FileMultipartSection(Microsoft.AspNetCore.WebUtilities.MultipartSection section) { } @@ -129,6 +152,8 @@ public HttpResponseStreamWriter(System.IO.Stream stream, System.Text.Encoding en public HttpResponseStreamWriter(System.IO.Stream stream, System.Text.Encoding encoding, int bufferSize, System.Buffers.ArrayPool bytePool, System.Buffers.ArrayPool charPool) { } public override System.Text.Encoding Encoding { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } protected override void Dispose(bool disposing) { } + [System.Diagnostics.DebuggerStepThroughAttribute] + public override System.Threading.Tasks.ValueTask DisposeAsync() { throw null; } public override void Flush() { } public override System.Threading.Tasks.Task FlushAsync() { throw null; } public override void Write(char value) { } diff --git a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs index 50a39b3b73aa..a1512c456a96 100644 --- a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs +++ b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs @@ -880,6 +880,7 @@ public MvcOptions() { } public int? SslPort { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public bool SuppressAsyncSuffixInActionNames { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public bool SuppressInputFormatterBuffering { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public bool SuppressOutputFormatterBuffering { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public System.Collections.Generic.IList ValueProviderFactories { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator() { throw null; } System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } From aefc38a0aa2fa1ad0d993983c914deb976e7d5d8 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 15 Apr 2019 20:49:32 -0700 Subject: [PATCH 4/7] More refs --- src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs | 1 - .../Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs b/src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs index 4070311a3b95..82c72e693ba5 100644 --- a/src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs +++ b/src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs @@ -306,7 +306,6 @@ public BindingAddress() { } } public static partial class BufferingHelper { - public static string TempDirectory { get { throw null; } } public static Microsoft.AspNetCore.Http.HttpRequest EnableRewind(this Microsoft.AspNetCore.Http.HttpRequest request, int bufferThreshold = 30720, long? bufferLimit = default(long?)) { throw null; } public static Microsoft.AspNetCore.WebUtilities.MultipartSection EnableRewind(this Microsoft.AspNetCore.WebUtilities.MultipartSection section, System.Action registerForDispose, int bufferThreshold = 30720, long? bufferLimit = default(long?)) { throw null; } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/ref/Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs b/src/Mvc/Mvc.NewtonsoftJson/ref/Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs index 066dba3dd51f..94884372e6f2 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/ref/Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/ref/Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs @@ -32,7 +32,7 @@ protected virtual void ReleaseJsonSerializer(Newtonsoft.Json.JsonSerializer seri } public partial class NewtonsoftJsonOutputFormatter : Microsoft.AspNetCore.Mvc.Formatters.TextOutputFormatter { - public NewtonsoftJsonOutputFormatter(Newtonsoft.Json.JsonSerializerSettings serializerSettings, System.Buffers.ArrayPool charPool) { } + public NewtonsoftJsonOutputFormatter(Newtonsoft.Json.JsonSerializerSettings serializerSettings, System.Buffers.ArrayPool charPool, Microsoft.AspNetCore.Mvc.MvcOptions mvcOptions) { } protected Newtonsoft.Json.JsonSerializerSettings SerializerSettings { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } protected virtual Newtonsoft.Json.JsonSerializer CreateJsonSerializer() { throw null; } protected virtual Newtonsoft.Json.JsonSerializer CreateJsonSerializer(Microsoft.AspNetCore.Mvc.Formatters.OutputFormatterWriteContext context) { throw null; } From 47ddc22c99362e0e71cec2263ba7f6c38859a0ca Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 16 Apr 2019 10:05:42 -0700 Subject: [PATCH 5/7] Changes per PR --- .../src/FileBufferingWriteStream.cs | 66 +++++---------- src/Http/WebUtilities/src/PagedByteBuffer.cs | 14 +--- ...ts.cs => FileBufferingWriteStreamTests.cs} | 80 ++----------------- .../WebUtilities/test/PagedByteBufferTest.cs | 40 +++++----- ...mlDataContractSerializerOutputFormatter.cs | 2 +- .../src/XmlSerializerOutputFormatter.cs | 2 +- .../src/JsonResultExecutor.cs | 4 +- .../src/NewtonsoftJsonOutputFormatter.cs | 4 +- 8 files changed, 55 insertions(+), 157 deletions(-) rename src/Http/WebUtilities/test/{FileBufferingWriteTests.cs => FileBufferingWriteStreamTests.cs} (82%) diff --git a/src/Http/WebUtilities/src/FileBufferingWriteStream.cs b/src/Http/WebUtilities/src/FileBufferingWriteStream.cs index c42f64871141..ee03685312cf 100644 --- a/src/Http/WebUtilities/src/FileBufferingWriteStream.cs +++ b/src/Http/WebUtilities/src/FileBufferingWriteStream.cs @@ -12,16 +12,8 @@ namespace Microsoft.AspNetCore.WebUtilities { /// - /// A that wraps another stream and buffers content to be written. - /// - /// is intended to provide a workaround for APIs that need to perform - /// synchronous writes to the HTTP Response Stream while contending with a server that is configured to disallow synchronous writes. - /// Synchronous writes are buffered to a memory backed stream up to the specified threshold, after which further writes are spooled to - /// a temporary file on disk. - /// - /// - /// Consumers of this API can invoke to copy the results to the HTTP Response Stream. - /// + /// A that buffers content to be written to disk. Use + /// to write buffered content to a target . /// public sealed class FileBufferingWriteStream : Stream { @@ -131,8 +123,8 @@ public override void Write(byte[] buffer, int offset, int count) // spool to disk. EnsureFileStream(); - // Spool memory content to disk and clear in memory buffers. We no longer need to hold on to it - PagedByteBuffer.CopyTo(FileStream, clearBuffers: true); + // Spool memory content to disk. + PagedByteBuffer.MoveTo(FileStream); FileStream.Write(buffer, offset, count); } @@ -164,66 +156,44 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc // spool to disk. EnsureFileStream(); - // Spool memory content to disk and clear in memory buffers. We no longer need to hold on to it - await PagedByteBuffer.CopyToAsync(FileStream, clearBuffers: true, cancellationToken); - + // Spool memory content to disk. + await PagedByteBuffer.MoveToAsync(FileStream, cancellationToken); await FileStream.WriteAsync(buffer, offset, count, cancellationToken); } } + /// public override void Flush() { // Do nothing. } + /// + public override Task FlushAsync(CancellationToken cancellationToken) => Task.CompletedTask; + /// public override void SetLength(long value) => throw new NotSupportedException(); /// - /// Copies buffered content to . + /// Drains buffered content to . /// - /// The to copy to. - /// The size of the buffer. + /// The to drain buffered contents to. /// The . - /// A that represents the asynchronous copy operation. - /// - /// Users of this API do not need to reset the of this instance, prior to copying content. - /// - public override async Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) + /// A that represents the asynchronous drain operation. + public async Task DrainBufferAsync(Stream 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) { FileStream.Position = 0; - await FileStream.CopyToAsync(destination, bufferSize, cancellationToken); - } - - // Copy memory content, but do not clear the buffers. We want multiple invocations of CopyTo \ CopyToAsync - // on this instance to behave the same. - await PagedByteBuffer.CopyToAsync(destination, clearBuffers: false, cancellationToken); - } + await FileStream.CopyToAsync(destination, cancellationToken); - /// - /// Copies buffered content to . - /// - /// The to copy to. - /// The size of the buffer. - /// - /// Users of this API do not need to reset the of this instance, prior to copying content. - /// - public override void CopyTo(Stream destination, int bufferSize) - { - // See comments under CopyToAsync for an explanation for the order of execution. - if (FileStream != null) - { - FileStream.Position = 0; - FileStream.CopyTo(destination, bufferSize); + await FileStream.DisposeAsync(); + FileStream = null; } - // Copy memory content, but do not clear the buffers. We want multiple invocations of CopyTo \ CopyToAsync - // on this instance to behave the same. - PagedByteBuffer.CopyTo(destination, clearBuffers: false); + await PagedByteBuffer.MoveToAsync(destination, cancellationToken); } /// diff --git a/src/Http/WebUtilities/src/PagedByteBuffer.cs b/src/Http/WebUtilities/src/PagedByteBuffer.cs index f1e134238da5..cd9c19682d36 100644 --- a/src/Http/WebUtilities/src/PagedByteBuffer.cs +++ b/src/Http/WebUtilities/src/PagedByteBuffer.cs @@ -67,7 +67,7 @@ public void Add(byte[] buffer, int offset, int count) } } - public void CopyTo(Stream stream, bool clearBuffers) + public void MoveTo(Stream stream) { ThrowIfDisposed(); @@ -81,13 +81,10 @@ public void CopyTo(Stream stream, bool clearBuffers) stream.Write(page, 0, length); } - if (clearBuffers) - { - ClearBuffers(); - } + ClearBuffers(); } - public async Task CopyToAsync(Stream stream, bool clearBuffers, CancellationToken cancellationToken) + public async Task MoveToAsync(Stream stream, CancellationToken cancellationToken) { ThrowIfDisposed(); @@ -101,10 +98,7 @@ public async Task CopyToAsync(Stream stream, bool clearBuffers, CancellationToke await stream.WriteAsync(page, 0, length, cancellationToken); } - if (clearBuffers) - { - ClearBuffers(); - } + ClearBuffers(); } public void Dispose() diff --git a/src/Http/WebUtilities/test/FileBufferingWriteTests.cs b/src/Http/WebUtilities/test/FileBufferingWriteStreamTests.cs similarity index 82% rename from src/Http/WebUtilities/test/FileBufferingWriteTests.cs rename to src/Http/WebUtilities/test/FileBufferingWriteStreamTests.cs index 86bae92bb2d0..14f0c081b5f0 100644 --- a/src/Http/WebUtilities/test/FileBufferingWriteTests.cs +++ b/src/Http/WebUtilities/test/FileBufferingWriteStreamTests.cs @@ -331,7 +331,7 @@ public async Task WriteAsync_DoesNotThrow_IfBufferLimitIsReached() } [Fact] - public void CopyTo_CopiesContentFromMemoryStream() + public async Task DrainBufferAsync_CopiesContentFromMemoryStream() { // Arrange var input = new byte[] { 1, 2, 3, 4, 5 }; @@ -340,14 +340,14 @@ public void CopyTo_CopiesContentFromMemoryStream() var memoryStream = new MemoryStream(); // Act - bufferingStream.CopyTo(memoryStream); + await bufferingStream.DrainBufferAsync(memoryStream, default); // Assert Assert.Equal(input, memoryStream.ToArray()); } [Fact] - public void CopyTo_WithContentInDisk_CopiesContentFromMemoryStream() + public async Task DrainBufferAsync_WithContentInDisk_CopiesContentFromMemoryStream() { // Arrange var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); @@ -356,82 +356,12 @@ public void CopyTo_WithContentInDisk_CopiesContentFromMemoryStream() var memoryStream = new MemoryStream(); // Act - bufferingStream.CopyTo(memoryStream); + await bufferingStream.DrainBufferAsync(memoryStream, default); // Assert Assert.Equal(input, memoryStream.ToArray()); } - [Fact] - public void CopyTo_InvokedMultipleTimes_Works() - { - // Arrange - var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); - using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 21, tempFileDirectoryAccessor: () => TempDirectory); - bufferingStream.Write(input, 0, input.Length); - var memoryStream1 = new MemoryStream(); - var memoryStream2 = new MemoryStream(); - - // Act - bufferingStream.CopyTo(memoryStream1); - bufferingStream.CopyTo(memoryStream2); - - // Assert - Assert.Equal(input, memoryStream1.ToArray()); - Assert.Equal(input, memoryStream2.ToArray()); - } - - [Fact] - public async Task CopyToAsync_CopiesContentFromMemoryStream() - { - // Arrange - var input = new byte[] { 1, 2, 3, 4, 5 }; - using var bufferingStream = new FileBufferingWriteStream(tempFileDirectoryAccessor: () => TempDirectory); - bufferingStream.Write(input, 0, input.Length); - var memoryStream = new MemoryStream(); - - // Act - await bufferingStream.CopyToAsync(memoryStream); - - // Assert - Assert.Equal(input, memoryStream.ToArray()); - } - - [Fact] - public async Task CopyToAsync_WithContentInDisk_CopiesContentFromMemoryStream() - { - // Arrange - var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); - using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 21, tempFileDirectoryAccessor: () => TempDirectory); - bufferingStream.Write(input, 0, input.Length); - var memoryStream = new MemoryStream(); - - // Act - await bufferingStream.CopyToAsync(memoryStream); - - // Assert - Assert.Equal(input, memoryStream.ToArray()); - } - - [Fact] - public async Task CopyToAsync_InvokedMultipleTimes_Works() - { - // Arrange - var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); - using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 21, tempFileDirectoryAccessor: () => TempDirectory); - bufferingStream.Write(input, 0, input.Length); - var memoryStream1 = new MemoryStream(); - var memoryStream2 = new MemoryStream(); - - // Act - await bufferingStream.CopyToAsync(memoryStream1); - await bufferingStream.CopyToAsync(memoryStream2); - - // Assert - Assert.Equal(input, memoryStream1.ToArray()); - Assert.Equal(input, memoryStream2.ToArray()); - } - public void Dispose() { try @@ -455,7 +385,7 @@ private static byte[] ReadFileContent(FileStream fileStream) private static byte[] ReadBufferedContent(PagedByteBuffer buffer) { using var memoryStream = new MemoryStream(); - buffer.CopyTo(memoryStream, clearBuffers: false); + buffer.MoveTo(memoryStream); return memoryStream.ToArray(); } diff --git a/src/Http/WebUtilities/test/PagedByteBufferTest.cs b/src/Http/WebUtilities/test/PagedByteBufferTest.cs index 04b1869f1b6a..1494584da4c6 100644 --- a/src/Http/WebUtilities/test/PagedByteBufferTest.cs +++ b/src/Http/WebUtilities/test/PagedByteBufferTest.cs @@ -98,7 +98,7 @@ public void Add_AppendsToMultiplePages() } [Fact] - public void CopyTo_CopiesContentToStream() + public void MoveTo_CopiesContentToStream() { // Arrange var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); @@ -107,20 +107,23 @@ public void CopyTo_CopiesContentToStream() var stream = new MemoryStream(); // Act - buffer.CopyTo(stream, clearBuffers: false); + buffer.MoveTo(stream); // Assert Assert.Equal(input, stream.ToArray()); - // Verify copying it again works. + // Verify moving new content works. + var newInput = Enumerable.Repeat((byte)0xcb, PagedByteBuffer.PageSize * 2 + 13).ToArray(); + buffer.Add(newInput, 0, newInput.Length); + stream.SetLength(0); - buffer.CopyTo(stream, clearBuffers: false); + buffer.MoveTo(stream); - Assert.Equal(input, stream.ToArray()); + Assert.Equal(newInput, stream.ToArray()); } [Fact] - public async Task CopyToAsync_CopiesContentToStream() + public async Task MoveToAsync_CopiesContentToStream() { // Arrange var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); @@ -129,20 +132,22 @@ public async Task CopyToAsync_CopiesContentToStream() var stream = new MemoryStream(); // Act - await buffer.CopyToAsync(stream, clearBuffers: false, default); + await buffer.MoveToAsync(stream, default); // Assert Assert.Equal(input, stream.ToArray()); - // Verify copying it again works. + // Verify adding and moving new content works. + var newInput = Enumerable.Repeat((byte)0xcb, PagedByteBuffer.PageSize * 2 + 13).ToArray(); + buffer.Add(newInput, 0, newInput.Length); stream.SetLength(0); - await buffer.CopyToAsync(stream, clearBuffers: false, default); + await buffer.MoveToAsync(stream, default); - Assert.Equal(input, stream.ToArray()); + Assert.Equal(newInput, stream.ToArray()); } [Fact] - public async Task CopyToAsync_WithClear_ClearsBuffers() + public async Task MoveToAsync_ClearsBuffers() { // Arrange var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); @@ -151,7 +156,7 @@ public async Task CopyToAsync_WithClear_ClearsBuffers() var stream = new MemoryStream(); // Act - await buffer.CopyToAsync(stream, clearBuffers: true, default); + await buffer.MoveToAsync(stream, default); // Assert Assert.Equal(input, stream.ToArray()); @@ -163,7 +168,7 @@ public async Task CopyToAsync_WithClear_ClearsBuffers() } [Fact] - public void CopyTo_WithClear_ReturnsBuffers() + public void MoveTo_WithClear_ReturnsBuffers() { // Arrange var input = new byte[] { 1, }; @@ -178,7 +183,7 @@ public void CopyTo_WithClear_ReturnsBuffers() { // Act buffer.Add(input, 0, input.Length); - buffer.CopyTo(memoryStream, clearBuffers: true); + buffer.MoveTo(memoryStream); // Assert Assert.Equal(input, memoryStream.ToArray()); @@ -189,7 +194,7 @@ public void CopyTo_WithClear_ReturnsBuffers() } [Fact] - public async Task CopyToAsync_WithClear_ReturnsBuffers() + public async Task MoveToAsync_ReturnsBuffers() { // Arrange var input = new byte[] { 1, }; @@ -203,7 +208,7 @@ public async Task CopyToAsync_WithClear_ReturnsBuffers() { // Act buffer.Add(input, 0, input.Length); - await buffer.CopyToAsync(memoryStream, clearBuffers: true, default); + await buffer.MoveToAsync(memoryStream, default); // Assert Assert.Equal(input, memoryStream.ToArray()); @@ -236,8 +241,7 @@ public void Dispose_ReturnsBuffers_ExactlyOnce() private static byte[] ReadBufferedContent(PagedByteBuffer buffer) { using var stream = new MemoryStream(); - buffer.CopyTo(stream, clearBuffers: false); - + buffer.MoveTo(stream); return stream.ToArray(); } } diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs index 9564017061dc..ffb769f7900e 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs @@ -289,7 +289,7 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co if (fileBufferingWriteStream != null) { - await fileBufferingWriteStream.CopyToAsync(response.Body); + await fileBufferingWriteStream.DrainBufferAsync(response.Body); } } finally diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs index a7c7c6b05324..35d61ac63809 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs @@ -265,7 +265,7 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co if (fileBufferingWriteStream != null) { - await fileBufferingWriteStream.CopyToAsync(response.Body); + await fileBufferingWriteStream.DrainBufferAsync(response.Body); } } finally diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs index dee73bf96b5d..e2548cba9544 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs @@ -114,7 +114,7 @@ public async Task ExecuteAsync(ActionContext context, JsonResult result) _logger.JsonResultExecuting(result.Value); var responseStream = response.Body; - Stream fileBufferingWriteStream = null; + FileBufferingWriteStream fileBufferingWriteStream = null; if (!_mvcOptions.SuppressOutputFormatterBuffering) { fileBufferingWriteStream = new FileBufferingWriteStream(); @@ -141,7 +141,7 @@ public async Task ExecuteAsync(ActionContext context, JsonResult result) if (fileBufferingWriteStream != null) { - await fileBufferingWriteStream.CopyToAsync(response.Body); + await fileBufferingWriteStream.DrainBufferAsync(response.Body); } } finally diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs index d4076de44b5d..b0852212ba65 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs @@ -134,7 +134,7 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var response = context.HttpContext.Response; var responseStream = response.Body; - Stream fileBufferingWriteStream = null; + FileBufferingWriteStream fileBufferingWriteStream = null; if (!_mvcOptions.SuppressOutputFormatterBuffering) { fileBufferingWriteStream = new FileBufferingWriteStream(); @@ -158,7 +158,7 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co if (fileBufferingWriteStream != null) { - await fileBufferingWriteStream.CopyToAsync(response.Body); + await fileBufferingWriteStream.DrainBufferAsync(response.Body); } } finally From 47f0ff03e87e5fa488c53cab71ff74a372056ffb Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 16 Apr 2019 11:02:07 -0700 Subject: [PATCH 6/7] Ref again --- .../ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs b/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs index 6f79c995c389..45d806e29783 100644 --- a/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs +++ b/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs @@ -70,13 +70,13 @@ public sealed partial class FileBufferingWriteStream : System.IO.Stream public override bool CanWrite { get { throw null; } } public override long Length { get { throw null; } } public override long Position { get { throw null; } set { } } - public override void CopyTo(System.IO.Stream destination, int bufferSize) { } - [System.Diagnostics.DebuggerStepThroughAttribute] - public override System.Threading.Tasks.Task CopyToAsync(System.IO.Stream destination, int bufferSize, System.Threading.CancellationToken cancellationToken) { throw null; } 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.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; } public override int Read(byte[] buffer, int offset, int count) { throw null; } public override System.Threading.Tasks.Task ReadAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } public override long Seek(long offset, System.IO.SeekOrigin origin) { throw null; } From d837880fb701f730205e478cd6130c1708354794 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Wed, 17 Apr 2019 12:04:12 -0700 Subject: [PATCH 7/7] Remove FlushAsync --- .../src/XmlDataContractSerializerOutputFormatter.cs | 5 ----- .../Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs | 7 +------ src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs | 5 ----- .../src/NewtonsoftJsonOutputFormatter.cs | 4 ---- 4 files changed, 1 insertion(+), 20 deletions(-) diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs index ffb769f7900e..b2a7d486b911 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs @@ -280,11 +280,6 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co { dataContractSerializer.WriteObject(xmlWriter, value); } - - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await textWriter.FlushAsync(); } if (fileBufferingWriteStream != null) diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs index 35d61ac63809..fc845f75642b 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs @@ -256,11 +256,6 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co { Serialize(xmlSerializer, xmlWriter, value); } - - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await textWriter.FlushAsync(); } if (fileBufferingWriteStream != null) @@ -307,4 +302,4 @@ protected virtual XmlSerializer GetCachedSerializer(Type type) return (XmlSerializer)serializer; } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs index e2548cba9544..9bff0e56df9c 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs @@ -132,11 +132,6 @@ public async Task ExecuteAsync(ActionContext context, JsonResult result) var jsonSerializer = JsonSerializer.Create(jsonSerializerSettings); jsonSerializer.Serialize(jsonWriter, result.Value); - - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await writer.FlushAsync(); } if (fileBufferingWriteStream != null) diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs index b0852212ba65..80a1fc3b7dee 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs @@ -150,10 +150,6 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var jsonSerializer = CreateJsonSerializer(context); jsonSerializer.Serialize(jsonWriter, context.Object); } - - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous write). - await writer.FlushAsync(); } if (fileBufferingWriteStream != null)