Fix Utf8JsonWriter.Dispose{Async} after failed Stream Write{Async} (dotnet/corefx...

* Fix Utf8JsonWriter.Dispose{Async} after failed Stream Write{Async}

By the time the `Utf8JsonWriter` writes to the underlying `Stream` as part of `Flush{Async}` it has already `Advance`'d the `_arrayBufferWriter`, but it hasn't reset `BytesPending` to 0.  If the write to the stream throws an exception then, we end up in an inconsistent state where `BytesPending` != 0 but there's no corresponding pending bytes to `Advance` the `_arrayBufferWriter`.  When we then `Dispose` the `Utf8JsonWriter`, we attempt to `Advance` the `_arrayBufferWriter` and fail.  Since `Utf8JsonWriter` will often be used in a `using` block, an exception from a `Write`/`Flush{Async}` call inside the `using` will then be masked as the `using`'s call to `Dispose` ends up failing and throwing a secondary exception.

* Address PR feedback

* Address PR feedback

Commit migrated from https://github.com/dotnet/corefx/commit/c38f2327e2d4e159acba06e23f38daed24661652

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs
index 329eab4..05f5879 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs
@@ -280,13 +280,16 @@ namespace System.Text.Json
         {
             CheckNotDisposed();
 
+            _memory = default;
+
             if (_stream != null)
             {
                 Debug.Assert(_arrayBufferWriter != null);
                 if (BytesPending != 0)
                 {
                     _arrayBufferWriter.Advance(BytesPending);
-                    Debug.Assert(BytesPending == _arrayBufferWriter.WrittenCount);
+                    BytesPending = 0;
+
 #if BUILDING_INBOX_LIBRARY
                     _stream.Write(_arrayBufferWriter.WrittenSpan);
 #else
@@ -296,6 +299,8 @@ namespace System.Text.Json
                     Debug.Assert(_arrayBufferWriter.WrittenCount == underlyingBuffer.Count);
                     _stream.Write(underlyingBuffer.Array, underlyingBuffer.Offset, underlyingBuffer.Count);
 #endif
+
+                    BytesCommitted += _arrayBufferWriter.WrittenCount;
                     _arrayBufferWriter.Clear();
                 }
                 _stream.Flush();
@@ -306,12 +311,10 @@ namespace System.Text.Json
                 if (BytesPending != 0)
                 {
                     _output.Advance(BytesPending);
+                    BytesCommitted += BytesPending;
+                    BytesPending = 0;
                 }
             }
-
-            _memory = default;
-            BytesCommitted += BytesPending;
-            BytesPending = 0;
         }
 
         /// <summary>
@@ -390,13 +393,16 @@ namespace System.Text.Json
         {
             CheckNotDisposed();
 
+            _memory = default;
+
             if (_stream != null)
             {
                 Debug.Assert(_arrayBufferWriter != null);
                 if (BytesPending != 0)
                 {
                     _arrayBufferWriter.Advance(BytesPending);
-                    Debug.Assert(BytesPending == _arrayBufferWriter.WrittenCount);
+                    BytesPending = 0;
+
 #if BUILDING_INBOX_LIBRARY
                     await _stream.WriteAsync(_arrayBufferWriter.WrittenMemory, cancellationToken).ConfigureAwait(false);
 #else
@@ -406,6 +412,8 @@ namespace System.Text.Json
                     Debug.Assert(_arrayBufferWriter.WrittenCount == underlyingBuffer.Count);
                     await _stream.WriteAsync(underlyingBuffer.Array, underlyingBuffer.Offset, underlyingBuffer.Count, cancellationToken).ConfigureAwait(false);
 #endif
+
+                    BytesCommitted += _arrayBufferWriter.WrittenCount;
                     _arrayBufferWriter.Clear();
                 }
                 await _stream.FlushAsync(cancellationToken).ConfigureAwait(false);
@@ -416,12 +424,10 @@ namespace System.Text.Json
                 if (BytesPending != 0)
                 {
                     _output.Advance(BytesPending);
+                    BytesCommitted += BytesPending;
+                    BytesPending = 0;
                 }
             }
-
-            _memory = default;
-            BytesCommitted += BytesPending;
-            BytesPending = 0;
         }
 
         /// <summary>
@@ -1028,13 +1034,14 @@ namespace System.Text.Json
 
             Debug.Assert(BytesPending != 0);
 
+            _memory = default;
+
             if (_stream != null)
             {
                 Debug.Assert(_arrayBufferWriter != null);
 
                 _arrayBufferWriter.Advance(BytesPending);
-
-                Debug.Assert(BytesPending == _arrayBufferWriter.WrittenCount);
+                BytesPending = 0;
 
 #if BUILDING_INBOX_LIBRARY
                 _stream.Write(_arrayBufferWriter.WrittenSpan);
@@ -1045,6 +1052,7 @@ namespace System.Text.Json
                 Debug.Assert(_arrayBufferWriter.WrittenCount == underlyingBuffer.Count);
                 _stream.Write(underlyingBuffer.Array, underlyingBuffer.Offset, underlyingBuffer.Count);
 #endif
+                BytesCommitted += _arrayBufferWriter.WrittenCount;
                 _arrayBufferWriter.Clear();
 
                 _memory = _arrayBufferWriter.GetMemory(sizeHint);
@@ -1056,6 +1064,8 @@ namespace System.Text.Json
                 Debug.Assert(_output != null);
 
                 _output.Advance(BytesPending);
+                BytesCommitted += BytesPending;
+                BytesPending = 0;
 
                 _memory = _output.GetMemory(sizeHint);
 
@@ -1064,9 +1074,6 @@ namespace System.Text.Json
                     ThrowHelper.ThrowInvalidOperationException_NeedLargerSpan();
                 }
             }
-
-            BytesCommitted += BytesPending;
-            BytesPending = 0;
         }
 
         private void FirstCallToGetMemory(int requiredSize)
@@ -1079,7 +1086,6 @@ namespace System.Text.Json
             if (_stream != null)
             {
                 Debug.Assert(_arrayBufferWriter != null);
-                Debug.Assert(BytesPending == _arrayBufferWriter.WrittenCount);
                 _memory = _arrayBufferWriter.GetMemory(sizeHint);
                 Debug.Assert(_memory.Length >= sizeHint);
             }

diff --git a/src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs b/src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs
index 5574cbf..0ea7fa5 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs
+++ b/src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs
@@ -2,17 +2,18 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using Xunit;
 using System.Buffers;
-using System.IO;
-using Newtonsoft.Json;
+using System.Collections.Generic;
+using System.Diagnostics;
 using System.Globalization;
-using System.Threading.Tasks;
+using System.IO;
 using System.IO.Pipelines;
-using System.Collections.Generic;
 using System.Text.Encodings.Web;
 using System.Text.Unicode;
-using System.Diagnostics;
+using System.Threading;
+using System.Threading.Tasks;
+using Newtonsoft.Json;
+using Xunit;
 
 namespace System.Text.Json.Tests
 {
@@ -581,6 +582,63 @@ namespace System.Text.Json.Tests
         }
 
         [Theory]
+        [InlineData(false, false)]
+        [InlineData(false, true)]
+        [InlineData(true, false)]
+        [InlineData(true, true)]
+        public async Task FlushToStreamThrows_WriterRemainsInConsistentState(bool useAsync, bool throwFromDispose)
+        {
+            var stream = new ThrowingFromWriteMemoryStream();
+            var jsonUtf8 = new Utf8JsonWriter(stream);
+
+            // Write and flush some of an object.
+            jsonUtf8.WriteStartObject();
+            jsonUtf8.WriteString("someProp1", "someValue1");
+            await jsonUtf8.FlushAsync(useAsync);
+
+            // Write some more, but fail while flushing to write to the underlying stream.
+            stream.ExceptionToThrow = new FormatException("uh oh");
+            jsonUtf8.WriteString("someProp2", "someValue2");
+            Assert.Same(stream.ExceptionToThrow, await Assert.ThrowsAsync<FormatException>(() => jsonUtf8.FlushAsync(useAsync)));
+
+            // Write some more.
+            jsonUtf8.WriteEndObject();
+
+            // Dispose, potentially throwing from the subsequent attempt to flush.
+            if (throwFromDispose)
+            {
+                // Disposing should propagate the new exception
+                stream.ExceptionToThrow = new FormatException("uh oh again");
+                Assert.Same(stream.ExceptionToThrow, await Assert.ThrowsAsync<FormatException>(() => jsonUtf8.DisposeAsync(useAsync)));
+                Assert.Equal("{\"someProp1\":\"someValue1\"", Encoding.UTF8.GetString(stream.ToArray()));
+            }
+            else
+            {
+                // Disposing should not fail.
+                stream.ExceptionToThrow = null;
+                await jsonUtf8.DisposeAsync(useAsync);
+                Assert.Equal("{\"someProp1\":\"someValue1\",\"someProp2\":\"someValue2\"}", Encoding.UTF8.GetString(stream.ToArray()));
+            }
+        }
+
+        private sealed class ThrowingFromWriteMemoryStream : MemoryStream
+        {
+            public Exception ExceptionToThrow { get; set; }
+
+            public override void Write(byte[] buffer, int offset, int count)
+            {
+                if (ExceptionToThrow != null) throw ExceptionToThrow;
+                base.Write(buffer, offset, count);
+            }
+
+            public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
+            {
+                if (ExceptionToThrow != null) throw ExceptionToThrow;
+                await base.WriteAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false);
+            }
+        }
+
+        [Theory]
         [InlineData(true, true)]
         [InlineData(true, false)]
         [InlineData(false, true)]
@@ -6589,5 +6647,29 @@ namespace System.Text.Json.Tests
 
             return sb.ToString();
         }
+
+        public static async Task FlushAsync(this Utf8JsonWriter writer, bool useAsync)
+        {
+            if (useAsync)
+            {
+                await writer.FlushAsync();
+            }
+            else
+            {
+                writer.Flush();
+            }
+        }
+
+        public static async Task DisposeAsync(this Utf8JsonWriter writer, bool useAsync)
+        {
+            if (useAsync)
+            {
+                await writer.DisposeAsync();
+            }
+            else
+            {
+                writer.Dispose();
+            }
+        }
     }
 }