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 dab36e4ed3ce92..a16888ed3ecbb1 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 @@ -250,7 +250,7 @@ public bool Remove(string propertyName) if (dictionary is null) { - dictionary = CreateDictionary(Options); + OrderedDictionary newDictionary = CreateDictionary(Options); if (jsonElement.HasValue) { @@ -259,14 +259,24 @@ public bool Remove(string propertyName) JsonNode? node = JsonNodeConverter.Create(jElementProperty.Value, Options); node?.Parent = this; - dictionary.Add(jElementProperty.Name, node); + newDictionary.Add(jElementProperty.Name, node); } } - // Ensure _jsonElement is written to after _dictionary - _dictionary = dictionary; - Interlocked.MemoryBarrier(); - _jsonElement = null; + // Ensure only one dictionary instance is published using CompareExchange + OrderedDictionary? exchangedDictionary = Interlocked.CompareExchange(ref _dictionary, newDictionary, null); + if (exchangedDictionary is null) + { + // We won the race and published our dictionary + // Ensure _jsonElement is written to after _dictionary + _jsonElement = null; + dictionary = newDictionary; + } + else + { + // Another thread won the race, use their dictionary + dictionary = exchangedDictionary; + } } return dictionary; 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 d24280b86c5fb0..6787c1a6f95b28 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 @@ -1789,5 +1789,26 @@ public static void Deserialize_WrongType(string json) { Assert.Throws(() => JsonSerializer.Deserialize(json)); } + + [Fact] + public static void GetPath_IsThreadSafe() + { + for (int attempt = 0; attempt < 20; attempt++) + { + var tree = (JsonNode.Parse( + """ + { + "oh": "noes" + } + """ + ) as JsonObject)!; + + Parallel.ForEach(Enumerable.Range(0, 100), new ParallelOptions { MaxDegreeOfParallelism = 16 }, _ => + { + string path = tree.First().Value!.GetPath(); + Assert.Equal("$.oh", path); + }); + } + } } }