Skip to content

Commit

Permalink
Remove implicit fallback to reflection-based serialization. Fix dotne…
Browse files Browse the repository at this point in the history
…t#71714

Include JsonSerializerContext in JsonSerializerOptions copy constructor. Fix dotnet#71716

Move reflection-based converter resolution out of JsonSerializerOptions. Fix dotnet#68878
  • Loading branch information
eiriktsarpalis committed Jul 7, 2022
1 parent 11bfa9b commit 708b8cc
Show file tree
Hide file tree
Showing 23 changed files with 330 additions and 318 deletions.
1 change: 1 addition & 0 deletions src/libraries/System.Text.Json/ref/System.Text.Json.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ internal JsonConverter GetConverterInternal(Type typeToConvert, JsonSerializerOp
break;
}

return converter!;
return converter;
}

internal sealed override object ReadCoreAsObject(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,35 @@ 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
{
if (_options is null)
{
var options = new JsonSerializerOptions { TypeInfoResolver = this, IsLockedInstance = true };
_options = options;
}

return _options;
}

internal set
{
value.TypeInfoResolver = this;
value.IsLockedInstance = true;
_options = value;
}
}

/// <summary>
/// Indicates whether pre-generated serialization logic for types in the context
Expand Down Expand Up @@ -84,8 +103,7 @@ protected JsonSerializerContext(JsonSerializerOptions? options)
{
if (options != null)
{
options.TypeInfoResolver = this;
Debug.Assert(_options == options, "options.TypeInfoResolver setter did not assign options");
Options = options;
}
}

Expand All @@ -98,12 +116,6 @@ protected JsonSerializerContext(JsonSerializerOptions? options)

JsonTypeInfo? IJsonTypeInfoResolver.GetTypeInfo(Type type, JsonSerializerOptions options)
{
if (options != null && _options != options)
{
// TODO is this the appropriate exception message to throw?
ThrowHelper.ThrowInvalidOperationException_SerializerContextOptionsImmutable();
}

return GetTypeInfo(type);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,27 @@ 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 = _cachingContext.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)
{
Expand All @@ -57,20 +57,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;
}

Expand All @@ -86,8 +84,7 @@ internal void ClearCaches()
[MemberNotNull(nameof(_cachingContext))]
private void InitializeCachingContext()
{
_isLockedInstance = true;
_cachingContext = TrackedCachingContexts.GetOrCreate(this);
_cachingContext ??= TrackedCachingContexts.GetOrCreate(this);
}

/// <summary>
Expand Down Expand Up @@ -117,17 +114,16 @@ public CachingContext(JsonSerializerOptions options)
return typeInfo;
}

typeInfo = Options.GetTypeInfoInternal(type);
typeInfo = Options.GetTypeInfoNoCaching(type);
if (typeInfo != null)
{
return _jsonTypeInfoCache.GetOrAdd(type, _ => typeInfo);
return _jsonTypeInfoCache.GetOrAdd(type, typeInfo);
}

return null;
}

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()
{
Expand All @@ -147,12 +143,12 @@ 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");
ConcurrentDictionary<JsonSerializerOptions, WeakReference<CachingContext>> cache = s_cache;

if (cache.TryGetValue(options, out WeakReference<CachingContext>? wr) && wr.TryGetTarget(out CachingContext? ctx))
Expand Down Expand Up @@ -187,12 +183,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);
Expand Down Expand Up @@ -371,8 +362,9 @@ static void GetHashCode<TValue>(ref HashCode hc, ConfigurationList<TValue> list)
}

// An options instance might be locked but not initialized for reflection serialization yet.
// Ensure hashing is consistent before and after rooting by returning null when the default resolver is used.
private static IJsonTypeInfoResolver? NormalizeResolver(IJsonTypeInfoResolver? resolver)
=> resolver ?? DefaultJsonTypeInfoResolver.DefaultInstance;
=> resolver == DefaultJsonTypeInfoResolver.DefaultInstance ? null : resolver;

#if !NETCOREAPP
/// <summary>
Expand Down
Loading

0 comments on commit 708b8cc

Please sign in to comment.