diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index c8759d3fe2710d..100a2068127ed0 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -569,9 +569,6 @@ A 'JsonSerializerOptions' instance associated with a 'JsonSerializerContext' instance cannot be mutated once the context has been instantiated. - - "The specified 'JsonSerializerOptions' instance is already bound with a 'JsonSerializerContext' instance." - The generic type of the converter for property '{0}.{1}' must match with the specified converter type '{2}'. The converter must not be 'null'. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs index 599f57c8ba5b90..61ce1b8faa9590 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs +++ b/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 info && info.SerializeHandler != null && - info.Options._serializerContext?.CanUseSerializationLogic == true) + info.Options.JsonSerializerContext?.CanUseSerializationLogic == true) { info.SerializeHandler(writer, value); return true; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs index fa39f8daaf871b..71686187483625 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs +++ b/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); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs index 10ede4e2a4a700..cd8584f03b4291 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs +++ b/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); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs index 45a39dc58511fe..607cb383cbaace 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs @@ -41,7 +41,7 @@ private static void WriteUsingGeneratedSerializer(Utf8JsonWriter writer, if (jsonTypeInfo.HasSerialize && jsonTypeInfo is JsonTypeInfo 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(Utf8JsonWriter writer, in TValu Debug.Assert(!jsonTypeInfo.HasSerialize || jsonTypeInfo is not JsonTypeInfo || - 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; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs index 7254cd6f2cf09e..09361a251de569 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs @@ -23,19 +23,7 @@ public abstract partial class JsonSerializerContext /// /// The instance cannot be mutated once it is bound with the context instance. /// - public JsonSerializerOptions Options - { - get - { - if (_options == null) - { - _options = new JsonSerializerOptions(); - _options._serializerContext = this; - } - - return _options; - } - } + public JsonSerializerOptions Options => _options ??= new JsonSerializerOptions { JsonSerializerContext = this }; /// /// Indicates whether pre-generated serialization logic for types in the context @@ -97,13 +85,8 @@ protected JsonSerializerContext(JsonSerializerOptions? options) { if (options != null) { - if (options._serializerContext != null) - { - ThrowHelper.ThrowInvalidOperationException_JsonSerializerOptionsAlreadyBoundToContext(); - } - + options.JsonSerializerContext = this; _options = options; - options._serializerContext = this; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs index 774bd817ea9904..75f7b9801a4269 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs +++ b/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; + } } /// @@ -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); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 6d3a3c88d61647..91264247bc9539 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/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? 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); } 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 caa8066ec21abb..95199d7ead9c54 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 @@ -33,13 +33,8 @@ public sealed partial class JsonSerializerOptions /// 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? 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,7 +138,7 @@ public JsonSerializerOptions(JsonSerializerDefaults defaults) : this() } /// - /// Binds current instance with a new instance of the specified type. + /// Binds current instance with a new instance of the specified type. /// /// The generic definition of the specified context type. /// When serializing and deserializing types using the options @@ -151,11 +146,7 @@ public JsonSerializerOptions(JsonSerializerDefaults defaults) : this() /// public void AddContext() 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 } /// - /// Whether needs to be called. + /// Whether the options instance has been primed for reflection-based serialization. /// - internal static bool IsInitializedForReflectionSerializer { get; set; } + internal bool IsInitializedForReflectionSerializer { get; private set; } /// /// Initializes the converters for the reflection-based serializer. /// must be checked before calling. /// [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); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs index e138cc79df92df..50648a870559cd 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs +++ b/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; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index 96e002f55944a8..c9d55ea80912b2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/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) { diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs index 6442dbb6de2c4e..bbba4443cc474a 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs +++ b/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. diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs index 8b4bd83ff46804..25c7f684abc588 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs @@ -257,7 +257,6 @@ static Func 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 GetJsonSerializerOptions() } [Fact] - [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] public static void JsonSerializerOptions_EqualityComparer_ChangingAnySettingShouldReturnFalse() { // This test uses reflection to: @@ -386,7 +386,6 @@ static IEnumerable GetAllPublicPropertiesWithSetters() } [Fact] - [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] public static void JsonSerializerOptions_EqualityComparer_ApplyingJsonSerializerContextShouldReturnFalse() { // This test uses reflection to: diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.cs index 80067292399c46..c97f3e0ed3b9c0 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.cs +++ b/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() } } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void GetConverter_Poco_WriteThrowsNotSupportedException() + { + RemoteExecutor.Invoke(() => + { + JsonSerializerOptions options = new(); + JsonConverter converter = (JsonConverter)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(() => 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(); + Assert.Throws(() => converter.Write(writer, value, options2)); + Debug.Assert(writer.BytesCommitted + writer.BytesPending == 0); + }).Dispose(); + } + [Fact] public static void GetConverterTypeToConvertNull() { 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 b034f232f35c67..791fe69b99f183 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 @@ -532,6 +532,8 @@ public static void JsonSerializerOptions_Default_IsReadOnly() var optionsSingleton = JsonSerializerOptions.Default; Assert.Throws(() => optionsSingleton.IncludeFields = true); Assert.Throws(() => optionsSingleton.Converters.Add(new JsonStringEnumConverter())); + Assert.Throws(() => optionsSingleton.AddContext()); + Assert.Throws(() => new JsonContext(optionsSingleton)); } [Fact]