Fix issues related to JsonSerializerOptions mutation and caching. (#6…

…5863)

* Fix issues related to JsonSerializerOptions mutation and caching.

* fix test style

* fix linker warning

src/libraries/System.Text.Json/src/Resources/Strings.resx

@@ -569,9 +569,6 @@
   <data name="SerializerContextOptionsImmutable" xml:space="preserve">
     <value>A 'JsonSerializerOptions' instance associated with a 'JsonSerializerContext' instance cannot be mutated once the context has been instantiated.</value>
   </data>
-  <data name="OptionsAlreadyBoundToContext" xml:space="preserve">
-    <value>"The specified 'JsonSerializerOptions' instance is already bound with a 'JsonSerializerContext' instance."</value>
-  </data>
   <data name="ConverterForPropertyMustBeValid" xml:space="preserve">
     <value>The generic type of the converter for property '{0}.{1}' must match with the specified converter type '{2}'. The converter must not be 'null'.</value>
   </data>

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

@@ -76,7 +76,7 @@ internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializer
             if (!state.SupportContinuation &&
                 jsonTypeInfo is JsonTypeInfo<T> info &&
                 info.SerializeHandler != null &&
-                info.Options._serializerContext?.CanUseSerializationLogic == true)
+                info.Options.JsonSerializerContext?.CanUseSerializationLogic == true)
             {
                 info.SerializeHandler(writer, value);
                 return true;

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

@@ -18,9 +18,9 @@ private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type run
             Debug.Assert(runtimeType != null);
 
             options ??= JsonSerializerOptions.Default;
-            if (!JsonSerializerOptions.IsInitializedForReflectionSerializer)
+            if (!options.IsInitializedForReflectionSerializer)
             {
-                JsonSerializerOptions.InitializeForReflectionSerializer();
+                options.InitializeForReflectionSerializer();
             }
 
             return options.GetOrAddJsonTypeInfoForRootType(runtimeType);

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs

@@ -287,9 +287,9 @@ public static partial class JsonSerializer
             CancellationToken cancellationToken = default)
         {
             options ??= JsonSerializerOptions.Default;
-            if (!JsonSerializerOptions.IsInitializedForReflectionSerializer)
+            if (!options.IsInitializedForReflectionSerializer)
             {
-                JsonSerializerOptions.InitializeForReflectionSerializer();
+                options.InitializeForReflectionSerializer();
             }
 
             return CreateAsyncEnumerableDeserializer(utf8Json, options, cancellationToken);

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

@@ -41,7 +41,7 @@ private static void WriteUsingGeneratedSerializer<TValue>(Utf8JsonWriter writer,
 
             if (jsonTypeInfo.HasSerialize &&
                 jsonTypeInfo is JsonTypeInfo<TValue> typedInfo &&
-                typedInfo.Options._serializerContext?.CanUseSerializationLogic == true)
+                typedInfo.Options.JsonSerializerContext?.CanUseSerializationLogic == true)
             {
                 Debug.Assert(typedInfo.SerializeHandler != null);
                 typedInfo.SerializeHandler(writer, value);
@@ -59,8 +59,8 @@ private static void WriteUsingSerializer<TValue>(Utf8JsonWriter writer, in TValu
 
             Debug.Assert(!jsonTypeInfo.HasSerialize ||
                 jsonTypeInfo is not JsonTypeInfo<TValue> ||
-                jsonTypeInfo.Options._serializerContext == null ||
-                !jsonTypeInfo.Options._serializerContext.CanUseSerializationLogic,
+                jsonTypeInfo.Options.JsonSerializerContext == null ||
+                !jsonTypeInfo.Options.JsonSerializerContext.CanUseSerializationLogic,
                 "Incorrect method called. WriteUsingGeneratedSerializer() should have been called instead.");
 
             WriteStack state = default;

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

@@ -21,19 +21,7 @@ public abstract partial class JsonSerializerContext
         /// <remarks>
         /// The instance cannot be mutated once it is bound with the context instance.
         /// </remarks>
-        public JsonSerializerOptions Options
-        {
-            get
-            {
-                if (_options == null)
-                {
-                    _options = new JsonSerializerOptions();
-                    _options._serializerContext = this;
-                }
-
-                return _options;
-            }
-        }
+        public JsonSerializerOptions Options => _options ??= new JsonSerializerOptions { JsonSerializerContext = this };
 
         /// <summary>
         /// Indicates whether pre-generated serialization logic for types in the context
@@ -95,13 +83,8 @@ protected JsonSerializerContext(JsonSerializerOptions? options)
         {
             if (options != null)
             {
-                if (options._serializerContext != null)
-                {
-                    ThrowHelper.ThrowInvalidOperationException_JsonSerializerOptionsAlreadyBoundToContext();
-                }
-
+                options.JsonSerializerContext = this;
                 _options = options;
-                options._serializerContext = this;
             }
         }
 

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

@@ -74,6 +74,10 @@ internal void ClearCaches()
         private void InitializeCachingContext()
         {
             _cachingContext = TrackedCachingContexts.GetOrCreate(this);
+            if (IsInitializedForReflectionSerializer)
+            {
+                _cachingContext.Options.IsInitializedForReflectionSerializer = true;
+            }
         }
 
         /// <summary>
@@ -159,7 +163,12 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options)
 
                     // Use a defensive copy of the options instance as key to
                     // avoid capturing references to any caching contexts.
-                    var key = new JsonSerializerOptions(options) { _serializerContext = options._serializerContext };
+                    var key = new JsonSerializerOptions(options)
+                    {
+                        // Copy fields ignored by the copy constructor
+                        // but are necessary to determine equivalence.
+                        _serializerContext = options._serializerContext,
+                    };
                     Debug.Assert(key._cachingContext == null);
 
                     ctx = new CachingContext(options);

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

@@ -23,11 +23,27 @@ public sealed partial class JsonSerializerOptions
         // The global list of built-in converters that override CanConvert().
         private static JsonConverter[]? s_defaultFactoryConverters;
 
+        // Stores the JsonTypeInfo factory, which requires unreferenced code and must be rooted by the reflection-based serializer.
+        private static Func<Type, JsonSerializerOptions, JsonTypeInfo>? s_typeInfoCreationFunc;
+
+        [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
+        private static void RootReflectionSerializerDependencies()
+        {
+            if (s_defaultSimpleConverters is null)
+            {
+                s_defaultSimpleConverters = GetDefaultSimpleConverters();
+                s_defaultFactoryConverters = GetDefaultFactoryConverters();
+                s_typeInfoCreationFunc = CreateJsonTypeInfo;
+            }
+
+            [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
+            static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options) => new JsonTypeInfo(type, options);
+        }
+
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
-        private static void RootBuiltInConverters()
+        private static JsonConverter[] GetDefaultFactoryConverters()
         {
-            s_defaultSimpleConverters = GetDefaultSimpleConverters();
-            s_defaultFactoryConverters = new JsonConverter[]
+            return new JsonConverter[]
             {
                 // Check for disallowed types.
                 new UnsupportedTypeConverterFactory(),
@@ -159,7 +175,7 @@ internal JsonConverter GetConverterFromMember(Type? parentClassType, Type proper
         [RequiresUnreferencedCode("Getting a converter for a type may require reflection which depends on unreferenced code.")]
         public JsonConverter GetConverter(Type typeToConvert!!)
         {
-            RootBuiltInConverters();
+            RootReflectionSerializerDependencies();
             return GetConverterInternal(typeToConvert);
         }
 

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

@@ -33,13 +33,8 @@ public sealed partial class JsonSerializerOptions
         /// </remarks>
         public static JsonSerializerOptions Default { get; } = CreateDefaultImmutableInstance();
 
-        internal JsonSerializerContext? _serializerContext;
-
-        // Stores the JsonTypeInfo factory, which requires unreferenced code and must be rooted by the reflection-based serializer.
-        private static Func<Type, JsonSerializerOptions, JsonTypeInfo>? s_typeInfoCreationFunc;
-
         // For any new option added, adding it to the options copied in the copy constructor below must be considered.
-
+        private JsonSerializerContext? _serializerContext;
         private MemberAccessor? _memberAccessorStrategy;
         private JsonNamingPolicy? _dictionaryKeyPolicy;
         private JsonNamingPolicy? _jsonPropertyNamingPolicy;
@@ -143,19 +138,15 @@ public JsonSerializerOptions(JsonSerializerDefaults defaults) : this()
         }
 
         /// <summary>
-        /// Binds current <see cref="JsonSerializerOptions"/> instance with a new instance of the specified <see cref="JsonSerializerContext"/> type.
+        /// Binds current <see cref="JsonSerializerOptions"/> instance with a new instance of the specified <see cref="Serialization.JsonSerializerContext"/> type.
         /// </summary>
         /// <typeparam name="TContext">The generic definition of the specified context type.</typeparam>
         /// <remarks>When serializing and deserializing types using the options
         /// instance, metadata for the types will be fetched from the context instance.
         /// </remarks>
         public void AddContext<TContext>() where TContext : JsonSerializerContext, new()
         {
-            if (_serializerContext != null)
-            {
-                ThrowHelper.ThrowInvalidOperationException_JsonSerializerOptionsAlreadyBoundToContext();
-            }
-
+            VerifyMutable();
             TContext context = new();
             _serializerContext = context;
             context._options = this;
@@ -550,6 +541,16 @@ public ReferenceHandler? ReferenceHandler
             }
         }
 
+        internal JsonSerializerContext? JsonSerializerContext
+        {
+            get => _serializerContext;
+            set
+            {
+                VerifyMutable();
+                _serializerContext = value;
+            }
+        }
+
         // The cached value used to determine if ReferenceHandler should use Preserve or IgnoreCycles semanitcs or None of them.
         internal ReferenceHandlingStrategy ReferenceHandlingStrategy = ReferenceHandlingStrategy.None;
 
@@ -576,27 +577,25 @@ internal MemberAccessor MemberAccessorStrategy
         }
 
         /// <summary>
-        /// Whether <see cref="InitializeForReflectionSerializer()"/> needs to be called.
+        /// Whether the options instance has been primed for reflection-based serialization.
         /// </summary>
-        internal static bool IsInitializedForReflectionSerializer { get; set; }
+        internal bool IsInitializedForReflectionSerializer { get; private set; }
 
         /// <summary>
         /// Initializes the converters for the reflection-based serializer.
         /// <seealso cref="InitializeForReflectionSerializer"/> must be checked before calling.
         /// </summary>
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
-        internal static void InitializeForReflectionSerializer()
+        internal void InitializeForReflectionSerializer()
         {
-            // For threading cases, the state that is set here can be overwritten.
-            RootBuiltInConverters();
-            s_typeInfoCreationFunc = CreateJsonTypeInfo;
+            RootReflectionSerializerDependencies();
             IsInitializedForReflectionSerializer = true;
-
-            [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
-            static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options) => new JsonTypeInfo(type, options);
+            if (_cachingContext != null)
+            {
+                _cachingContext.Options.IsInitializedForReflectionSerializer = true;
+            }
         }
 
-
         private JsonTypeInfo GetJsonTypeInfoFromContextOrCreate(Type type)
         {
             JsonTypeInfo? info = _serializerContext?.GetTypeInfo(type);
@@ -605,12 +604,13 @@ private JsonTypeInfo GetJsonTypeInfoFromContextOrCreate(Type type)
                 return info;
             }
 
-            if (s_typeInfoCreationFunc == null)
+            if (!IsInitializedForReflectionSerializer)
             {
                 ThrowHelper.ThrowNotSupportedException_NoMetadataForType(type);
                 return null!;
             }
 
+            Debug.Assert(s_typeInfoCreationFunc != null);
             return s_typeInfoCreationFunc(type, this);
         }
 

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs

@@ -574,7 +574,7 @@ internal void InitializePropCache()
             Debug.Assert(PropertyCache == null);
             Debug.Assert(PropertyInfoForTypeInfo.ConverterStrategy == ConverterStrategy.Object);
 
-            JsonSerializerContext? context = Options._serializerContext;
+            JsonSerializerContext? context = Options.JsonSerializerContext;
             Debug.Assert(context != null);
 
             JsonPropertyInfo[] array;
@@ -630,7 +630,7 @@ internal void InitializeParameterCache()
             Debug.Assert(PropertyCache != null);
             Debug.Assert(PropertyInfoForTypeInfo.ConverterStrategy == ConverterStrategy.Object);
 
-            JsonSerializerContext? context = Options._serializerContext;
+            JsonSerializerContext? context = Options.JsonSerializerContext;
             Debug.Assert(context != null);
 
             JsonParameterInfoValues[] array;

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

@@ -591,12 +591,6 @@ internal static void ThrowUnexpectedMetadataException(
             }
         }
 
-        [DoesNotReturn]
-        public static void ThrowInvalidOperationException_JsonSerializerOptionsAlreadyBoundToContext()
-        {
-            throw new InvalidOperationException(SR.Format(SR.OptionsAlreadyBoundToContext));
-        }
-
         [DoesNotReturn]
         public static void ThrowNotSupportedException_BuiltInConvertersNotRooted(Type type)
         {

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs

@@ -38,6 +38,7 @@ public static void Converters_AndTypeInfoCreator_NotRooted_WhenMetadataNotPresen
                     // This test uses reflection to:
                     // - Access JsonSerializerOptions.s_defaultSimpleConverters
                     // - Access JsonSerializerOptions.s_defaultFactoryConverters
+                    // - Access JsonSerializerOptions.s_typeInfoCreationFunc
                     //
                     // If any of them changes, this test will need to be kept in sync.
 

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

@@ -257,7 +257,6 @@ static Func<JsonSerializerOptions, int> CreateCacheCountAccessor()
 
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         [MemberData(nameof(GetJsonSerializerOptions))]
-        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
         public static void JsonSerializerOptions_ReuseConverterCaches()
         {
             // This test uses reflection to:
@@ -286,10 +285,12 @@ public static void JsonSerializerOptions_ReuseConverterCaches()
                     for (int i = 0; i < 5; i++)
                     {
                         var options2 = new JsonSerializerOptions(options);
-                        Assert.True(equalityComparer.Equals(options2, originalCacheOptions));
-                        Assert.Equal(equalityComparer.GetHashCode(options2), equalityComparer.GetHashCode(originalCacheOptions));
                         Assert.Null(getCacheOptions(options2));
+
                         JsonSerializer.Serialize(42, options2);
+
+                        Assert.True(equalityComparer.Equals(options2, originalCacheOptions));
+                        Assert.Equal(equalityComparer.GetHashCode(options2), equalityComparer.GetHashCode(originalCacheOptions));
                         Assert.Same(originalCacheOptions, getCacheOptions(options2));
                     }
                 }
@@ -318,7 +319,6 @@ public static IEnumerable<object[]> GetJsonSerializerOptions()
         }
 
         [Fact]
-        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
         public static void JsonSerializerOptions_EqualityComparer_ChangingAnySettingShouldReturnFalse()
         {
             // This test uses reflection to:
@@ -386,7 +386,6 @@ static IEnumerable<PropertyInfo> GetAllPublicPropertiesWithSetters()
         }
 
         [Fact]
-        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
         public static void JsonSerializerOptions_EqualityComparer_ApplyingJsonSerializerContextShouldReturnFalse()
         {
             // This test uses reflection to:

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

@@ -1,6 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
+using System.IO;
+using Microsoft.DotNet.RemoteExecutor;
 using Xunit;
 
 namespace System.Text.Json.Serialization.Tests
@@ -176,6 +179,38 @@ void RunTest<TConverterReturn>()
             }
         }
 
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public static void GetConverter_Poco_WriteThrowsNotSupportedException()
+        {
+            RemoteExecutor.Invoke(() =>
+            {
+                JsonSerializerOptions options = new();
+                JsonConverter<Point_2D> converter = (JsonConverter<Point_2D>)options.GetConverter(typeof(Point_2D));
+
+                using var writer = new Utf8JsonWriter(new MemoryStream());
+                var value = new Point_2D(0, 0);
+
+                // Running the converter without priming the options instance
+                // for reflection-based serialization should throw NotSupportedException
+                // since it can't resolve reflection-based metadata.
+                Assert.Throws<NotSupportedException>(() => converter.Write(writer, value, options));
+                Debug.Assert(writer.BytesCommitted + writer.BytesPending == 0);
+
+                JsonSerializer.Serialize(42, options);
+
+                // Same operation should succeed when instance has been primed.
+                converter.Write(writer, value, options);
+                Debug.Assert(writer.BytesCommitted + writer.BytesPending > 0);
+                writer.Reset();
+
+                // State change should not leak into unrelated options instances.
+                var options2 = new JsonSerializerOptions();
+                options2.AddContext<JsonContext>();
+                Assert.Throws<NotSupportedException>(() => converter.Write(writer, value, options2));
+                Debug.Assert(writer.BytesCommitted + writer.BytesPending == 0);
+            }).Dispose();
+        }
+
         [Fact]
         public static void GetConverterTypeToConvertNull()
         {