Add JsonSerializerOptions.MakeReadOnly(bool) overload. (#90013)

src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/JsonHelpers.cs

@@ -23,18 +23,7 @@ internal static JsonTypeInfo GetJsonTypeInfo(Type type, JsonSerializerOptions? o
             // Resolves JsonTypeInfo metadata using the appropriate JsonSerializerOptions configuration,
             // following the semantics of the JsonSerializer reflection methods.
             options ??= s_defaultSerializerOptions;
-
-            if (options.TypeInfoResolver is null)
-            {
-                // Public STJ APIs have no way of configuring TypeInfoResolver
-                // instances in a thread-safe manner. Let STJ do it for us by
-                // running a simple reflection-based serialization operation.
-                // TODO remove once https://github.com/dotnet/runtime/issues/89934 is implemented.
-                JsonSerializer.Deserialize<int>("0"u8, options);
-            }
-
-            Debug.Assert(options.TypeInfoResolver != null);
-            Debug.Assert(options.IsReadOnly);
+            options.MakeReadOnly(populateMissingResolver: true);
             return options.GetTypeInfo(type);
         }
 

src/libraries/System.Text.Json/ref/System.Text.Json.cs

@@ -402,6 +402,9 @@ public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
         public System.Text.Json.Serialization.JsonConverter GetConverter(System.Type typeToConvert) { throw null; }
         public System.Text.Json.Serialization.Metadata.JsonTypeInfo GetTypeInfo(System.Type type) { throw null; }
         public void MakeReadOnly() { }
+        [System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("Populating unconfigured TypeInfoResolver properties with the reflection resolver requires runtime code generation.")]
+        [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Populating unconfigured TypeInfoResolver properties with the reflection resolver requires unreferenced code.")]
+        public void MakeReadOnly(bool populateMissingResolver) { }
         public bool TryGetTypeInfo(System.Type type, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Text.Json.Serialization.Metadata.JsonTypeInfo? typeInfo) { throw null; }
     }
     public enum JsonTokenType : byte

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs

@@ -34,11 +34,7 @@ private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type inp
             Debug.Assert(inputType != null);
 
             options ??= JsonSerializerOptions.Default;
-
-            if (!options.IsConfiguredForJsonSerializer)
-            {
-                options.ConfigureForJsonSerializer();
-            }
+            options.MakeReadOnly(populateMissingResolver: true);
 
             // In order to improve performance of polymorphic root-level object serialization,
             // we bypass GetTypeInfoForRootType and cache JsonTypeInfo<object> in a dedicated property.

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs

@@ -45,11 +45,14 @@ public JsonConverter GetConverter(Type typeToConvert)
                 ThrowHelper.ThrowArgumentNullException(nameof(typeToConvert));
             }
 
-            if (JsonSerializer.IsReflectionEnabledByDefault && _typeInfoResolver is null)
+            if (JsonSerializer.IsReflectionEnabledByDefault)
             {
                 // Backward compatibility -- root & query the default reflection converters
                 // but do not populate the TypeInfoResolver setting.
-                return DefaultJsonTypeInfoResolver.GetConverterForType(typeToConvert, this);
+                if (_typeInfoResolver is null)
+                {
+                    return DefaultJsonTypeInfoResolver.GetConverterForType(typeToConvert, this);
+                }
             }
 
             return GetConverterInternal(typeToConvert);

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs

@@ -76,8 +76,6 @@ public static JsonSerializerOptions Default
         private bool _propertyNameCaseInsensitive;
         private bool _writeIndented;
 
-        private volatile bool _isReadOnly;
-
         /// <summary>
         /// Constructs a new <see cref="JsonSerializerOptions"/> instance.
         /// </summary>
@@ -680,18 +678,21 @@ internal bool CanUseFastPathSerializationLogic
         internal ReferenceHandlingStrategy ReferenceHandlingStrategy = ReferenceHandlingStrategy.None;
 
         /// <summary>
-        /// Specifies whether the current instance has been locked for modification.
+        /// Specifies whether the current instance has been locked for user modification.
         /// </summary>
         /// <remarks>
         /// A <see cref="JsonSerializerOptions"/> instance can be locked either if
         /// it has been passed to one of the <see cref="JsonSerializer"/> methods,
         /// has been associated with a <see cref="JsonSerializerContext"/> instance,
-        /// or a user explicitly called the <see cref="MakeReadOnly"/> method on the instance.
+        /// or a user explicitly called the <see cref="MakeReadOnly()"/> methods on the instance.
+        ///
+        /// Read-only instances use caching when querying <see cref="JsonConverter"/> and <see cref="JsonTypeInfo"/> metadata.
         /// </remarks>
         public bool IsReadOnly => _isReadOnly;
+        private volatile bool _isReadOnly;
 
         /// <summary>
-        /// Locks the current instance for further modification.
+        /// Marks the current instance as read-only preventing any further user modification.
         /// </summary>
         /// <exception cref="InvalidOperationException">The instance does not specify a <see cref="TypeInfoResolver"/> setting.</exception>
         /// <remarks>This method is idempotent.</remarks>
@@ -706,11 +707,45 @@ public void MakeReadOnly()
         }
 
         /// <summary>
-        /// Configures the instance for use by the JsonSerializer APIs.
+        /// Marks the current instance as read-only preventing any further user modification.
+        /// </summary>
+        /// <param name="populateMissingResolver">Populates unconfigured <see cref="TypeInfoResolver"/> properties with the reflection-based default.</param>
+        /// <exception cref="InvalidOperationException">
+        /// The instance does not specify a <see cref="TypeInfoResolver"/> setting. Thrown if <paramref name="populateMissingResolver"/> is <see langword="false"/>.
+        /// -OR-
+        /// The <see cref="JsonSerializer.IsReflectionEnabledByDefault"/> feature switch has been turned off.
+        /// </exception>
+        /// <remarks>
+        /// When <paramref name="populateMissingResolver"/> is set to <see langword="true" />, configures the instance following
+        /// the semantics of the <see cref="JsonSerializer"/> methods accepting <see cref="JsonSerializerOptions"/> parameters.
+        ///
+        /// This method is idempotent.
+        /// </remarks>
+        [RequiresUnreferencedCode("Populating unconfigured TypeInfoResolver properties with the reflection resolver requires unreferenced code.")]
+        [RequiresDynamicCode("Populating unconfigured TypeInfoResolver properties with the reflection resolver requires runtime code generation.")]
+        public void MakeReadOnly(bool populateMissingResolver)
+        {
+            if (populateMissingResolver)
+            {
+                if (!_isConfiguredForJsonSerializer)
+                {
+                    ConfigureForJsonSerializer();
+                }
+            }
+            else
+            {
+                MakeReadOnly();
+            }
+
+            Debug.Assert(IsReadOnly);
+        }
+
+        /// <summary>
+        /// Configures the instance for use by the JsonSerializer APIs, applying reflection-based fallback where applicable.
         /// </summary>
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
         [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
-        internal void ConfigureForJsonSerializer()
+        private void ConfigureForJsonSerializer()
         {
             if (JsonSerializer.IsReflectionEnabledByDefault)
             {
@@ -734,7 +769,7 @@ internal void ConfigureForJsonSerializer()
                             // A cache has already been created by the source generator.
                             // Repeat the same configuration routine for that options instance, if different.
                             // Invalidate any cache entries that have already been stored.
-                            if (cachingContext.Options != this)
+                            if (cachingContext.Options != this && !cachingContext.Options._isConfiguredForJsonSerializer)
                             {
                                 cachingContext.Options.ConfigureForJsonSerializer();
                             }
@@ -751,11 +786,18 @@ internal void ConfigureForJsonSerializer()
                 ThrowHelper.ThrowInvalidOperationException_JsonSerializerIsReflectionDisabled();
             }
 
-            MakeReadOnly();
+            Debug.Assert(_typeInfoResolver != null);
+            // NB preserve write order.
+            _isReadOnly = true;
             _isConfiguredForJsonSerializer = true;
         }
 
-        internal bool IsConfiguredForJsonSerializer => _isConfiguredForJsonSerializer;
+        /// <summary>
+        /// This flag is supplementary to <see cref="_isReadOnly"/> and is only used to keep track
+        /// of source-gen reflection fallback (assuming the IsSourceGenReflectionFallbackEnabled feature switch is on).
+        /// This mode necessitates running the <see cref="ConfigureForJsonSerializer"/> method even
+        /// for options instances that have been marked as read-only.
+        /// </summary>
         private volatile bool _isConfiguredForJsonSerializer;
 
         // Only populated in .NET 6 compatibility mode encoding reflection fallback in source gen

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs

@@ -176,6 +176,57 @@ public static void NewDefaultOptions_MakeReadOnly_NoTypeInfoResolver_ThrowsInval
             Assert.True(options.IsReadOnly);
         }
 
+        [Fact]
+        public static void NewDefaultOptions_MakeReadOnlyWithPopulateParameterFalse_NoTypeInfoResolver_ThrowsInvalidOperation()
+        {
+            var options = new JsonSerializerOptions();
+            Assert.False(options.IsReadOnly);
+
+            Assert.Throws<InvalidOperationException>(() => options.MakeReadOnly(populateMissingResolver: false));
+            Assert.Null(options.TypeInfoResolver);
+            Assert.False(options.IsReadOnly);
+        }
+
+        [Fact]
+        public static void NewDefaultOptions_MakeReadOnlyWithPopulateParameterTrue_NoTypeInfoResolver_UsesDefaultTypeInfoResolver()
+        {
+            var options = new JsonSerializerOptions();
+            Assert.Null(options.TypeInfoResolver);
+            Assert.False(options.IsReadOnly);
+
+            options.MakeReadOnly(populateMissingResolver: true);
+
+            DefaultJsonTypeInfoResolver resolver = Assert.IsType<DefaultJsonTypeInfoResolver>(options.TypeInfoResolver);
+            Assert.True(options.IsReadOnly);
+
+            // Method is idempotent
+            options.MakeReadOnly(populateMissingResolver: false);
+            Assert.True(options.IsReadOnly);
+            Assert.Same(resolver, options.TypeInfoResolver);
+
+            options.MakeReadOnly(populateMissingResolver: true);
+            Assert.True(options.IsReadOnly);
+            Assert.Same(resolver, options.TypeInfoResolver);
+
+            options.MakeReadOnly();
+            Assert.True(options.IsReadOnly);
+            Assert.Same(resolver, options.TypeInfoResolver);
+        }
+
+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public static void NewDefaultOptions_MakeReadOnlyWithPopulateParameter_HasTypeInfoResolver_DoesNotReplaceResolver(bool populateMissingResolver)
+        {
+            var options = new JsonSerializerOptions { TypeInfoResolver = JsonContext.Default };
+            Assert.False(options.IsReadOnly);
+
+            options.MakeReadOnly(populateMissingResolver);
+
+            Assert.True(options.IsReadOnly);
+            Assert.Same(JsonContext.Default, options.TypeInfoResolver);
+        }
+
         [Fact]
         public static void TypeInfoResolverCannotBeSetAfterAddingContext()
         {
@@ -565,10 +616,15 @@ public static void Options_DisablingIsReflectionEnabledByDefaultSwitch_NewOption
                 ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<string>("\"string\"", options));
                 Assert.Contains("JsonSerializerOptions.TypeInfoResolver", ex.Message);
 
+                ex = Assert.Throws<InvalidOperationException>(() => options.MakeReadOnly(populateMissingResolver: true));
+                Assert.Contains("JsonSerializerOptions.TypeInfoResolver", ex.Message);
+
                 Assert.False(options.TryGetTypeInfo(typeof(string), out JsonTypeInfo? typeInfo));
                 Assert.Null(typeInfo);
 
                 Assert.False(options.IsReadOnly); // failed operations should not lock the instance
+                Assert.Empty(options.TypeInfoResolverChain);
+                Assert.Null(options.TypeInfoResolver);
 
                 // Can still use reflection via explicit configuration
                 options.TypeInfoResolver = new DefaultJsonTypeInfoResolver();
@@ -973,7 +1029,11 @@ public static void JsonSerializerOptions_Default_IsReadOnly()
             Assert.Throws<InvalidOperationException>(() => resolver.Modifiers.Add(ti => { }));
             Assert.Throws<InvalidOperationException>(() => resolver.Modifiers.Insert(0, ti => { }));
 
-            optionsSingleton.MakeReadOnly(); // MakeReadOnly is idempotent.
+            // MakeReadOnly is idempotent.
+            optionsSingleton.MakeReadOnly();
+            optionsSingleton.MakeReadOnly(populateMissingResolver: false);
+            optionsSingleton.MakeReadOnly(populateMissingResolver: true);
+            Assert.Same(resolver, optionsSingleton.TypeInfoResolver);
         }
 
         [Theory]

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs

@@ -44,18 +44,41 @@ public static int Main()
         {
         }
 
+        // Calling GetConverter should throw NotSupportedException.
+        try
+        {
+            _ = options.GetConverter(typeof(MyPoco));
+            return -4;
+        }
+        catch (NotSupportedException)
+        {
+        }
+
+        // Calling MakeReadOnly(populateMissingResolver: true) should throw InvalidOperationException.
+        try
+        {
+            options.MakeReadOnly(populateMissingResolver: true);
+            return -5;
+        }
+        catch (InvalidOperationException)
+        {
+        }
+
         // Serializing with a custom resolver should work as expected.
         options.TypeInfoResolver = new MyJsonResolver();
+        options.MakeReadOnly(populateMissingResolver: true);
+        options.GetConverter(typeof(MyPoco));
+
         if (JsonSerializer.Serialize(valueToSerialize, options) != "{\"Value\":42}")
         {
-            return -4;
+            return -6;
         }
 
         // The Default resolver should have been trimmed from the application.
         Type? reflectionResolver = GetJsonType("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver");
         if (reflectionResolver != null)
         {
-            return -5;
+            return -7;
         }
 
         return 100;