diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs index bcd88d9bc6f1d..ddcad35a9e34a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization.Converters; +using System.Threading; namespace System.Text.Json.Nodes { @@ -85,7 +86,7 @@ private void InitializeFromArray(JsonNode?[] items) throw new InvalidOperationException(SR.Format(SR.NodeElementWrongType, nameof(JsonValueKind.Array))); } - internal JsonArray (JsonElement element, JsonNodeOptions? options = null) : base(options) + internal JsonArray(JsonElement element, JsonNodeOptions? options = null) : base(options) { Debug.Assert(element.ValueKind == JsonValueKind.Array); _jsonElement = element; @@ -180,33 +181,47 @@ public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? optio private void CreateNodes() { - if (_list == null) + if (_list is null) { - List list; + CreateNodesCore(); + } - if (_jsonElement == null) - { - list = new List(); - } - else - { - JsonElement jElement = _jsonElement.Value; - Debug.Assert(jElement.ValueKind == JsonValueKind.Array); + void CreateNodesCore() + { + // Even though _list initialization can be subject to races, + // ensure that contending threads use a coherent view of jsonElement. + + // Because JsonElement cannot be read atomically there might be torn reads, + // however the order of read/write operations guarantees that that's only + // possible if the value of _list is non-null. + JsonElement? jsonElement = _jsonElement; + Interlocked.MemoryBarrier(); + List? list = _list; - list = new List(jElement.GetArrayLength()); + if (list is null) + { + list = new(); - foreach (JsonElement element in jElement.EnumerateArray()) + if (jsonElement.HasValue) { - JsonNode? node = JsonNodeConverter.Create(element, Options); - node?.AssignParent(this); - list.Add(node); + JsonElement jElement = jsonElement.Value; + Debug.Assert(jElement.ValueKind == JsonValueKind.Array); + + list = new List(jElement.GetArrayLength()); + + foreach (JsonElement element in jElement.EnumerateArray()) + { + JsonNode? node = JsonNodeConverter.Create(element, Options); + node?.AssignParent(this); + list.Add(node); + } } - // Clear since no longer needed. + // Ensure _jsonElement is written to after _list + _list = list; + Interlocked.MemoryBarrier(); _jsonElement = null; } - - _list = list; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs index bf843dac3788a..e298b3ad6bd1f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Text.Json.Serialization.Converters; +using System.Threading; namespace System.Text.Json.Nodes { @@ -261,32 +262,47 @@ IEnumerator IEnumerable.GetEnumerator() private void InitializeIfRequired() { - if (_dictionary != null) + if (_dictionary is null) { - return; + InitializeCore(); } - bool caseInsensitive = Options.HasValue ? Options.Value.PropertyNameCaseInsensitive : false; - var dictionary = new JsonPropertyDictionary(caseInsensitive); - if (_jsonElement.HasValue) + void InitializeCore() { - JsonElement jElement = _jsonElement.Value; + // Even though _dictionary initialization can be subject to races, + // ensure that contending threads use a coherent view of jsonElement. + + // Because JsonElement cannot be read atomically there might be torn reads, + // however the order of read/write operations guarantees that that's only + // possible if the value of _dictionary is non-null. + JsonElement? jsonElement = _jsonElement; + Interlocked.MemoryBarrier(); + JsonPropertyDictionary? dictionary = _dictionary; - foreach (JsonProperty jElementProperty in jElement.EnumerateObject()) + if (dictionary is null) { - JsonNode? node = JsonNodeConverter.Create(jElementProperty.Value, Options); - if (node != null) + bool caseInsensitive = Options.HasValue ? Options.Value.PropertyNameCaseInsensitive : false; + dictionary = new JsonPropertyDictionary(caseInsensitive); + if (jsonElement.HasValue) { - node.Parent = this; + foreach (JsonProperty jElementProperty in jsonElement.Value.EnumerateObject()) + { + JsonNode? node = JsonNodeConverter.Create(jElementProperty.Value, Options); + if (node != null) + { + node.Parent = this; + } + + dictionary.Add(new KeyValuePair(jElementProperty.Name, node)); + } } - dictionary.Add(new KeyValuePair(jElementProperty.Name, node)); + // Ensure _jsonElement is written to after _dictionary + _dictionary = dictionary; + Interlocked.MemoryBarrier(); + _jsonElement = null; } - - _jsonElement = null; } - - _dictionary = dictionary; } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs index 68abbe1fd1657..9e32f3298d462 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading.Tasks; using Xunit; namespace System.Text.Json.Nodes.Tests @@ -473,5 +474,17 @@ public static void GetJsonArrayIEnumerable() Assert.True(jArrayEnumerator.MoveNext()); Assert.Equal("value", ((JsonValue)jArrayEnumerator.Current).GetValue()); } + + [Fact] + public static void LazyInitializationIsThreadSafe() + { + string arrayText = "[\"elem0\",\"elem1\"]"; + JsonArray node = Assert.IsType(JsonNode.Parse(arrayText)); + Parallel.For(0, 128, i => + { + Assert.Equal("elem0", (string)node[0]); + Assert.Equal("elem1", (string)node[1]); + }); + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs index f6a22aff6e819..8d6332e25e974 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs @@ -6,6 +6,7 @@ using System.IO; using System.Linq; using System.Text.Json.Serialization.Tests; +using System.Threading.Tasks; using Xunit; namespace System.Text.Json.Nodes.Tests @@ -956,5 +957,17 @@ static void Test(JsonObject jObject) Assert.Equal("World", (string)jObject["hello"]); // Test case insensitivity } } + + [Fact] + public static void LazyInitializationIsThreadSafe() + { + string arrayText = "{\"prop0\":0,\"prop1\":1}"; + JsonObject jObj = Assert.IsType(JsonNode.Parse(arrayText)); + Parallel.For(0, 128, i => + { + Assert.Equal(0, (int)jObj["prop0"]); + Assert.Equal(1, (int)jObj["prop1"]); + }); + } } }