From dd2b1f68c03037d3afdf60357a69791eaaccbe8b Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 8 Jun 2023 15:08:50 +0100 Subject: [PATCH] Fix DeserializeAsyncEnumerable generic recursion issue. --- .../System.Text.Json/Common/JsonHelpers.cs | 28 ++++++++--- .../src/System/Text/Json/JsonHelpers.cs | 4 +- .../Text/Json/JsonPropertyDictionary.cs | 4 +- .../JsonSerializer.Read.Stream.cs | 47 +++++++++++++++++- .../Serialization/Metadata/JsonTypeInfo.cs | 2 +- .../Metadata/JsonTypeInfoOfT.ReadHelper.cs | 48 ++----------------- .../PreserveReferenceResolver.cs | 2 +- 7 files changed, 77 insertions(+), 58 deletions(-) diff --git a/src/libraries/System.Text.Json/Common/JsonHelpers.cs b/src/libraries/System.Text.Json/Common/JsonHelpers.cs index 939e6e8fb1b0f9..523d2fde6ee6a3 100644 --- a/src/libraries/System.Text.Json/Common/JsonHelpers.cs +++ b/src/libraries/System.Text.Json/Common/JsonHelpers.cs @@ -1,20 +1,21 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Runtime.InteropServices; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices; namespace System.Text.Json { internal static partial class JsonHelpers { +#if !NETCOREAPP /// - /// Emulates Dictionary.TryAdd on netstandard. + /// netstandard/netfx polyfill for Dictionary.TryAdd /// - public static bool TryAdd(this Dictionary dictionary, in TKey key, in TValue value) where TKey : notnull + public static bool TryAdd(this Dictionary dictionary, TKey key, TValue value) where TKey : notnull { -#if NETSTANDARD2_0 || NETFRAMEWORK if (!dictionary.ContainsKey(key)) { dictionary[key] = value; @@ -22,11 +23,24 @@ public static bool TryAdd(this Dictionary dictionary } return false; -#else - return dictionary.TryAdd(key, value); -#endif } + /// + /// netstandard/netfx polyfill for Queue.TryDequeue + /// + public static bool TryDequeue(this Queue queue, [NotNullWhen(true)] out T? result) + { + if (queue.Count > 0) + { + result = queue.Dequeue(); + return true; + } + + result = default; + return false; + } +#endif + /// /// Provides an in-place, stable sorting implementation for List. /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs index ca1567138797ea..87e34f44969865 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs @@ -94,7 +94,7 @@ public static void ReadWithVerify(this ref Utf8JsonReader reader) public static string Utf8GetString(ReadOnlySpan bytes) { return Encoding.UTF8.GetString(bytes -#if NETSTANDARD2_0 || NETFRAMEWORK +#if !NETCOREAPP .ToArray() #endif ); @@ -108,7 +108,7 @@ public static Dictionary CreateDictionaryFromCollection comparer) where TKey : notnull { -#if NETSTANDARD2_0 || NETFRAMEWORK +#if !NETCOREAPP var dictionary = new Dictionary(comparer); foreach (KeyValuePair item in collection) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs index c89a91e34e035a..4ad855e3bbbc58 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs @@ -228,7 +228,7 @@ public T? this[string propertyName] if (_propertyDictionary != null) { // Fast path if item doesn't exist in dictionary. - if (JsonHelpers.TryAdd(_propertyDictionary, propertyName, value)) + if (_propertyDictionary.TryAdd(propertyName, value)) { assignParent?.Invoke(); _propertyList.Add(new KeyValuePair(propertyName, value)); @@ -303,7 +303,7 @@ internal bool TryAddValue(string propertyName, T value) } else { - if (!JsonHelpers.TryAdd(_propertyDictionary, propertyName, value)) + if (!_propertyDictionary.TryAdd(propertyName, value)) { return false; } 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 1c13474d1e1638..f3a9ba2d4b0aa8 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 @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; +using System.Runtime.CompilerServices; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Converters; using System.Text.Json.Serialization.Metadata; @@ -467,8 +468,49 @@ public static partial class JsonSerializer private static IAsyncEnumerable DeserializeAsyncEnumerableCore(Stream utf8Json, JsonTypeInfo jsonTypeInfo, CancellationToken cancellationToken) { Debug.Assert(jsonTypeInfo.IsConfigured); - jsonTypeInfo._asyncEnumerableQueueTypeInfo ??= CreateQueueTypeInfo(jsonTypeInfo); - return jsonTypeInfo.DeserializeAsyncEnumerable(utf8Json, cancellationToken); + + JsonTypeInfo> queueTypeInfo = jsonTypeInfo._asyncEnumerableQueueTypeInfo is { } cachedQueueTypeInfo + ? (JsonTypeInfo>)cachedQueueTypeInfo + : CreateQueueTypeInfo(jsonTypeInfo); + + return CreateAsyncEnumerable(utf8Json, queueTypeInfo, cancellationToken); + + static async IAsyncEnumerable CreateAsyncEnumerable(Stream utf8Json, JsonTypeInfo> queueTypeInfo, [EnumeratorCancellation] CancellationToken cancellationToken) + { + Debug.Assert(queueTypeInfo.IsConfigured); + JsonSerializerOptions options = queueTypeInfo.Options; + var bufferState = new ReadBufferState(options.DefaultBufferSize); + ReadStack readStack = default; + readStack.Initialize(queueTypeInfo, supportContinuation: true); + + var jsonReaderState = new JsonReaderState(options.GetReaderOptions()); + + try + { + do + { + bufferState = await bufferState.ReadFromStreamAsync(utf8Json, cancellationToken, fillBuffer: false).ConfigureAwait(false); + queueTypeInfo.ContinueDeserialize( + ref bufferState, + ref jsonReaderState, + ref readStack); + + if (readStack.Current.ReturnValue is { } returnValue) + { + var queue = (Queue)returnValue!; + while (queue.TryDequeue(out T? element)) + { + yield return element; + } + } + } + while (!bufferState.IsFinalBlock); + } + finally + { + bufferState.Dispose(); + } + } static JsonTypeInfo> CreateQueueTypeInfo(JsonTypeInfo jsonTypeInfo) { @@ -481,6 +523,7 @@ static JsonTypeInfo> CreateQueueTypeInfo(JsonTypeInfo jsonTypeInfo) }; queueTypeInfo.EnsureConfigured(); + jsonTypeInfo._asyncEnumerableQueueTypeInfo = queueTypeInfo; return queueTypeInfo; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index 4ee8969fac4a60..7000de54f1fa7c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -1125,7 +1125,7 @@ internal void ConfigureConstructorParameters() ParameterLookupKey key = new(propertyName, jsonProperty.PropertyType); ParameterLookupValue value = new(jsonProperty); - if (!JsonHelpers.TryAdd(nameLookup, key, value)) + if (!nameLookup.TryAdd(key, value)) { // More than one property has the same case-insensitive name and Type. // Remember so we can throw a nice exception if this property is used as a parameter name. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.ReadHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.ReadHelper.cs index f41bba085afba5..46a33e624cfbb7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.ReadHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.ReadHelper.cs @@ -79,49 +79,11 @@ public partial class JsonTypeInfo } /// - /// Creating a queue JsonTypeInfo from within the DeserializeAsyncEnumerable method - /// triggers generic recursion warnings from the AOT compiler so we instead - /// have the caller do it for us externally (cf. https://github.com/dotnet/runtime/issues/85184) + /// Caches a JsonTypeInfo<Queue<T>> instance used by the DeserializeAsyncEnumerable method. + /// Store as a non-generic type to avoid triggering generic recursion in the AOT compiler. + /// cf. https://github.com/dotnet/runtime/issues/85184 /// - internal JsonTypeInfo>? _asyncEnumerableQueueTypeInfo; - - internal async IAsyncEnumerable DeserializeAsyncEnumerable(Stream utf8Json, [EnumeratorCancellation] CancellationToken cancellationToken) - { - Debug.Assert(_asyncEnumerableQueueTypeInfo?.IsConfigured == true, "must be populated before calling the method."); - JsonTypeInfo> queueTypeInfo = _asyncEnumerableQueueTypeInfo; - JsonSerializerOptions options = queueTypeInfo.Options; - var bufferState = new ReadBufferState(options.DefaultBufferSize); - ReadStack readStack = default; - readStack.Initialize(queueTypeInfo, supportContinuation: true); - - var jsonReaderState = new JsonReaderState(options.GetReaderOptions()); - - try - { - do - { - bufferState = await bufferState.ReadFromStreamAsync(utf8Json, cancellationToken, fillBuffer: false).ConfigureAwait(false); - queueTypeInfo.ContinueDeserialize( - ref bufferState, - ref jsonReaderState, - ref readStack); - - if (readStack.Current.ReturnValue is { } returnValue) - { - var queue = (Queue)returnValue!; - while (queue.Count > 0) - { - yield return queue.Dequeue(); - } - } - } - while (!bufferState.IsFinalBlock); - } - finally - { - bufferState.Dispose(); - } - } + internal JsonTypeInfo? _asyncEnumerableQueueTypeInfo; internal sealed override object? DeserializeAsObject(ref Utf8JsonReader reader, ref ReadStack state) => Deserialize(ref reader, ref state); @@ -135,7 +97,7 @@ internal async IAsyncEnumerable DeserializeAsyncEnumerable(Stream utf8Json, [ internal sealed override object? DeserializeAsObject(Stream utf8Json) => Deserialize(utf8Json); - private T? ContinueDeserialize( + internal T? ContinueDeserialize( ref ReadBufferState bufferState, ref JsonReaderState jsonReaderState, ref ReadStack readStack) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PreserveReferenceResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PreserveReferenceResolver.cs index 824506cda21a0e..a2bf1a53c5d401 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PreserveReferenceResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PreserveReferenceResolver.cs @@ -32,7 +32,7 @@ public override void AddReference(string referenceId, object value) { Debug.Assert(_referenceIdToObjectMap != null); - if (!JsonHelpers.TryAdd(_referenceIdToObjectMap, referenceId, value)) + if (!_referenceIdToObjectMap.TryAdd(referenceId, value)) { ThrowHelper.ThrowJsonException_MetadataDuplicateIdFound(referenceId); }