Remove implicit fallback to reflection-based serialization (#71746)

* Remove implicit fallback to reflection-based serialization. Fix #71714

Include JsonSerializerContext in JsonSerializerOptions copy constructor. Fix #71716

Move reflection-based converter resolution out of JsonSerializerOptions. Fix #68878

* Address feedback & add one more test

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

Co-authored-by: Stephen Toub <stoub@microsoft.com>

* fix build

* Bring back throwing behavior in JsonSerializerContext and add tests

* Only create caching contexts if a resolver is populated

* Add null test for JsonSerializerContext interface implementation.

* skip RemoteExecutor test in netfx targets

* Add DefaultJsonTypeInfoResolver test for types with JsonConverterAttribute

* remove nullability annotation

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

Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com>

Co-authored-by: Stephen Toub <stoub@microsoft.com>
Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com>

Author: 3 people

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

@@ -356,6 +356,7 @@ public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
         public System.Text.Json.JsonNamingPolicy? PropertyNamingPolicy { get { throw null; } set { } }
         public System.Text.Json.JsonCommentHandling ReadCommentHandling { get { throw null; } set { } }
         public System.Text.Json.Serialization.ReferenceHandler? ReferenceHandler { get { throw null; } set { } }
+        [System.Diagnostics.CodeAnalysis.AllowNullAttribute]
         public System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver TypeInfoResolver { [System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed and might need runtime code generation. Use System.Text.Json source generation for native AOT applications."), System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved.")] get { throw null; } set { } }
         public System.Text.Json.Serialization.JsonUnknownTypeHandling UnknownTypeHandling { get { throw null; } set { } }
         public bool WriteIndented { get { throw null; } set { } }

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

@@ -400,10 +400,10 @@
     <value>The converter '{0}' is not compatible with the type '{1}'.</value>
   </data>
   <data name="ResolverTypeNotCompatible" xml:space="preserve">
-    <value>TypeInfoResolver expected to return JsonTypeInfo of type '{0}' but returned JsonTypeInfo of type '{1}'.</value>
+    <value>The IJsonTypeInfoResolver returned an incompatible JsonTypeInfo instance of type '{0}', expected type '{1}'.</value>
   </data>
   <data name="ResolverTypeInfoOptionsNotCompatible" xml:space="preserve">
-    <value>TypeInfoResolver expected to return JsonTypeInfo options bound to the JsonSerializerOptions provided in the argument.</value>
+    <value>The IJsonTypeInfoResolver returned a JsonTypeInfo instance whose JsonSerializerOptions setting does not match the provided argument.</value>
   </data>
   <data name="SerializationConverterWrite" xml:space="preserve">
     <value>The converter '{0}' wrote too much or not enough.</value>

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

@@ -65,7 +65,7 @@ internal JsonConverter GetConverterInternal(Type typeToConvert, JsonSerializerOp
                     break;
             }
 
-            return converter!;
+            return converter;
         }
 
         internal sealed override object ReadCoreAsObject(

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

@@ -84,6 +84,7 @@ internal sealed override JsonParameterInfo CreateJsonParameterInfo()
 
         internal sealed override JsonConverter<TTarget> CreateCastingConverter<TTarget>()
         {
+            JsonSerializerOptions.CheckConverterNullabilityIsSameAsPropertyType(this, typeof(TTarget));
             return new CastingConverter<TTarget, T>(this);
         }
 

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

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

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

@@ -366,13 +366,7 @@ public static partial class JsonSerializer
                 ThrowHelper.ThrowArgumentNullException(nameof(utf8Json));
             }
 
-            options ??= JsonSerializerOptions.Default;
-            if (!options.IsInitializedForReflectionSerializer)
-            {
-                options.InitializeForReflectionSerializer();
-            }
-
-            JsonTypeInfo jsonTypeInfo = options.GetOrAddJsonTypeInfoForRootType(typeof(TValue));
+            JsonTypeInfo jsonTypeInfo = GetTypeInfo(options, typeof(TValue));
             return CreateAsyncEnumerableDeserializer(utf8Json, CreateQueueTypeInfo<TValue>(jsonTypeInfo), cancellationToken);
         }
 

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

@@ -57,11 +57,7 @@ private static void WriteUsingSerializer<TValue>(Utf8JsonWriter writer, in TValu
         {
             Debug.Assert(writer != null);
 
-            Debug.Assert(!jsonTypeInfo.HasSerialize ||
-                jsonTypeInfo is not JsonTypeInfo<TValue> ||
-                jsonTypeInfo.Options.SerializerContext == null ||
-                !jsonTypeInfo.Options.SerializerContext.CanUseSerializationLogic,
-                "Incorrect method called. WriteUsingGeneratedSerializer() should have been called instead.");
+            // TODO unify method with WriteUsingGeneratedSerializer
 
             WriteStack state = default;
             jsonTypeInfo.EnsureConfigured();

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

@@ -13,16 +13,27 @@ public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver
     {
         private bool? _canUseSerializationLogic;
 
-        internal JsonSerializerOptions? _options;
+        private JsonSerializerOptions? _options;
 
         /// <summary>
         /// Gets the run time specified options of the context. If no options were passed
         /// when instanciating the context, then a new instance is bound and returned.
         /// </summary>
         /// <remarks>
-        /// The instance cannot be mutated once it is bound to the context instance.
+        /// The options instance cannot be mutated once it is bound to the context instance.
         /// </remarks>
-        public JsonSerializerOptions Options => _options ??= new JsonSerializerOptions { TypeInfoResolver = this };
+        public JsonSerializerOptions Options
+        {
+            get => _options ??= new JsonSerializerOptions { TypeInfoResolver = this, IsLockedInstance = true };
+
+            internal set
+            {
+                Debug.Assert(!value.IsLockedInstance);
+                value.TypeInfoResolver = this;
+                value.IsLockedInstance = true;
+                _options = value;
+            }
+        }
 
         /// <summary>
         /// Indicates whether pre-generated serialization logic for types in the context
@@ -84,8 +95,8 @@ protected JsonSerializerContext(JsonSerializerOptions? options)
         {
             if (options != null)
             {
-                options.TypeInfoResolver = this;
-                Debug.Assert(_options == options, "options.TypeInfoResolver setter did not assign options");
+                options.VerifyMutable();
+                Options = options;
             }
         }
 
@@ -98,10 +109,9 @@ protected JsonSerializerContext(JsonSerializerOptions? options)
 
         JsonTypeInfo? IJsonTypeInfoResolver.GetTypeInfo(Type type, JsonSerializerOptions options)
         {
-            if (options != null && _options != options)
+            if (options != null && options != _options)
             {
-                // TODO is this the appropriate exception message to throw?
-                ThrowHelper.ThrowInvalidOperationException_SerializerContextOptionsImmutable();
+                ThrowHelper.ThrowInvalidOperationException_ResolverTypeInfoOptionsNotCompatible();
             }
 
             return GetTypeInfo(type);

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

@@ -26,27 +26,26 @@ public sealed partial class JsonSerializerOptions
         /// <summary>
         /// This method returns configured non-null JsonTypeInfo
         /// </summary>
-        internal JsonTypeInfo GetOrAddJsonTypeInfo(Type type)
+        internal JsonTypeInfo GetJsonTypeInfoCached(Type type)
         {
-            if (_cachingContext == null)
+            JsonTypeInfo? typeInfo = null;
+
+            if (IsLockedInstance)
             {
-                InitializeCachingContext();
+                typeInfo = GetCachingContext()?.GetOrAddJsonTypeInfo(type);
             }
 
-            JsonTypeInfo? typeInfo = _cachingContext.GetOrAddJsonTypeInfo(type);
-
             if (typeInfo == null)
             {
                 ThrowHelper.ThrowNotSupportedException_NoMetadataForType(type);
                 return null;
             }
 
             typeInfo.EnsureConfigured();
-
             return typeInfo;
         }
 
-        internal bool TryGetJsonTypeInfo(Type type, [NotNullWhen(true)] out JsonTypeInfo? typeInfo)
+        internal bool TryGetJsonTypeInfoCached(Type type, [NotNullWhen(true)] out JsonTypeInfo? typeInfo)
         {
             if (_cachingContext == null)
             {
@@ -57,20 +56,18 @@ internal bool TryGetJsonTypeInfo(Type type, [NotNullWhen(true)] out JsonTypeInfo
             return _cachingContext.TryGetJsonTypeInfo(type, out typeInfo);
         }
 
-        internal bool IsJsonTypeInfoCached(Type type) => _cachingContext?.IsJsonTypeInfoCached(type) == true;
-
         /// <summary>
         /// Return the TypeInfo for root API calls.
         /// This has an LRU cache that is intended only for public API calls that specify the root type.
         /// </summary>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal JsonTypeInfo GetOrAddJsonTypeInfoForRootType(Type type)
+        internal JsonTypeInfo GetJsonTypeInfoForRootType(Type type)
         {
             JsonTypeInfo? jsonTypeInfo = _lastTypeInfo;
 
             if (jsonTypeInfo?.Type != type)
             {
-                jsonTypeInfo = GetOrAddJsonTypeInfo(type);
+                jsonTypeInfo = GetJsonTypeInfoCached(type);
                 _lastTypeInfo = jsonTypeInfo;
             }
 
@@ -83,11 +80,16 @@ internal void ClearCaches()
             _lastTypeInfo = null;
         }
 
-        [MemberNotNull(nameof(_cachingContext))]
-        private void InitializeCachingContext()
+        private CachingContext? GetCachingContext()
         {
-            _isLockedInstance = true;
-            _cachingContext = TrackedCachingContexts.GetOrCreate(this);
+            Debug.Assert(IsLockedInstance);
+
+            if (_cachingContext is null && _typeInfoResolver is not null)
+            {
+                _cachingContext = TrackedCachingContexts.GetOrCreate(this);
+            }
+
+            return _cachingContext;
         }
 
         /// <summary>
@@ -98,7 +100,7 @@ private void InitializeCachingContext()
         /// </summary>
         internal sealed class CachingContext
         {
-            private readonly ConcurrentDictionary<Type, JsonTypeInfo> _jsonTypeInfoCache = new();
+            private readonly ConcurrentDictionary<Type, JsonTypeInfo?> _jsonTypeInfoCache = new();
 
             public CachingContext(JsonSerializerOptions options)
             {
@@ -110,24 +112,8 @@ public CachingContext(JsonSerializerOptions options)
             // If changing please ensure that src/ILLink.Descriptors.LibraryBuild.xml is up-to-date.
             public int Count => _jsonTypeInfoCache.Count;
 
-            public JsonTypeInfo? GetOrAddJsonTypeInfo(Type type)
-            {
-                if (_jsonTypeInfoCache.TryGetValue(type, out JsonTypeInfo? typeInfo))
-                {
-                    return typeInfo;
-                }
-
-                typeInfo = Options.GetTypeInfoInternal(type);
-                if (typeInfo != null)
-                {
-                    return _jsonTypeInfoCache.GetOrAdd(type, _ => typeInfo);
-                }
-
-                return null;
-            }
-
+            public JsonTypeInfo? GetOrAddJsonTypeInfo(Type type) => _jsonTypeInfoCache.GetOrAdd(type, Options.GetTypeInfoNoCaching);
             public bool TryGetJsonTypeInfo(Type type, [NotNullWhen(true)] out JsonTypeInfo? typeInfo) => _jsonTypeInfoCache.TryGetValue(type, out typeInfo);
-            public bool IsJsonTypeInfoCached(Type type) => _jsonTypeInfoCache.ContainsKey(type);
 
             public void Clear()
             {
@@ -147,12 +133,14 @@ internal static class TrackedCachingContexts
                 new(concurrencyLevel: 1, capacity: MaxTrackedContexts, new EqualityComparer());
 
             private const int EvictionCountHistory = 16;
-            private static Queue<int> s_recentEvictionCounts = new(EvictionCountHistory);
+            private static readonly Queue<int> s_recentEvictionCounts = new(EvictionCountHistory);
             private static int s_evictionRunsToSkip;
 
             public static CachingContext GetOrCreate(JsonSerializerOptions options)
             {
-                Debug.Assert(options._isLockedInstance, "Cannot create caching contexts for mutable JsonSerializerOptions instances");
+                Debug.Assert(options.IsLockedInstance, "Cannot create caching contexts for mutable JsonSerializerOptions instances");
+                Debug.Assert(options._typeInfoResolver != null);
+
                 ConcurrentDictionary<JsonSerializerOptions, WeakReference<CachingContext>> cache = s_cache;
 
                 if (cache.TryGetValue(options, out WeakReference<CachingContext>? wr) && wr.TryGetTarget(out CachingContext? ctx))
@@ -187,12 +175,7 @@ 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)
-                    {
-                        // Copy fields ignored by the copy constructor
-                        // but are necessary to determine equivalence.
-                        _typeInfoResolver = options._typeInfoResolver,
-                    };
+                    var key = new JsonSerializerOptions(options);
                     Debug.Assert(key._cachingContext == null);
 
                     ctx = new CachingContext(options);
@@ -312,7 +295,7 @@ public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right)
                     left._includeFields == right._includeFields &&
                     left._propertyNameCaseInsensitive == right._propertyNameCaseInsensitive &&
                     left._writeIndented == right._writeIndented &&
-                    NormalizeResolver(left._typeInfoResolver) == NormalizeResolver(right._typeInfoResolver) &&
+                    left._typeInfoResolver == right._typeInfoResolver &&
                     CompareLists(left._converters, right._converters);
 
                 static bool CompareLists<TValue>(ConfigurationList<TValue> left, ConfigurationList<TValue> right)
@@ -356,7 +339,7 @@ public int GetHashCode(JsonSerializerOptions options)
                 hc.Add(options._includeFields);
                 hc.Add(options._propertyNameCaseInsensitive);
                 hc.Add(options._writeIndented);
-                hc.Add(NormalizeResolver(options._typeInfoResolver));
+                hc.Add(options._typeInfoResolver);
                 GetHashCode(ref hc, options._converters);
 
                 static void GetHashCode<TValue>(ref HashCode hc, ConfigurationList<TValue> list)
@@ -370,10 +353,6 @@ static void GetHashCode<TValue>(ref HashCode hc, ConfigurationList<TValue> list)
                 return hc.ToHashCode();
             }
 
-            // An options instance might be locked but not initialized for reflection serialization yet.
-            private static IJsonTypeInfoResolver? NormalizeResolver(IJsonTypeInfoResolver? resolver)
-                => resolver ?? DefaultJsonTypeInfoResolver.DefaultInstance;
-
 #if !NETCOREAPP
             /// <summary>
             /// Polyfill for System.HashCode.