From 3d00657ad8d1be941e82b9bfef896822afb5a965 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 6 Oct 2022 16:35:13 +0100 Subject: [PATCH] Fix JsonDocument thread safety. Co-authored-by: stoub@microsoft.com --- .../System/Text/Json/Document/JsonDocument.cs | 32 ++------------ .../JsonDocumentTests.cs | 42 +++++++++++++++++-- 2 files changed, 41 insertions(+), 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 index 2c15d1e63860a2..45fa176cd2e4db 100644 --- 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; } /// @@ -266,14 +264,6 @@ private ReadOnlyMemory 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 GetPropertyRawValue(int valueIndex) ReadOnlySpan data = _utf8Json.Span; ReadOnlySpan 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 otherText, bool isPropertyName) @@ -308,13 +289,6 @@ internal bool TextEquals(int index, ReadOnlySpan 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 index cc3d827ca729fa..9bae2532b760c8 100644 --- 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 data,