Fix DeserializeAsyncEnumerable generic recursion issue. (#87276)

dotnet · Jun 8, 2023 · 2905fe7 · 2905fe7
1 parent 56c427b
commit 2905fe7
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 58 deletions.
diff --git a/src/libraries/System.Text.Json/Common/JsonHelpers.cs b/src/libraries/System.Text.Json/Common/JsonHelpers.cs
@@ -1,32 +1,46 @@
 // 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
         /// <summary>
-        /// Emulates Dictionary.TryAdd on netstandard.
+        /// netstandard/netfx polyfill for Dictionary.TryAdd
         /// </summary>
-        public static bool TryAdd<TKey, TValue>(this Dictionary<TKey, TValue> dictionary, in TKey key, in TValue value) where TKey : notnull
+        public static bool TryAdd<TKey, TValue>(this Dictionary<TKey, TValue> dictionary, TKey key, TValue value) where TKey : notnull
         {
-#if NETSTANDARD2_0 || NETFRAMEWORK
             if (!dictionary.ContainsKey(key))
             {
                 dictionary[key] = value;
                 return true;
             }
 
             return false;
-#else
-            return dictionary.TryAdd(key, value);
-#endif
         }
 
+        /// <summary>
+        /// netstandard/netfx polyfill for Queue.TryDequeue
+        /// </summary>
+        public static bool TryDequeue<T>(this Queue<T> queue, [NotNullWhen(true)] out T? result)
+        {
+            if (queue.Count > 0)
+            {
+                result = queue.Dequeue();
+                return true;
+            }
+
+            result = default;
+            return false;
+        }
+#endif
+
         /// <summary>
         /// Provides an in-place, stable sorting implementation for List.
         /// </summary>

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
@@ -94,7 +94,7 @@ public static void ReadWithVerify(this ref Utf8JsonReader reader)
         public static string Utf8GetString(ReadOnlySpan<byte> bytes)
         {
             return Encoding.UTF8.GetString(bytes
-#if NETSTANDARD2_0 || NETFRAMEWORK
+#if !NETCOREAPP
                         .ToArray()
 #endif
                 );
@@ -108,7 +108,7 @@ public static Dictionary<TKey, TValue> CreateDictionaryFromCollection<TKey, TVal
             IEqualityComparer<TKey> comparer)
             where TKey : notnull
         {
-#if NETSTANDARD2_0 || NETFRAMEWORK
+#if !NETCOREAPP
             var dictionary = new Dictionary<TKey, TValue>(comparer);
 
             foreach (KeyValuePair<TKey, TValue> 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
@@ -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<string, T>(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/...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs b/...braries/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<T> DeserializeAsyncEnumerableCore<T>(Stream utf8Json, JsonTypeInfo<T> jsonTypeInfo, CancellationToken cancellationToken)
         {
             Debug.Assert(jsonTypeInfo.IsConfigured);
-            jsonTypeInfo._asyncEnumerableQueueTypeInfo ??= CreateQueueTypeInfo(jsonTypeInfo);
-            return jsonTypeInfo.DeserializeAsyncEnumerable(utf8Json, cancellationToken);
+
+            JsonTypeInfo<Queue<T>> queueTypeInfo = jsonTypeInfo._asyncEnumerableQueueTypeInfo is { } cachedQueueTypeInfo
+                ? (JsonTypeInfo<Queue<T>>)cachedQueueTypeInfo
+                : CreateQueueTypeInfo(jsonTypeInfo);
+
+            return CreateAsyncEnumerable(utf8Json, queueTypeInfo, cancellationToken);
+
+            static async IAsyncEnumerable<T> CreateAsyncEnumerable(Stream utf8Json, JsonTypeInfo<Queue<T>> 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<T>)returnValue!;
+                            while (queue.TryDequeue(out T? element))
+                            {
+                                yield return element;
+                            }
+                        }
+                    }
+                    while (!bufferState.IsFinalBlock);
+                }
+                finally
+                {
+                    bufferState.Dispose();
+                }
+            }
 
             static JsonTypeInfo<Queue<T>> CreateQueueTypeInfo(JsonTypeInfo<T> jsonTypeInfo)
             {
@@ -481,6 +523,7 @@ static JsonTypeInfo<Queue<T>> CreateQueueTypeInfo(JsonTypeInfo<T> 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
@@ -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/...ystem.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.ReadHelper.cs b/...ystem.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.ReadHelper.cs
@@ -79,49 +79,11 @@ public partial class JsonTypeInfo<T>
         }
 
         /// <summary>
-        /// 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&lt;Queue&lt;T&gt;&gt; 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
         /// </summary>
-        internal JsonTypeInfo<Queue<T>>? _asyncEnumerableQueueTypeInfo;
-
-        internal async IAsyncEnumerable<T> DeserializeAsyncEnumerable(Stream utf8Json, [EnumeratorCancellation] CancellationToken cancellationToken)
-        {
-            Debug.Assert(_asyncEnumerableQueueTypeInfo?.IsConfigured == true, "must be populated before calling the method.");
-            JsonTypeInfo<Queue<T>> 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<T>)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<T> 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/...ibraries/System.Text.Json/src/System/Text/Json/Serialization/PreserveReferenceResolver.cs b/...ibraries/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);
             }