From 246d830dd53ed8d3d008cf33a0b766225e2d2cd6 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 14 Sep 2022 17:16:03 +0100 Subject: [PATCH 1/2] Implement an AppContext compatibility switch re-enabling reflection fallback in sourcegen. --- .../src/System.Text.Json.csproj | 1 + .../Text/Json/AppContextSwitchHelper.cs | 45 +++++++++++++++++++ .../Serialization/JsonSerializerOptions.cs | 18 +++++++- .../Serialization/OptionsTests.cs | 40 +++++++++++++++-- 4 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index e17ce01c770357..8cd779f5a02188 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -36,6 +36,7 @@ The System.Text.Json library is built-in as part of the shared framework in .NET + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs new file mode 100644 index 00000000000000..c9d5f84999a844 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading.Tasks; + +namespace System.Text.Json +{ + internal static class AppContextSwitchHelper + { + private const string SourceGenReflectionFallbackEnabled_SwitchName = "System.Text.Json.Serialization.EnableSourceGenReflectionFallback"; + private static int s_SourceGenReflectionFallbackEnabled_CachedValue; + + public static bool IsSourceGenReflectionFallbackEnabled + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => GetCachedSwitchValue(SourceGenReflectionFallbackEnabled_SwitchName, ref s_SourceGenReflectionFallbackEnabled_CachedValue, defaultValue: false); + } + + // Returns value of given switch using provided cache. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool GetCachedSwitchValue(string switchName, ref int cachedSwitchValue, bool defaultValue) + { + // The cached switch value has 3 states: 0 - unknown, 1 - true, -1 - false + if (cachedSwitchValue < 0) return false; + if (cachedSwitchValue > 0) return true; + + return GetCachedSwitchValueInternal(switchName, ref cachedSwitchValue, defaultValue); + } + + private static bool GetCachedSwitchValueInternal(string switchName, ref int cachedSwitchValue, bool defaultValue) + { + if (!AppContext.TryGetSwitch(switchName, out bool isSwitchEnabled)) + { + isSwitchEnabled = defaultValue; + } + + cachedSwitchValue = isSwitchEnabled ? 1 /*true*/ : -1 /*false*/; + return isSwitchEnabled; + } + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index d84cc56e236b30..cda505f31ccd0a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -643,17 +643,31 @@ internal void InitializeForReflectionSerializer() // 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(); - _typeInfoResolver ??= defaultResolver; + + 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; } internal bool IsInitializedForReflectionSerializer => _isInitializedForReflectionSerializer; private volatile bool _isInitializedForReflectionSerializer; + private IJsonTypeInfoResolver? _effectiveJsonTypeInfoResolver; private JsonTypeInfo? GetTypeInfoNoCaching(Type type) { - JsonTypeInfo? info = _typeInfoResolver?.GetTypeInfo(type, this); + JsonTypeInfo? info = (_effectiveJsonTypeInfoResolver ?? _typeInfoResolver)?.GetTypeInfo(type, this); if (info != null) { diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs index 0e01eb9f8717fb..1006e366869deb 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs @@ -475,11 +475,18 @@ public static void Options_JsonSerializerContext_DoesNotFallbackToReflection() } [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public static void Options_JsonSerializerContext_GetConverter_DoesNotFallBackToReflectionConverter() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public static void Options_JsonSerializerContext_GetConverter_DoesNotFallBackToReflectionConverter(bool isCompatibilitySwitchExplicitlyDisabled) { - RemoteExecutor.Invoke(static () => + RemoteExecutor.Invoke(static (string isCompatibilitySwitchExplicitlyDisabled) => { + if (bool.Parse(isCompatibilitySwitchExplicitlyDisabled)) + { + AppContext.SetSwitch("System.Text.Json.Serialization.EnableSourceGenReflectionFallback", isEnabled: false); + } + JsonContext context = JsonContext.Default; var unsupportedValue = new MyClass(); @@ -498,6 +505,33 @@ public static void Options_JsonSerializerContext_GetConverter_DoesNotFallBackToR Assert.Throws(() => context.Options.GetConverter(typeof(MyClass))); Assert.Throws(() => JsonSerializer.Serialize(unsupportedValue, context.Options)); + }, isCompatibilitySwitchExplicitlyDisabled.ToString()).Dispose(); + } + + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_FallsBackToReflectionResolver() + { + RemoteExecutor.Invoke(static () => + { + AppContext.SetSwitch("System.Text.Json.Serialization.EnableSourceGenReflectionFallback", isEnabled: true); + + var unsupportedValue = new MyClass { Value = "value" }; + + // JsonSerializerContext does not return metadata for the type + Assert.Null(JsonContext.Default.GetTypeInfo(typeof(MyClass))); + + // Serialization fails using the JsonSerializerContext overload + Assert.Throws(() => JsonSerializer.Serialize(unsupportedValue, unsupportedValue.GetType(), JsonContext.Default)); + + // Serialization uses reflection fallback using the JsonSerializerOptions overload + string json = JsonSerializer.Serialize(unsupportedValue, JsonContext.Default.Options); + JsonTestHelper.AssertJsonEqual("""{"Value":"value", "Thing":null}""", json); + + // A converter can be resolved when looking up JsonSerializerOptions + JsonConverter converter = JsonContext.Default.Options.GetConverter(typeof(MyClass)); + Assert.IsAssignableFrom>(converter); + }).Dispose(); } From bd16419f929d4fddd768d32192deebc3cd1c160e Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 15 Sep 2022 14:10:59 +0100 Subject: [PATCH 2/2] address feedback --- .../Text/Json/AppContextSwitchHelper.cs | 41 +++---------------- .../Serialization/JsonSerializerOptions.cs | 2 + .../Serialization/OptionsTests.cs | 26 ++++++++---- 3 files changed, 25 insertions(+), 44 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs index c9d5f84999a844..9c028f02165171 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs @@ -1,45 +1,16 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Runtime.CompilerServices; -using System.Text; -using System.Threading.Tasks; - namespace System.Text.Json { internal static class AppContextSwitchHelper { - private const string SourceGenReflectionFallbackEnabled_SwitchName = "System.Text.Json.Serialization.EnableSourceGenReflectionFallback"; - private static int s_SourceGenReflectionFallbackEnabled_CachedValue; - - public static bool IsSourceGenReflectionFallbackEnabled - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => GetCachedSwitchValue(SourceGenReflectionFallbackEnabled_SwitchName, ref s_SourceGenReflectionFallbackEnabled_CachedValue, defaultValue: false); - } - - // Returns value of given switch using provided cache. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool GetCachedSwitchValue(string switchName, ref int cachedSwitchValue, bool defaultValue) - { - // The cached switch value has 3 states: 0 - unknown, 1 - true, -1 - false - if (cachedSwitchValue < 0) return false; - if (cachedSwitchValue > 0) return true; - - return GetCachedSwitchValueInternal(switchName, ref cachedSwitchValue, defaultValue); - } - - private static bool GetCachedSwitchValueInternal(string switchName, ref int cachedSwitchValue, bool defaultValue) - { - if (!AppContext.TryGetSwitch(switchName, out bool isSwitchEnabled)) - { - isSwitchEnabled = defaultValue; - } + public static bool IsSourceGenReflectionFallbackEnabled => s_isSourceGenReflectionFallbackEnabled; - cachedSwitchValue = isSwitchEnabled ? 1 /*true*/ : -1 /*false*/; - return isSwitchEnabled; - } + private static readonly bool s_isSourceGenReflectionFallbackEnabled = + AppContext.TryGetSwitch( + switchName: "System.Text.Json.Serialization.EnableSourceGenReflectionFallback", + isEnabled: out bool value) + ? value : false; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index cda505f31ccd0a..698be489adf6ac 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -663,6 +663,8 @@ internal void InitializeForReflectionSerializer() internal bool IsInitializedForReflectionSerializer => _isInitializedForReflectionSerializer; private volatile bool _isInitializedForReflectionSerializer; + + // Only populated in .NET 6 compatibility mode encoding reflection fallback in source gen private IJsonTypeInfoResolver? _effectiveJsonTypeInfoResolver; private JsonTypeInfo? GetTypeInfoNoCaching(Type type) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs index 1006e366869deb..da548831a1a7a1 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs @@ -480,13 +480,15 @@ public static void Options_JsonSerializerContext_DoesNotFallbackToReflection() [InlineData(true)] public static void Options_JsonSerializerContext_GetConverter_DoesNotFallBackToReflectionConverter(bool isCompatibilitySwitchExplicitlyDisabled) { - RemoteExecutor.Invoke(static (string isCompatibilitySwitchExplicitlyDisabled) => + var options = new RemoteInvokeOptions(); + + if (isCompatibilitySwitchExplicitlyDisabled) { - if (bool.Parse(isCompatibilitySwitchExplicitlyDisabled)) - { - AppContext.SetSwitch("System.Text.Json.Serialization.EnableSourceGenReflectionFallback", isEnabled: false); - } + options.RuntimeConfigurationOptions.Add("System.Text.Json.Serialization.EnableSourceGenReflectionFallback", false); + } + RemoteExecutor.Invoke(static () => + { JsonContext context = JsonContext.Default; var unsupportedValue = new MyClass(); @@ -505,17 +507,23 @@ public static void Options_JsonSerializerContext_GetConverter_DoesNotFallBackToR Assert.Throws(() => context.Options.GetConverter(typeof(MyClass))); Assert.Throws(() => JsonSerializer.Serialize(unsupportedValue, context.Options)); - }, isCompatibilitySwitchExplicitlyDisabled.ToString()).Dispose(); + }, options).Dispose(); } [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_FallsBackToReflectionResolver() { + var options = new RemoteInvokeOptions + { + RuntimeConfigurationOptions = + { + ["System.Text.Json.Serialization.EnableSourceGenReflectionFallback"] = true + } + }; + RemoteExecutor.Invoke(static () => { - AppContext.SetSwitch("System.Text.Json.Serialization.EnableSourceGenReflectionFallback", isEnabled: true); - var unsupportedValue = new MyClass { Value = "value" }; // JsonSerializerContext does not return metadata for the type @@ -532,7 +540,7 @@ public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_FallsBa JsonConverter converter = JsonContext.Default.Options.GetConverter(typeof(MyClass)); Assert.IsAssignableFrom>(converter); - }).Dispose(); + }, options).Dispose(); } [Fact]