Fix race condition when accessing nested JsonTypeInfo metadata (#81821)
authorEirik Tsarpalis <eirik.tsarpalis@gmail.com>
Wed, 8 Feb 2023 14:10:17 +0000 (14:10 +0000)
committerGitHub <noreply@github.com>
Wed, 8 Feb 2023 14:10:17 +0000 (14:10 +0000)
* Fix race condition when accessing nested JsonTypeInfo metadata

* Improve wording in comments.

* Remove DEBUG conditional

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPolymorphismOptions.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs

index e19ea7c..b4c225c 100644 (file)
@@ -94,7 +94,7 @@ namespace System.Text.Json.Serialization.Metadata
                 _parent = parent;
             }
 
-            protected override bool IsImmutable => _parent.DeclaringTypeInfo?.IsConfigured == true;
+            protected override bool IsImmutable => _parent.DeclaringTypeInfo?.IsReadOnly == true;
             protected override void VerifyMutable() => _parent.DeclaringTypeInfo?.VerifyMutable();
         }
 
index 5eb7539..57bd626 100644 (file)
@@ -791,8 +791,14 @@ namespace System.Text.Json.Serialization.Metadata
             get
             {
                 Debug.Assert(IsConfigured);
-                Debug.Assert(_jsonTypeInfo?.IsConfigured == true);
-                return _jsonTypeInfo;
+                Debug.Assert(_jsonTypeInfo?.IsConfigurationStarted == true);
+                // Even though this instance has already been configured,
+                // it is possible for contending threads to call the property
+                // while the wider JsonTypeInfo graph is still being configured.
+                // Call EnsureConfigured() to force synchronization if necessary.
+                JsonTypeInfo jsonTypeInfo = _jsonTypeInfo;
+                jsonTypeInfo.EnsureConfigured();
+                return jsonTypeInfo;
             }
             set
             {
index 251d64e..397d5c6 100644 (file)
@@ -322,7 +322,14 @@ namespace System.Text.Json.Serialization.Metadata
             get
             {
                 Debug.Assert(IsConfigured);
-                return _elementTypeInfo;
+                Debug.Assert(_elementTypeInfo is null or { IsConfigurationStarted: true });
+                // Even though this instance has already been configured,
+                // it is possible for contending threads to call the property
+                // while the wider JsonTypeInfo graph is still being configured.
+                // Call EnsureConfigured() to force synchronization if necessary.
+                JsonTypeInfo? elementTypeInfo = _elementTypeInfo;
+                elementTypeInfo?.EnsureConfigured();
+                return elementTypeInfo;
             }
             set
             {
@@ -340,7 +347,14 @@ namespace System.Text.Json.Serialization.Metadata
             get
             {
                 Debug.Assert(IsConfigured);
-                return _keyTypeInfo;
+                Debug.Assert(_keyTypeInfo is null or { IsConfigurationStarted: true });
+                // Even though this instance has already been configured,
+                // it is possible for contending threads to call the property
+                // while the wider JsonTypeInfo graph is still being configured.
+                // Call EnsureConfigured() to force synchronization if necessary.
+                JsonTypeInfo? keyTypeInfo = _keyTypeInfo;
+                keyTypeInfo?.EnsureConfigured();
+                return keyTypeInfo;
             }
             set
             {
@@ -491,6 +505,7 @@ namespace System.Text.Json.Serialization.Metadata
         }
 
         internal bool IsConfigured => _configurationState == ConfigurationState.Configured;
+        internal bool IsConfigurationStarted => _configurationState is not ConfigurationState.NotConfigured;
         private volatile ConfigurationState _configurationState;
         private enum ConfigurationState : byte { NotConfigured = 0, Configuring = 1, Configured = 2 };