Fix JsonDocument thread safety. (dotnet#76716)

Co-authored-by: stoub@microsoft.com
vargaz · Oct 11, 2022 · c141b8a · c141b8a
1 parent 94d7503
commit c141b8a
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 33 deletions.
diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
@@ -27,8 +27,6 @@ public sealed partial class JsonDocument : IDisposable
         private byte[]? _extraRentedArrayPoolBytes;
         private PooledByteBufferWriter? _extraPooledByteBufferWriter;
 
-        private (int, string?) _lastIndexAndString = (-1, null);
-
         internal bool IsDisposable { get; }
 
         /// <summary>
@@ -266,14 +264,6 @@ private ReadOnlyMemory<byte> GetPropertyRawValue(int valueIndex)
         {
             CheckNotDisposed();
 
-            (int lastIdx, string? lastString) = _lastIndexAndString;
-
-            if (lastIdx == index)
-            {
-                Debug.Assert(lastString != null);
-                return lastString;
-            }
-
             DbRow row = _parsedData.Get(index);
 
             JsonTokenType tokenType = row.TokenType;
@@ -288,18 +278,9 @@ private ReadOnlyMemory<byte> GetPropertyRawValue(int valueIndex)
             ReadOnlySpan<byte> data = _utf8Json.Span;
             ReadOnlySpan<byte> segment = data.Slice(row.Location, row.SizeOrLength);
 
-            if (row.HasComplexChildren)
-            {
-                lastString = JsonReaderHelper.GetUnescapedString(segment);
-            }
-            else
-            {
-                lastString = JsonReaderHelper.TranscodeHelper(segment);
-            }
-
-            Debug.Assert(lastString != null);
-            _lastIndexAndString = (index, lastString);
-            return lastString;
+            return row.HasComplexChildren
+                ? JsonReaderHelper.GetUnescapedString(segment)
+                : JsonReaderHelper.TranscodeHelper(segment);
         }
 
         internal bool TextEquals(int index, ReadOnlySpan<char> otherText, bool isPropertyName)
@@ -308,13 +289,6 @@ internal bool TextEquals(int index, ReadOnlySpan<char> otherText, bool isPropert
 
             int matchIndex = isPropertyName ? index - DbRow.Size : index;
 
-            (int lastIdx, string? lastString) = _lastIndexAndString;
-
-            if (lastIdx == matchIndex)
-            {
-                return otherText.SequenceEqual(lastString.AsSpan());
-            }
-
             byte[]? otherUtf8TextArray = null;
 
             int length = checked(otherText.Length * JsonConstants.MaxExpansionFactorWhileTranscoding);

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonDocumentTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonDocumentTests.cs
@@ -14,6 +14,7 @@
 using System.Linq;
 using System.Runtime.InteropServices;
 using System.Threading.Tasks;
+using System.Threading;
 
 namespace System.Text.Json.Tests
 {
@@ -2744,11 +2745,9 @@ public static void ObjectEnumeratorIndependentWalk()
                     Assert.Equal(test, property.Value.GetInt32());
                     test++;
 
-                    // Subsequent read of the same JsonProperty doesn't allocate a new string
-                    // (if another property is inspected from the same document that guarantee
-                    // doesn't hold).
+                    // Subsequent read of the same JsonProperty should return an equal string
                     string propertyName2 = property.Name;
-                    Assert.Same(propertyName, propertyName2);
+                    Assert.Equal(propertyName, propertyName2);
                 }
 
                 test = 0;
@@ -3607,6 +3606,41 @@ public static void NameEquals_Empty_Throws()
             }
         }
 
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
+        [OuterLoop] // thread-safety / stress test
+        public static async Task GetString_ConcurrentUse_ThreadSafe()
+        {
+            using (JsonDocument doc = JsonDocument.Parse(SR.SimpleObjectJson))
+            {
+                JsonElement first = doc.RootElement.GetProperty("first");
+                JsonElement last = doc.RootElement.GetProperty("last");
+
+                const int Iters = 10_000;
+                using (var gate = new Barrier(2))
+                {
+                    await Task.WhenAll(
+                        Task.Factory.StartNew(() =>
+                        {
+                            gate.SignalAndWait();
+                            for (int i = 0; i < Iters; i++)
+                            {
+                                Assert.Equal("John", first.GetString());
+                                Assert.True(first.ValueEquals("John"));
+                            }
+                        }, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default),
+                        Task.Factory.StartNew(() =>
+                        {
+                            gate.SignalAndWait();
+                            for (int i = 0; i < Iters; i++)
+                            {
+                                Assert.Equal("Smith", last.GetString());
+                                Assert.True(last.ValueEquals("Smith"));
+                            }
+                        }, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default));
+                }
+            }
+        }
+
         private static void BuildSegmentedReader(
             out Utf8JsonReader reader,
             ReadOnlyMemory<byte> data,