Skip to content

Commit

Permalink
Implement the JsonSerializer.IsReflectionEnabledByDefault feature swi…
Browse files Browse the repository at this point in the history
…tch (#83844)

* Implement the STJ.DisableDefaultReflection feature switch.

* Reinstate accidentally stripped attribute

* Address feedback.

* Address feedback.

* Add a trimming test for STJ

* Move trimming test to existing trimming tests folder.

* Add source gen serialization test case to Trimming test.

* Fix style.

* Expose the feature switch as a property on JsonSerializer -- rename feature switch to match namespace.

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

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

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

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

* Address feedback.

* Address feedback.

* Add entry to feature-switches.md

---------

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
  • Loading branch information
eiriktsarpalis and eerhardt authored Apr 5, 2023
1 parent 042a350 commit 0f11212
Show file tree
Hide file tree
Showing 17 changed files with 413 additions and 79 deletions.
1 change: 1 addition & 0 deletions docs/workflow/trimming/feature-switches.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ configurations but their defaults might vary as any SDK can set the defaults dif
| NullabilityInfoContextSupport | System.Reflection.NullabilityInfoContext.IsSupported | Nullable attributes can be trimmed when set to false |
| DynamicCodeSupport | System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported | Changes RuntimeFeature.IsDynamicCodeSupported to false to allow testing AOT-safe fallback code without publishing for Native AOT. |
| _AggressiveAttributeTrimming | System.AggressiveAttributeTrimming | When set to true, aggressively trims attributes to allow for the most size savings possible, even if it could result in runtime behavior changes |
| JsonSerializerIsReflectionEnabledByDefault | System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault | When set to false, disables using reflection as the default contract resolver in System.Text.Json |

Any feature-switch which defines property can be set in csproj file or
on the command line as any other MSBuild property. Those without predefined property name
Expand Down
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 @@ -280,6 +280,7 @@ public static partial class JsonSerializer
[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.")]
public static TValue? Deserialize<TValue>(ref System.Text.Json.Utf8JsonReader reader, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
public static TValue? Deserialize<TValue>(ref System.Text.Json.Utf8JsonReader reader, System.Text.Json.Serialization.Metadata.JsonTypeInfo<TValue> jsonTypeInfo) { throw null; }
public static bool IsReflectionEnabledByDefault { get { throw null; } }
public static void Serialize(System.IO.Stream utf8Json, object? value, System.Text.Json.Serialization.Metadata.JsonTypeInfo jsonTypeInfo) { }
[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.")]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<linker>
<assembly fullname="System.Text.Json">
<type fullname="System.Text.Json.JsonSerializer">
<method signature="System.Boolean get_IsReflectionEnabledByDefault()" body="stub" value="false"
feature="System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault" featurevalue="false"/>
</type>
</assembly>
</linker>
4 changes: 4 additions & 0 deletions src/libraries/System.Text.Json/src/System.Text.Json.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ The System.Text.Json library is built-in as part of the shared framework in .NET
<NoWarn Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) != '.NETCoreApp'">$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
<ILLinkSubstitutionsXmls Include="ILLink\ILLink.Substitutions.xml" />
</ItemGroup>

<ItemGroup>
<Compile Include="$(CommonPath)System\HexConverter.cs" Link="Common\System\HexConverter.cs" />
<Compile Include="$(CommonPath)System\Text\Json\PooledByteBufferWriter.cs" Link="Common\System\Text\Json\PooledByteBufferWriter.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ namespace System.Text.Json
{
internal static class AppContextSwitchHelper
{
public static bool IsSourceGenReflectionFallbackEnabled => s_isSourceGenReflectionFallbackEnabled;

private static readonly bool s_isSourceGenReflectionFallbackEnabled =
public static bool IsSourceGenReflectionFallbackEnabled { get; } =
AppContext.TryGetSwitch(
switchName: "System.Text.Json.Serialization.EnableSourceGenReflectionFallback",
isEnabled: out bool value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,20 @@ public static partial class JsonSerializer
internal const string SerializationUnreferencedCodeMessage = "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.";
internal const string SerializationRequiresDynamicCodeMessage = "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.";

/// <summary>
/// Indicates whether unconfigured <see cref="JsonSerializerOptions"/> instances
/// should be set to use the reflection-based <see cref="DefaultJsonTypeInfoResolver"/>.
/// </summary>
/// <remarks>
/// The value of the property is backed by the "System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault"
/// <see cref="AppContext"/> setting and defaults to <see langword="true"/> if unset.
/// </remarks>
public static bool IsReflectionEnabledByDefault { get; } =
AppContext.TryGetSwitch(
switchName: "System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault",
isEnabled: out bool value)
? value : true;

[RequiresUnreferencedCode(SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(SerializationRequiresDynamicCodeMessage)]
private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type inputType, bool fallBackToNearestAncestorType = false)
Expand All @@ -21,9 +35,9 @@ private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type inp

options ??= JsonSerializerOptions.Default;

if (!options.IsInitializedForReflectionSerializer)
if (!options.IsConfiguredForJsonSerializer)
{
options.InitializeForReflectionSerializer();
options.ConfigureForJsonSerializer();
}

// In order to improve performance of polymorphic root-level object serialization,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace System.Text.Json.Serialization
/// <summary>
/// Provides metadata about a set of types that is relevant to JSON serialization.
/// </summary>
public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver
public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver, IBuiltInJsonTypeInfoResolver
{
private JsonSerializerOptions? _options;

Expand Down Expand Up @@ -49,7 +49,7 @@ internal void AssociateWithOptions(JsonSerializerOptions options)
/// Indicates whether pre-generated serialization logic for types in the context
/// is compatible with the run time specified <see cref="JsonSerializerOptions"/>.
/// </summary>
internal bool IsCompatibleWithGeneratedOptions(JsonSerializerOptions options)
bool IBuiltInJsonTypeInfoResolver.IsCompatibleWithOptions(JsonSerializerOptions options)
{
Debug.Assert(options != null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public JsonConverter GetConverter(Type typeToConvert)
ThrowHelper.ThrowArgumentNullException(nameof(typeToConvert));
}

if (_typeInfoResolver is null)
if (JsonSerializer.IsReflectionEnabledByDefault && _typeInfoResolver is null)
{
// Backward compatibility -- root & query the default reflection converters
// but do not populate the TypeInfoResolver setting.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,19 +130,6 @@ public JsonSerializerOptions(JsonSerializerOptions options)
TrackOptionsInstance(this);
}

/// <summary>Tracks the options instance to enable all instances to be enumerated.</summary>
private static void TrackOptionsInstance(JsonSerializerOptions options) => TrackedOptionsInstances.All.Add(options, null);

internal static class TrackedOptionsInstances
{
/// <summary>Tracks all live JsonSerializerOptions instances.</summary>
/// <remarks>Instances are added to the table in their constructor.</remarks>
public static ConditionalWeakTable<JsonSerializerOptions, object?> All { get; } =
// TODO https://github.com/dotnet/runtime/issues/51159:
// Look into linking this away / disabling it when hot reload isn't in use.
new ConditionalWeakTable<JsonSerializerOptions, object?>();
}

/// <summary>
/// Constructs a new <see cref="JsonSerializerOptions"/> instance with a predefined set of options determined by the specified <see cref="JsonSerializerDefaults"/>.
/// </summary>
Expand All @@ -161,6 +148,19 @@ public JsonSerializerOptions(JsonSerializerDefaults defaults) : this()
}
}

/// <summary>Tracks the options instance to enable all instances to be enumerated.</summary>
private static void TrackOptionsInstance(JsonSerializerOptions options) => TrackedOptionsInstances.All.Add(options, null);

internal static class TrackedOptionsInstances
{
/// <summary>Tracks all live JsonSerializerOptions instances.</summary>
/// <remarks>Instances are added to the table in their constructor.</remarks>
public static ConditionalWeakTable<JsonSerializerOptions, object?> All { get; } =
// TODO https://github.com/dotnet/runtime/issues/51159:
// Look into linking this away / disabling it when hot reload isn't in use.
new ConditionalWeakTable<JsonSerializerOptions, object?>();
}

/// <summary>
/// Binds current <see cref="JsonSerializerOptions"/> instance with a new instance of the specified <see cref="Serialization.JsonSerializerContext"/> type.
/// </summary>
Expand Down Expand Up @@ -638,32 +638,7 @@ internal bool CanUseFastPathSerializationLogic
{
Debug.Assert(IsReadOnly);
Debug.Assert(TypeInfoResolver != null);
return _canUseFastPathSerializationLogic ??= CanUseFastPath(TypeInfoResolver);

bool CanUseFastPath(IJsonTypeInfoResolver resolver)
{
switch (resolver)
{
case DefaultJsonTypeInfoResolver defaultResolver:
return defaultResolver.GetType() == typeof(DefaultJsonTypeInfoResolver) &&
defaultResolver.Modifiers.Count == 0;
case JsonSerializerContext ctx:
return ctx.IsCompatibleWithGeneratedOptions(this);
case JsonTypeInfoResolverChain resolverChain:
foreach (IJsonTypeInfoResolver component in resolverChain)
{
if (!CanUseFastPath(component))
{
return false;
}
}

return true;

default:
return false;
}
}
return _canUseFastPathSerializationLogic ??= TypeInfoResolver.IsCompatibleWithOptions(this);
}
}

Expand Down Expand Up @@ -699,35 +674,38 @@ public void MakeReadOnly()
}

/// <summary>
/// Initializes the converters for the reflection-based serializer.
/// Configures the instance for use by the JsonSerializer APIs.
/// </summary>
[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
internal void InitializeForReflectionSerializer()
internal void ConfigureForJsonSerializer()
{
// Even if a resolver has already been specified, we need to root
// the default resolver to gain access to the default converters.
DefaultJsonTypeInfoResolver defaultResolver = DefaultJsonTypeInfoResolver.RootDefaultInstance();

switch (_typeInfoResolver)
if (JsonSerializer.IsReflectionEnabledByDefault)
{
case null:
// Use the default reflection-based resolver if no resolver has been specified.
_typeInfoResolver = defaultResolver;
break;
// Even if a resolver has already been specified, we need to root
// the default resolver to gain access to the default converters.
DefaultJsonTypeInfoResolver defaultResolver = DefaultJsonTypeInfoResolver.RootDefaultInstance();

case JsonSerializerContext ctx when AppContextSwitchHelper.IsSourceGenReflectionFallbackEnabled:
// .NET 6 compatibility mode: enable fallback to reflection metadata for JsonSerializerContext
_effectiveJsonTypeInfoResolver = JsonTypeInfoResolver.Combine(ctx, defaultResolver);
break;
switch (_typeInfoResolver)
{
case null:
// Use the default reflection-based resolver if no resolver has been specified.
_typeInfoResolver = defaultResolver;
break;

case JsonSerializerContext ctx when AppContextSwitchHelper.IsSourceGenReflectionFallbackEnabled:
// .NET 6 compatibility mode: enable fallback to reflection metadata for JsonSerializerContext
_effectiveJsonTypeInfoResolver = JsonTypeInfoResolver.Combine(ctx, defaultResolver);
break;
}
}

MakeReadOnly();
_isInitializedForReflectionSerializer = true;
_isConfiguredForJsonSerializer = true;
}

internal bool IsInitializedForReflectionSerializer => _isInitializedForReflectionSerializer;
private volatile bool _isInitializedForReflectionSerializer;
internal bool IsConfiguredForJsonSerializer => _isConfiguredForJsonSerializer;
private volatile bool _isConfiguredForJsonSerializer;

// Only populated in .NET 6 compatibility mode encoding reflection fallback in source gen
private IJsonTypeInfoResolver? _effectiveJsonTypeInfoResolver;
Expand Down Expand Up @@ -852,8 +830,15 @@ private static JsonSerializerOptions GetOrCreateDefaultOptionsInstance()
{
var options = new JsonSerializerOptions
{
TypeInfoResolver = DefaultJsonTypeInfoResolver.RootDefaultInstance(),
_isReadOnly = true
// Because we're marking the default instance as read-only,
// we need to specify a resolver instance for the case where
// reflection is disabled by default: use one that returns null for all types.

TypeInfoResolver = JsonSerializer.IsReflectionEnabledByDefault
? DefaultJsonTypeInfoResolver.RootDefaultInstance()
: new JsonTypeInfoResolverChain(),

_isReadOnly = true,
};

return Interlocked.CompareExchange(ref s_defaultOptions, options, null) ?? options;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,25 +399,25 @@ internal static void DeterminePropertyAccessors<T>(JsonPropertyInfo<T> jsonPrope
MethodInfo? getMethod = propertyInfo.GetMethod;
if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors))
{
jsonPropertyInfo.Get = DefaultJsonTypeInfoResolver.MemberAccessor.CreatePropertyGetter<T>(propertyInfo);
jsonPropertyInfo.Get = MemberAccessor.CreatePropertyGetter<T>(propertyInfo);
}

MethodInfo? setMethod = propertyInfo.SetMethod;
if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors))
{
jsonPropertyInfo.Set = DefaultJsonTypeInfoResolver.MemberAccessor.CreatePropertySetter<T>(propertyInfo);
jsonPropertyInfo.Set = MemberAccessor.CreatePropertySetter<T>(propertyInfo);
}

break;

case FieldInfo fieldInfo:
Debug.Assert(fieldInfo.IsPublic);

jsonPropertyInfo.Get = DefaultJsonTypeInfoResolver.MemberAccessor.CreateFieldGetter<T>(fieldInfo);
jsonPropertyInfo.Get = MemberAccessor.CreateFieldGetter<T>(fieldInfo);

if (!fieldInfo.IsInitOnly)
{
jsonPropertyInfo.Set = DefaultJsonTypeInfoResolver.MemberAccessor.CreateFieldSetter<T>(fieldInfo);
jsonPropertyInfo.Set = MemberAccessor.CreateFieldSetter<T>(fieldInfo);
}

break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace System.Text.Json.Serialization.Metadata
/// <remarks>
/// The contract resolver used by <see cref="JsonSerializerOptions.Default"/>.
/// </remarks>
public partial class DefaultJsonTypeInfoResolver : IJsonTypeInfoResolver
public partial class DefaultJsonTypeInfoResolver : IJsonTypeInfoResolver, IBuiltInJsonTypeInfoResolver
{
private bool _mutable;

Expand Down Expand Up @@ -122,6 +122,11 @@ protected override void OnCollectionModifying()
}
}

bool IBuiltInJsonTypeInfoResolver.IsCompatibleWithOptions(JsonSerializerOptions _)
// Metadata generated by the default resolver is compatible by definition,
// provided that no user extensions have been made on the class.
=> _modifiers is null or { Count: 0 } && GetType() == typeof(DefaultJsonTypeInfoResolver);

internal static bool IsDefaultInstanceRooted => s_defaultInstance is not null;
private static DefaultJsonTypeInfoResolver? s_defaultInstance;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -695,12 +695,7 @@ bool IsCurrentNodeCompatible()
return false;
}

return OriginatingResolver switch
{
JsonSerializerContext ctx => ctx.IsCompatibleWithGeneratedOptions(Options),
DefaultJsonTypeInfoResolver => true, // generates default contracts by definition
_ => false
};
return OriginatingResolver.IsCompatibleWithOptions(Options);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,25 @@ public static IJsonTypeInfoResolver Combine(params IJsonTypeInfoResolver?[] reso

return resolverChain.Count == 1 ? resolverChain[0] : resolverChain;
}

/// <summary>
/// Indicates whether the metadata generated by the current resolver
/// are compatible with the run time specified <see cref="JsonSerializerOptions"/>.
/// </summary>
internal static bool IsCompatibleWithOptions(this IJsonTypeInfoResolver? resolver, JsonSerializerOptions options)
=> resolver is IBuiltInJsonTypeInfoResolver bir && bir.IsCompatibleWithOptions(options);
}

/// <summary>
/// Implemented by the built-in converters to avoid rooting
/// unused resolver dependencies in the context of the trimmer.
/// </summary>
internal interface IBuiltInJsonTypeInfoResolver
{
/// <summary>
/// Indicates whether the metadata generated by the current resolver
/// are compatible with the run time specified <see cref="JsonSerializerOptions"/>.
/// </summary>
bool IsCompatibleWithOptions(JsonSerializerOptions options);
}
}
Loading

0 comments on commit 0f11212

Please sign in to comment.