From a249031c0461d5dcb4ee82d26b423ec5dfbf8aa6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Oct 2025 14:49:02 +0000 Subject: [PATCH 1/4] Initial plan From 89838b00c76e6107010f89d35e50d4c125a3cd2f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Oct 2025 15:26:53 +0000 Subject: [PATCH 2/4] Fix thread safety issue in JsonObject.InitializeDictionary using Interlocked.CompareExchange Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Text/Json/Nodes/JsonObject.IDictionary.cs | 23 ++++++++++++++----- .../JsonNode/JsonObjectTests.cs | 21 +++++++++++++++++ 2 files changed, 38 insertions(+), 6 deletions(-) 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..e43640ec528bcc 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,25 @@ 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 + Interlocked.MemoryBarrier(); + _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); + }); + } + } } } From 3d5208d77b1d7e23f237f2dc4f3a5a0d08ff6d8e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Oct 2025 16:11:31 +0000 Subject: [PATCH 3/4] Remove unnecessary Interlocked.MemoryBarrier since CompareExchange is already a full barrier Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../src/System/Text/Json/Nodes/JsonObject.IDictionary.cs | 2 -- 1 file changed, 2 deletions(-) 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 e43640ec528bcc..419bd887da6350 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 @@ -268,8 +268,6 @@ public bool Remove(string propertyName) if (exchangedDictionary is null) { // We won the race and published our dictionary - // Ensure _jsonElement is written to after _dictionary - Interlocked.MemoryBarrier(); _jsonElement = null; dictionary = newDictionary; } From e7323af6f78d70cfa929e3a731acd862cb17d445 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Sat, 11 Oct 2025 16:03:40 -0400 Subject: [PATCH 4/4] Update src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs --- .../src/System/Text/Json/Nodes/JsonObject.IDictionary.cs | 1 + 1 file changed, 1 insertion(+) 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 419bd887da6350..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 @@ -268,6 +268,7 @@ public bool Remove(string propertyName) if (exchangedDictionary is null) { // We won the race and published our dictionary + // Ensure _jsonElement is written to after _dictionary _jsonElement = null; dictionary = newDictionary; }