diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs index 4a2be44c8629c7..89b9cdad1eeffa 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs @@ -312,7 +312,9 @@ internal sealed override bool OnTryWrite( if (!jsonPropertyInfo.GetMemberAndWriteJson(objectValue!, ref state, writer)) { - Debug.Assert(jsonPropertyInfo.ConverterBase.ConverterStrategy != ConverterStrategy.Value); + Debug.Assert(jsonPropertyInfo.ConverterBase.ConverterStrategy != ConverterStrategy.Value || + jsonPropertyInfo.ConverterBase.TypeToConvert == JsonTypeInfo.ObjectType); + return false; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs index 6ea73dd49c9ec5..543fbe2f5229aa 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs @@ -22,7 +22,7 @@ protected internal JsonConverter() // In the future, this will be check for !IsSealed (and excluding value types). CanBePolymorphic = TypeToConvert == JsonTypeInfo.ObjectType; IsValueType = TypeToConvert.IsValueType; - CanBeNull = !IsValueType || TypeToConvert.IsNullableOfT(); + CanBeNull = default(T) is null; IsInternalConverter = GetType().Assembly == typeof(JsonConverter).Assembly; if (HandleNull) @@ -220,7 +220,12 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali // Remember if we were a continuation here since Push() may affect IsContinuation. bool wasContinuation = state.IsContinuation; +#if DEBUG + // DEBUG: ensure push/pop operations preserve stack integrity + JsonTypeInfo originalJsonTypeInfo = state.Current.JsonTypeInfo; +#endif state.Push(); + Debug.Assert(TypeToConvert.IsAssignableFrom(state.Current.JsonTypeInfo.Type)); #if !DEBUG // For performance, only perform validation on internal converters on debug builds. @@ -257,6 +262,9 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali value = default; state.Pop(true); +#if DEBUG + Debug.Assert(ReferenceEquals(originalJsonTypeInfo, state.Current.JsonTypeInfo)); +#endif return true; } @@ -288,6 +296,9 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali } state.Pop(success); +#if DEBUG + Debug.Assert(ReferenceEquals(originalJsonTypeInfo, state.Current.JsonTypeInfo)); +#endif return success; } @@ -298,6 +309,9 @@ internal override sealed bool TryReadAsObject(ref Utf8JsonReader reader, JsonSer return success; } + /// + /// Overridden by the nullable converter to prevent boxing of values by the JIT. + /// internal virtual bool IsNull(in T value) => value == null; internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions options, ref WriteStack state) @@ -307,7 +321,7 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions ThrowHelper.ThrowJsonException_SerializerCycleDetected(options.EffectiveMaxDepth); } - if (CanBeNull && !HandleNullOnWrite && IsNull(value)) + if (default(T) is null && !HandleNullOnWrite && IsNull(value)) { // We do not pass null values to converters unless HandleNullOnWrite is true. Null values for properties were // already handled in GetMemberAndWriteJson() so we don't need to check for IgnoreNullValues here. @@ -317,81 +331,76 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions bool ignoreCyclesPopReference = false; - if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && - // .NET types that are serialized as JSON primitive values don't need to be tracked for cycle detection e.g: string. - ConverterStrategy != ConverterStrategy.Value && - !IsValueType && !IsNull(value)) + if ( +#if NET6_0_OR_GREATER + !typeof(T).IsValueType && // treated as a constant by recent versions of the JIT. +#else + !IsValueType && +#endif + value is not null) { - // Custom (user) converters shall not track references - // it is responsibility of the user to break cycles in case there's any - // if we compare against Preserve, objects don't get preserved when a custom converter exists - // given that the custom converter executes prior to the preserve logic. - Debug.Assert(IsInternalConverter); - Debug.Assert(value != null); - - ReferenceResolver resolver = state.ReferenceResolver; - - // Write null to break reference cycles. - if (resolver.ContainsReferenceForCycleDetection(value)) - { - writer.WriteNullValue(); - return true; - } - // For boxed reference types: do not push when boxed in order to avoid false positives - // when we run the ContainsReferenceForCycleDetection check for the converter of the unboxed value. - Debug.Assert(!CanBePolymorphic); - resolver.PushReferenceForCycleDetection(value); - ignoreCyclesPopReference = true; - } - - if (CanBePolymorphic) - { - if (value == null) + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && + // .NET types that are serialized as JSON primitive values don't need to be tracked for cycle detection e.g: string. + ConverterStrategy != ConverterStrategy.Value) { - Debug.Assert(ConverterStrategy == ConverterStrategy.Value); - Debug.Assert(!state.IsContinuation); - Debug.Assert(HandleNullOnWrite); + // Custom (user) converters shall not track references + // it is responsibility of the user to break cycles in case there's any + // if we compare against Preserve, objects don't get preserved when a custom converter exists + // given that the custom converter executes prior to the preserve logic. + Debug.Assert(IsInternalConverter); - int originalPropertyDepth = writer.CurrentDepth; - Write(writer, value, options); - VerifyWrite(originalPropertyDepth, writer); + ReferenceResolver resolver = state.ReferenceResolver; - return true; - } + // Write null to break reference cycles. + if (resolver.ContainsReferenceForCycleDetection(value)) + { + writer.WriteNullValue(); + return true; + } - Type type = value.GetType(); - if (type == JsonTypeInfo.ObjectType) - { - writer.WriteStartObject(); - writer.WriteEndObject(); - return true; + // For boxed reference types: do not push when boxed in order to avoid false positives + // when we run the ContainsReferenceForCycleDetection check for the converter of the unboxed value. + Debug.Assert(!CanBePolymorphic); + resolver.PushReferenceForCycleDetection(value); + ignoreCyclesPopReference = true; } - if (type != TypeToConvert && IsInternalConverter) + if (CanBePolymorphic) { - // For internal converter only: Handle polymorphic case and get the new converter. - // Custom converter, even though polymorphic converter, get called for reading AND writing. - JsonConverter jsonConverter = state.Current.InitializeReEntry(type, options); - Debug.Assert(jsonConverter != this); - - if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && - jsonConverter.IsValueType) + Type type = value.GetType(); + if (type == JsonTypeInfo.ObjectType) { - // For boxed value types: push the value before it gets unboxed on TryWriteAsObject. - state.ReferenceResolver.PushReferenceForCycleDetection(value); - ignoreCyclesPopReference = true; + writer.WriteStartObject(); + writer.WriteEndObject(); + return true; } - // We found a different converter; forward to that. - bool success2 = jsonConverter.TryWriteAsObject(writer, value, options, ref state); - - if (ignoreCyclesPopReference) + if (type != TypeToConvert && IsInternalConverter) { - state.ReferenceResolver.PopReferenceForCycleDetection(); - } + // For internal converter only: Handle polymorphic case and get the new converter. + // Custom converter, even though polymorphic converter, get called for reading AND writing. + JsonConverter jsonConverter = state.Current.InitializeReEntry(type, options); + Debug.Assert(jsonConverter != this); + + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && + jsonConverter.IsValueType) + { + // For boxed value types: push the value before it gets unboxed on TryWriteAsObject. + state.ReferenceResolver.PushReferenceForCycleDetection(value); + ignoreCyclesPopReference = true; + } + + // We found a different converter; forward to that. + bool success2 = jsonConverter.TryWriteAsObject(writer, value, options, ref state); - return success2; + if (ignoreCyclesPopReference) + { + state.ReferenceResolver.PopReferenceForCycleDetection(); + } + + return success2; + } } } @@ -416,7 +425,12 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions bool isContinuation = state.IsContinuation; +#if DEBUG + // DEBUG: ensure push/pop operations preserve stack integrity + JsonTypeInfo originalJsonTypeInfo = state.Current.JsonTypeInfo; +#endif state.Push(); + Debug.Assert(TypeToConvert.IsAssignableFrom(state.Current.JsonTypeInfo.Type)); if (!isContinuation) { @@ -432,6 +446,9 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions } state.Pop(success); +#if DEBUG + Debug.Assert(ReferenceEquals(originalJsonTypeInfo, state.Current.JsonTypeInfo)); +#endif if (ignoreCyclesPopReference) { @@ -476,6 +493,7 @@ internal bool TryWriteDataExtensionProperty(Utf8JsonWriter writer, T value, Json // Ignore the naming policy for extension data. state.Current.IgnoreDictionaryKeyPolicy = true; + state.Current.DeclaredJsonPropertyInfo = state.Current.JsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo; success = dictionaryConverter.OnWriteResume(writer, value, options, ref state); if (success) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.String.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.String.cs index 71c30c512756c6..db52ffc079ffe3 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.String.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.String.cs @@ -125,9 +125,6 @@ private static string WriteUsingMetadata(in TValue value, JsonTypeInfo? throw new ArgumentNullException(nameof(jsonTypeInfo)); } - WriteStack state = default; - state.Initialize(jsonTypeInfo, supportContinuation: false); - JsonSerializerOptions options = jsonTypeInfo.Options; using (var output = new PooledByteBufferWriter(options.DefaultBufferSize)) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs index 911ee9f3ede5ff..0f2ff8574f08df 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs @@ -234,11 +234,17 @@ internal override bool GetMemberAndWriteJson(object obj, ref WriteStack state, U { T value = Get!(obj); - if (Options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && - value != null && + if ( +#if NET6_0_OR_GREATER + !typeof(T).IsValueType && // treated as a constant by recent versions of the JIT. +#else + !Converter.IsValueType && +#endif + Options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && + value is not null && // .NET types that are serialized as JSON primitive values don't need to be tracked for cycle detection e.g: string. // However JsonConverter that uses ConverterStrategy == Value is an exception. - (Converter.CanBePolymorphic || (!Converter.IsValueType && ConverterStrategy != ConverterStrategy.Value)) && + (Converter.CanBePolymorphic || ConverterStrategy != ConverterStrategy.Value) && state.ReferenceResolver.ContainsReferenceForCycleDetection(value)) { // If a reference cycle is detected, treat value as null. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs index 2f0347f68332ae..c78646f45f4b16 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs @@ -16,16 +16,24 @@ internal struct ReadStack internal static readonly char[] SpecialCharacters = { '.', ' ', '\'', '/', '"', '[', ']', '(', ')', '\t', '\n', '\r', '\f', '\b', '\\', '\u0085', '\u2028', '\u2029' }; /// - /// The number of stack frames when the continuation started. + /// Exposes the stackframe that is currently active. /// - private int _continuationCount; + public ReadStackFrame Current; + + /// + /// Buffer containing all frames in the stack. For performance it is only populated for serialization depths > 1. + /// + private ReadStackFrame[] _stack; /// - /// The number of stack frames including Current. _previous will contain _count-1 higher frames. + /// Tracks the current depth of the stack. /// private int _count; - private List _previous; + /// + /// If not zero, indicates that the stack is part of a re-entrant continuation of given depth. + /// + private int _continuationCount; // State cache when deserializing objects with parameterized constructors. private List? _ctorArgStateCache; @@ -35,11 +43,10 @@ internal struct ReadStack /// public long BytesConsumed; - // A field is used instead of a property to avoid value semantics. - public ReadStackFrame Current; - + /// + /// Indicates that the state still contains suspended frames waiting re-entry. + /// public bool IsContinuation => _continuationCount != 0; - public bool IsLastContinuation => _continuationCount == _count; /// /// Internal flag to let us know that we need to read ahead in the inner read loop. @@ -59,25 +66,19 @@ internal struct ReadStack /// public bool UseFastPath; - private void AddCurrent() + /// + /// Ensures that the stack buffer has sufficient capacity to hold an additional frame. + /// + private void EnsurePushCapacity() { - if (_previous == null) - { - _previous = new List(); - } - - if (_count > _previous.Count) + if (_stack is null) { - // Need to allocate a new array element. - _previous.Add(Current); + _stack = new ReadStackFrame[4]; } - else + else if (_count - 1 == _stack.Length) { - // Use a previously allocated slot. - _previous[_count - 1] = Current; + Array.Resize(ref _stack, 2 * _stack.Length); } - - _count++; } public void Initialize(Type type, JsonSerializerOptions options, bool supportContinuation) @@ -143,8 +144,10 @@ public void Push() jsonTypeInfo = Current.JsonTypeInfo.ElementTypeInfo!; } - AddCurrent(); - Current.Reset(); + EnsurePushCapacity(); + _stack[_count - 1] = Current; + Current = default; + _count++; Current.JsonTypeInfo = jsonTypeInfo; Current.JsonPropertyInfo = jsonTypeInfo.PropertyInfoForTypeInfo; @@ -152,29 +155,26 @@ public void Push() Current.NumberHandling = numberHandling ?? Current.JsonPropertyInfo.NumberHandling; } } - else if (_continuationCount == 1) - { - // No need for a push since there is only one stack frame. - Debug.Assert(_count == 1); - _continuationCount = 0; - } else { - // A continuation; adjust the index. - Current = _previous[_count - 1]; - - // Check if we are done. - if (_count == _continuationCount) + // We are re-entering a continuation, adjust indices accordingly + if (_count++ > 0) { - _continuationCount = 0; + Current = _stack[_count - 1]; } - else + + // check if we are done + if (_continuationCount == _count) { - _count++; + _continuationCount = 0; } } SetConstructorArgumentState(); +#if DEBUG + // Ensure the method is always exercised in debug builds. + _ = JsonPath(); +#endif } public void Pop(bool success) @@ -188,41 +188,34 @@ public void Pop(bool success) { if (_count == 1) { - // No need for a continuation since there is only one stack frame. + // No need to copy any frames here. _continuationCount = 1; - } - else - { - AddCurrent(); - _count--; - _continuationCount = _count; - _count--; - Current = _previous[_count - 1]; + _count = 0; + return; } - return; + // Need to push the Current frame to the stack, + // ensure that we have sufficient capacity. + EnsurePushCapacity(); + _continuationCount = _count--; } - - if (_continuationCount == 1) + else if (--_count == 0) { - // No need for a pop since there is only one stack frame. - Debug.Assert(_count == 1); + // reached the root, no need to copy frames. return; } - // Update the list entry to the current value. - _previous[_count - 1] = Current; - - Debug.Assert(_count > 0); + _stack[_count] = Current; + Current = _stack[_count - 1]; } else { Debug.Assert(_continuationCount == 0); - } - if (_count > 1) - { - Current = _previous[--_count -1]; + if (--_count > 0) + { + Current = _stack[_count - 1]; + } } SetConstructorArgumentState(); @@ -240,26 +233,25 @@ public string JsonPath() for (int i = 0; i < count - 1; i++) { - AppendStackFrame(sb, _previous[i]); + AppendStackFrame(sb, ref _stack[i]); } if (_continuationCount == 0) { - AppendStackFrame(sb, Current); + AppendStackFrame(sb, ref Current); } return sb.ToString(); - static void AppendStackFrame(StringBuilder sb, in ReadStackFrame frame) + static void AppendStackFrame(StringBuilder sb, ref ReadStackFrame frame) { // Append the property name. - string? propertyName = GetPropertyName(frame); + string? propertyName = GetPropertyName(ref frame); AppendPropertyName(sb, propertyName); if (frame.JsonTypeInfo != null && frame.IsProcessingEnumerable()) { - IEnumerable? enumerable = (IEnumerable?)frame.ReturnValue; - if (enumerable == null) + if (frame.ReturnValue is not IEnumerable enumerable) { return; } @@ -276,7 +268,7 @@ static void AppendStackFrame(StringBuilder sb, in ReadStackFrame frame) } } - static int GetCount(IEnumerable enumerable) + static int GetCount(IEnumerable enumerable) { if (enumerable is ICollection collection) { @@ -311,7 +303,7 @@ static void AppendPropertyName(StringBuilder sb, string? propertyName) } } - static string? GetPropertyName(in ReadStackFrame frame) + static string? GetPropertyName(ref ReadStackFrame frame) { string? propertyName = null; @@ -350,10 +342,7 @@ private void SetConstructorArgumentState() // A zero index indicates a new stack frame. if (Current.CtorArgumentStateIndex == 0) { - if (_ctorArgStateCache == null) - { - _ctorArgStateCache = new List(); - } + _ctorArgStateCache ??= new List(); var newState = new ArgumentState(); _ctorArgStateCache.Add(newState); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs index f4f64dcbb1a51d..137305f1d5b431 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs @@ -89,20 +89,5 @@ public bool IsProcessingEnumerable() { return (JsonTypeInfo.PropertyInfoForTypeInfo.ConverterStrategy & ConverterStrategy.Enumerable) != 0; } - - public void Reset() - { - CtorArgumentStateIndex = 0; - CtorArgumentState = null; - JsonTypeInfo = null!; - ObjectState = StackFrameObjectState.None; - OriginalDepth = 0; - OriginalTokenType = JsonTokenType.None; - PropertyIndex = 0; - PropertyRefCache = null; - ReturnValue = null; - - EndProperty(); - } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs index 22f67983ec3463..e9a9f1e9e75460 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs @@ -16,15 +16,25 @@ namespace System.Text.Json internal struct WriteStack { /// - /// The number of stack frames when the continuation started. + /// Exposes the stackframe that is currently active. /// - private int _continuationCount; + public WriteStackFrame Current; + + /// + /// Buffer containing all frames in the stack. For performance it is only populated for serialization depths > 1. + /// + private WriteStackFrame[] _stack; /// - /// The number of stack frames including Current. _previous will contain _count-1 higher frames. + /// Tracks the current depth of the stack. /// private int _count; + /// + /// If not zero, indicates that the stack is part of a re-entrant continuation of given depth. + /// + private int _continuationCount; + /// /// Cancellation token used by converters performing async serialization (e.g. IAsyncEnumerable) /// @@ -41,17 +51,15 @@ internal struct WriteStack /// public List? PendingAsyncDisposables; - private List _previous; - - // A field is used instead of a property to avoid value semantics. - public WriteStackFrame Current; - /// /// The amount of bytes to write before the underlying Stream should be flushed and the /// current buffer adjusted to remove the processed bytes. /// public int FlushThreshold; + /// + /// Indicates that the state still contains suspended frames waiting re-entry. + /// public bool IsContinuation => _continuationCount != 0; // The bag of preservable references. @@ -62,25 +70,16 @@ internal struct WriteStack /// public bool SupportContinuation; - private void AddCurrent() + private void EnsurePushCapacity() { - if (_previous == null) + if (_stack is null) { - _previous = new List(); + _stack = new WriteStackFrame[4]; } - - if (_count > _previous.Count) + else if (_count - 1 == _stack.Length) { - // Need to allocate a new array element. - _previous.Add(Current); + Array.Resize(ref _stack, 2 * _stack.Length); } - else - { - // Use a previously allocated slot. - _previous[_count - 1] = Current; - } - - _count++; } /// @@ -125,8 +124,10 @@ public void Push() JsonTypeInfo jsonTypeInfo = Current.GetPolymorphicJsonPropertyInfo().RuntimeTypeInfo; JsonNumberHandling? numberHandling = Current.NumberHandling; - AddCurrent(); - Current.Reset(); + EnsurePushCapacity(); + _stack[_count - 1] = Current; + Current = default; + _count++; Current.JsonTypeInfo = jsonTypeInfo; Current.DeclaredJsonPropertyInfo = jsonTypeInfo.PropertyInfoForTypeInfo; @@ -134,27 +135,25 @@ public void Push() Current.NumberHandling = numberHandling ?? Current.DeclaredJsonPropertyInfo.NumberHandling; } } - else if (_continuationCount == 1) - { - // No need for a push since there is only one stack frame. - Debug.Assert(_count == 1); - _continuationCount = 0; - } else { - // A continuation, adjust the index. - Current = _previous[_count - 1]; - - // Check if we are done. - if (_count == _continuationCount) + // We are re-entering a continuation, adjust indices accordingly + if (_count++ > 0) { - _continuationCount = 0; + Current = _stack[_count - 1]; } - else + + // check if we are done + if (_continuationCount == _count) { - _count++; + _continuationCount = 0; } } + +#if DEBUG + // Ensure the method is always exercised in debug builds. + _ = PropertyPath(); +#endif } public void Pop(bool success) @@ -168,33 +167,25 @@ public void Pop(bool success) { if (_count == 1) { - // No need for a continuation since there is only one stack frame. + // No need to copy any frames here. _continuationCount = 1; - _count = 1; - } - else - { - AddCurrent(); - _count--; - _continuationCount = _count; - _count--; - Current = _previous[_count - 1]; + _count = 0; + return; } - return; + // Need to push the Current frame to the stack, + // ensure that we have sufficient capacity. + EnsurePushCapacity(); + _continuationCount = _count--; } - - if (_continuationCount == 1) + else if (--_count == 0) { - // No need for a pop since there is only one stack frame. - Debug.Assert(_count == 1); + // reached the root, no need to copy frames. return; } - // Update the list entry to the current value. - _previous[_count - 1] = Current; - - Debug.Assert(_count > 0); + _stack[_count] = Current; + Current = _stack[_count - 1]; } else { @@ -207,11 +198,11 @@ public void Pop(bool success) PendingAsyncDisposables ??= new List(); PendingAsyncDisposables.Add(Current.AsyncEnumerator); } - } - if (_count > 1) - { - Current = _previous[--_count - 1]; + if (--_count > 0) + { + Current = _stack[_count - 1]; + } } } @@ -253,13 +244,10 @@ public void DisposePendingDisposablesOnException() DisposeFrame(Current.CollectionEnumerator, ref exception); int stackSize = Math.Max(_count, _continuationCount); - if (stackSize > 1) + for (int i = 0; i < stackSize - 1; i++) { - for (int i = 0; i < stackSize - 1; i++) - { - Debug.Assert(_previous[i].AsyncEnumerator is null); - DisposeFrame(_previous[i].CollectionEnumerator, ref exception); - } + Debug.Assert(_stack[i].AsyncEnumerator is null); + DisposeFrame(_stack[i].CollectionEnumerator, ref exception); } if (exception is not null) @@ -294,12 +282,9 @@ public async ValueTask DisposePendingDisposablesOnExceptionAsync() exception = await DisposeFrame(Current.CollectionEnumerator, Current.AsyncEnumerator, exception).ConfigureAwait(false); int stackSize = Math.Max(_count, _continuationCount); - if (stackSize > 1) + for (int i = 0; i < stackSize - 1; i++) { - for (int i = 0; i < stackSize - 1; i++) - { - exception = await DisposeFrame(_previous[i].CollectionEnumerator, _previous[i].AsyncEnumerator, exception).ConfigureAwait(false); - } + exception = await DisposeFrame(_stack[i].CollectionEnumerator, _stack[i].AsyncEnumerator, exception).ConfigureAwait(false); } if (exception is not null) @@ -343,17 +328,17 @@ public string PropertyPath() for (int i = 0; i < count - 1; i++) { - AppendStackFrame(sb, _previous[i]); + AppendStackFrame(sb, ref _stack[i]); } if (_continuationCount == 0) { - AppendStackFrame(sb, Current); + AppendStackFrame(sb, ref Current); } return sb.ToString(); - void AppendStackFrame(StringBuilder sb, in WriteStackFrame frame) + static void AppendStackFrame(StringBuilder sb, ref WriteStackFrame frame) { // Append the property name. string? propertyName = frame.DeclaredJsonPropertyInfo?.MemberInfo?.Name; @@ -366,7 +351,7 @@ void AppendStackFrame(StringBuilder sb, in WriteStackFrame frame) AppendPropertyName(sb, propertyName); } - void AppendPropertyName(StringBuilder sb, string? propertyName) + static void AppendPropertyName(StringBuilder sb, string? propertyName) { if (propertyName != null) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs index b162b55709c300..7d9ed3b6863f34 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs @@ -120,20 +120,5 @@ public JsonConverter InitializeReEntry(Type type, JsonSerializerOptions options) return PolymorphicJsonPropertyInfo.ConverterBase; } - - public void Reset() - { - CollectionEnumerator = null; - EnumeratorIndex = 0; - AsyncEnumerator = null; - AsyncEnumeratorIsPendingCompletion = false; - IgnoreDictionaryKeyPolicy = false; - JsonTypeInfo = null!; - OriginalDepth = 0; - ProcessedStartToken = false; - ProcessedEndToken = false; - - EndProperty(); - } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs index 8157f9bbdf3c99..235fa8010deb87 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs @@ -14,7 +14,7 @@ namespace System.Text.Json.Serialization.Tests public class ReferenceHandlerTests_IgnoreCycles { private static readonly JsonSerializerOptions s_optionsIgnoreCycles = - new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.IgnoreCycles }; + new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.IgnoreCycles, DefaultBufferSize = 1 }; [Fact] public async Task IgnoreCycles_OnObject() @@ -401,6 +401,25 @@ public async void IgnoreCycles_BoxedValueShouldNotBeIgnored() await Test_Serialize_And_SerializeAsync_Contains(root, expectedSubstring: @"""DayOfBirth"":15", expectedTimes: 2, s_optionsIgnoreCycles); } + [Fact] + public async Task CycleDetectionStatePersistsAcrossContinuations() + { + string expectedValueJson = @"{""LargePropertyName"":""A large-ish string to force continuations"",""Nested"":null}"; + var recVal = new RecursiveValue { LargePropertyName = "A large-ish string to force continuations" }; + recVal.Nested = recVal; + + var value = new List { recVal, recVal }; + string expectedJson = $"[{expectedValueJson},{expectedValueJson}]"; + + await Test_Serialize_And_SerializeAsync(value, expectedJson, s_optionsIgnoreCycles); + } + + public class RecursiveValue + { + public string LargePropertyName { get; set; } + public RecursiveValue? Nested { get; set; } + } + private async Task Test_Serialize_And_SerializeAsync(T obj, string expected, JsonSerializerOptions options) { string json;