From 4f219fbc4c0a87b966c4a624d40b9b89c4a2b330 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 15:47:45 +0000 Subject: [PATCH 1/5] Initial plan From a048652682267ac12a1e5746caf0f63307d2caa8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 16:31:39 +0000 Subject: [PATCH 2/5] Fix ReadOnlyMemory/Memory converters to handle null during continuation Add IsContinuation check to prevent early return when resuming from continuation state with null tokens Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com> --- .../Converters/Collection/MemoryConverter.cs | 2 +- .../Collection/ReadOnlyMemoryConverter.cs | 2 +- .../CollectionTests/CollectionTests.Memory.cs | 79 +++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs index 2be0f0489000f2..f74ee631fe52a0 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs @@ -18,7 +18,7 @@ internal override bool OnTryRead( scoped ref ReadStack state, out Memory value) { - if (reader.TokenType is JsonTokenType.Null) + if (reader.TokenType is JsonTokenType.Null && !state.IsContinuation) { value = default; return true; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs index 6c6c079f6b878d..eef03dbc1e7a23 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs @@ -18,7 +18,7 @@ internal override bool OnTryRead( scoped ref ReadStack state, out ReadOnlyMemory value) { - if (reader.TokenType is JsonTokenType.Null) + if (reader.TokenType is JsonTokenType.Null && !state.IsContinuation) { value = default; return true; diff --git a/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs b/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs index b99428c39c7d6c..942c1a9ebba915 100644 --- a/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs +++ b/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Threading.Tasks; using Xunit; @@ -137,5 +138,83 @@ public async Task DeserializeMemoryByteClassAsync() MemoryOfTClass memoryOfByteClass = await Serializer.DeserializeWrapper>(json); AssertExtensions.SequenceEqual(s_testData.AsSpan(), memoryOfByteClass.Memory.Span); } + + [Fact] + public async Task DeserializeMemoryFromStreamWithNullElements() + { + // Regression test for https://github.com/dotnet/runtime/issues/118346 + // Tests that ReadOnlyMemory/Memory converters handle null elements correctly during streaming deserialization + if (StreamingSerializer is null) + { + return; + } + + string json = """[{"Name":"Alice"},null,{"Name":"Bob"}]"""; + using var stream = new Utf8MemoryStream(json); + + ReadOnlyMemory result = await StreamingSerializer.DeserializeWrapper>(stream); + + Assert.Equal(3, result.Length); + Assert.NotNull(result.Span[0]); + Assert.Equal("Alice", result.Span[0]!.Name); + Assert.Null(result.Span[1]); + Assert.NotNull(result.Span[2]); + Assert.Equal("Bob", result.Span[2]!.Name); + } + + [Fact] + public async Task DeserializeReadOnlyMemoryFromStreamWithNullElements() + { + // Regression test for https://github.com/dotnet/runtime/issues/118346 + // Tests that Memory converters handle null elements correctly during streaming deserialization + if (StreamingSerializer is null) + { + return; + } + + string json = """[{"Name":"Alice"},null,{"Name":"Bob"}]"""; + using var stream = new Utf8MemoryStream(json); + + Memory result = await StreamingSerializer.DeserializeWrapper>(stream); + + Assert.Equal(3, result.Length); + Assert.NotNull(result.Span[0]); + Assert.Equal("Alice", result.Span[0]!.Name); + Assert.Null(result.Span[1]); + Assert.NotNull(result.Span[2]); + Assert.Equal("Bob", result.Span[2]!.Name); + } + + [Fact] + public async Task DeserializeMemoryFromStreamManyElements() + { + // Regression test for https://github.com/dotnet/runtime/issues/118346 + // Tests that ReadOnlyMemory/Memory converters handle streaming deserialization with nulls + if (StreamingSerializer is null) + { + return; + } + + // Create a JSON array with nulls that may trigger continuation + string json = """[{"Name":"Alice"},null,{"Name":"Bob"},null,{"Name":"Charlie"}]"""; + using var stream = new Utf8MemoryStream(json); + + ReadOnlyMemory result = await StreamingSerializer.DeserializeWrapper>(stream); + + Assert.Equal(5, result.Length); + Assert.NotNull(result.Span[0]); + Assert.Equal("Alice", result.Span[0]!.Name); + Assert.Null(result.Span[1]); + Assert.NotNull(result.Span[2]); + Assert.Equal("Bob", result.Span[2]!.Name); + Assert.Null(result.Span[3]); + Assert.NotNull(result.Span[4]); + Assert.Equal("Charlie", result.Span[4]!.Name); + } + + public class SimpleClass + { + public string? Name { get; set; } + } } } From 803dcb6757bd9476a40ad77a2368f542d8b18507 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 18:47:53 +0000 Subject: [PATCH 3/5] Use ObjectState check for more comprehensive null handling during resumption Changed from !state.IsContinuation to state.Current.ObjectState == StackFrameObjectState.None for proper resumption handling. Added tests with 200 nulls and DefaultBufferSize=1 as requested. Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com> --- .../Converters/Collection/MemoryConverter.cs | 2 +- .../Collection/ReadOnlyMemoryConverter.cs | 2 +- .../CollectionTests/CollectionTests.Memory.cs | 74 ++++++------------- 3 files changed, 26 insertions(+), 52 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs index f74ee631fe52a0..7fa4ab83c19fa3 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs @@ -18,7 +18,7 @@ internal override bool OnTryRead( scoped ref ReadStack state, out Memory value) { - if (reader.TokenType is JsonTokenType.Null && !state.IsContinuation) + if (reader.TokenType is JsonTokenType.Null && state.Current.ObjectState == StackFrameObjectState.None) { value = default; return true; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs index eef03dbc1e7a23..88e28dee9a4be9 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs @@ -18,7 +18,7 @@ internal override bool OnTryRead( scoped ref ReadStack state, out ReadOnlyMemory value) { - if (reader.TokenType is JsonTokenType.Null && !state.IsContinuation) + if (reader.TokenType is JsonTokenType.Null && state.Current.ObjectState == StackFrameObjectState.None) { value = default; return true; diff --git a/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs b/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs index 942c1a9ebba915..79796e2873cc69 100644 --- a/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs +++ b/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs @@ -140,81 +140,55 @@ public async Task DeserializeMemoryByteClassAsync() } [Fact] - public async Task DeserializeMemoryFromStreamWithNullElements() + public async Task DeserializeReadOnlyMemoryFromStreamWithManyNulls() { // Regression test for https://github.com/dotnet/runtime/issues/118346 - // Tests that ReadOnlyMemory/Memory converters handle null elements correctly during streaming deserialization + // Tests ReadOnlyMemory converters handle null elements during streaming with small buffer if (StreamingSerializer is null) { return; } - string json = """[{"Name":"Alice"},null,{"Name":"Bob"}]"""; + // Create an array of ~200 null elements to force resumptions with smallest buffer size + var nullElements = new string[200]; + Array.Fill(nullElements, "null"); + string json = $"[{string.Join(",", nullElements)}]"; using var stream = new Utf8MemoryStream(json); - ReadOnlyMemory result = await StreamingSerializer.DeserializeWrapper>(stream); + JsonSerializerOptions options = new() { DefaultBufferSize = 1 }; + ReadOnlyMemory result = await StreamingSerializer.DeserializeWrapper>(stream, options); - Assert.Equal(3, result.Length); - Assert.NotNull(result.Span[0]); - Assert.Equal("Alice", result.Span[0]!.Name); - Assert.Null(result.Span[1]); - Assert.NotNull(result.Span[2]); - Assert.Equal("Bob", result.Span[2]!.Name); - } - - [Fact] - public async Task DeserializeReadOnlyMemoryFromStreamWithNullElements() - { - // Regression test for https://github.com/dotnet/runtime/issues/118346 - // Tests that Memory converters handle null elements correctly during streaming deserialization - if (StreamingSerializer is null) + Assert.Equal(200, result.Length); + for (int i = 0; i < 200; i++) { - return; + Assert.Null(result.Span[i]); } - - string json = """[{"Name":"Alice"},null,{"Name":"Bob"}]"""; - using var stream = new Utf8MemoryStream(json); - - Memory result = await StreamingSerializer.DeserializeWrapper>(stream); - - Assert.Equal(3, result.Length); - Assert.NotNull(result.Span[0]); - Assert.Equal("Alice", result.Span[0]!.Name); - Assert.Null(result.Span[1]); - Assert.NotNull(result.Span[2]); - Assert.Equal("Bob", result.Span[2]!.Name); } [Fact] - public async Task DeserializeMemoryFromStreamManyElements() + public async Task DeserializeMemoryFromStreamWithManyNulls() { // Regression test for https://github.com/dotnet/runtime/issues/118346 - // Tests that ReadOnlyMemory/Memory converters handle streaming deserialization with nulls + // Tests Memory converters handle null elements during streaming with small buffer if (StreamingSerializer is null) { return; } - // Create a JSON array with nulls that may trigger continuation - string json = """[{"Name":"Alice"},null,{"Name":"Bob"},null,{"Name":"Charlie"}]"""; + // Create an array of ~200 null elements to force resumptions with smallest buffer size + var nullElements = new string[200]; + Array.Fill(nullElements, "null"); + string json = $"[{string.Join(",", nullElements)}]"; using var stream = new Utf8MemoryStream(json); - ReadOnlyMemory result = await StreamingSerializer.DeserializeWrapper>(stream); + JsonSerializerOptions options = new() { DefaultBufferSize = 1 }; + Memory result = await StreamingSerializer.DeserializeWrapper>(stream, options); - Assert.Equal(5, result.Length); - Assert.NotNull(result.Span[0]); - Assert.Equal("Alice", result.Span[0]!.Name); - Assert.Null(result.Span[1]); - Assert.NotNull(result.Span[2]); - Assert.Equal("Bob", result.Span[2]!.Name); - Assert.Null(result.Span[3]); - Assert.NotNull(result.Span[4]); - Assert.Equal("Charlie", result.Span[4]!.Name); - } - - public class SimpleClass - { - public string? Name { get; set; } + Assert.Equal(200, result.Length); + for (int i = 0; i < 200; i++) + { + Assert.Null(result.Span[i]); + } } } } From 932d87b6eff3255febec915060b6d8c0856c14b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 16:49:50 +0000 Subject: [PATCH 4/5] Use Enumerable.Repeat for test data generation Simplified null array generation using Enumerable.Repeat instead of Array.Fill as suggested in code review. Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com> --- .../Common/CollectionTests/CollectionTests.Memory.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs b/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs index 79796e2873cc69..b39b610a80f9a5 100644 --- a/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs +++ b/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Xunit; @@ -150,9 +151,7 @@ public async Task DeserializeReadOnlyMemoryFromStreamWithManyNulls() } // Create an array of ~200 null elements to force resumptions with smallest buffer size - var nullElements = new string[200]; - Array.Fill(nullElements, "null"); - string json = $"[{string.Join(",", nullElements)}]"; + string json = $"[{string.Join(",", Enumerable.Repeat("null", 200))}]"; using var stream = new Utf8MemoryStream(json); JsonSerializerOptions options = new() { DefaultBufferSize = 1 }; @@ -176,9 +175,7 @@ public async Task DeserializeMemoryFromStreamWithManyNulls() } // Create an array of ~200 null elements to force resumptions with smallest buffer size - var nullElements = new string[200]; - Array.Fill(nullElements, "null"); - string json = $"[{string.Join(",", nullElements)}]"; + string json = $"[{string.Join(",", Enumerable.Repeat("null", 200))}]"; using var stream = new Utf8MemoryStream(json); JsonSerializerOptions options = new() { DefaultBufferSize = 1 }; From e05cdf8d64bd52a2f2fffe782393eece587f77cb Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 7 Nov 2025 20:01:18 +0200 Subject: [PATCH 5/5] Apply the correct fix. --- .../Converters/Collection/MemoryConverter.cs | 2 +- .../Collection/ReadOnlyMemoryConverter.cs | 2 +- .../CollectionTests/CollectionTests.Memory.cs | 18 ++---------------- .../Serialization/CollectionTests.cs | 4 ++++ 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs index 7fa4ab83c19fa3..26cbeb7a25fc61 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs @@ -18,7 +18,7 @@ internal override bool OnTryRead( scoped ref ReadStack state, out Memory value) { - if (reader.TokenType is JsonTokenType.Null && state.Current.ObjectState == StackFrameObjectState.None) + if (reader.TokenType is JsonTokenType.Null && state.Current.ReturnValue is null) { value = default; return true; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs index 88e28dee9a4be9..be069d580d8275 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs @@ -18,7 +18,7 @@ internal override bool OnTryRead( scoped ref ReadStack state, out ReadOnlyMemory value) { - if (reader.TokenType is JsonTokenType.Null && state.Current.ObjectState == StackFrameObjectState.None) + if (reader.TokenType is JsonTokenType.Null && state.Current.ReturnValue is null) { value = default; return true; diff --git a/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs b/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs index b39b610a80f9a5..26331175eeeb46 100644 --- a/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs +++ b/src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Memory.cs @@ -144,7 +144,6 @@ public async Task DeserializeMemoryByteClassAsync() public async Task DeserializeReadOnlyMemoryFromStreamWithManyNulls() { // Regression test for https://github.com/dotnet/runtime/issues/118346 - // Tests ReadOnlyMemory converters handle null elements during streaming with small buffer if (StreamingSerializer is null) { return; @@ -153,22 +152,15 @@ public async Task DeserializeReadOnlyMemoryFromStreamWithManyNulls() // Create an array of ~200 null elements to force resumptions with smallest buffer size string json = $"[{string.Join(",", Enumerable.Repeat("null", 200))}]"; using var stream = new Utf8MemoryStream(json); + ReadOnlyMemory result = await StreamingSerializer.DeserializeWrapper>(stream); - JsonSerializerOptions options = new() { DefaultBufferSize = 1 }; - ReadOnlyMemory result = await StreamingSerializer.DeserializeWrapper>(stream, options); - Assert.Equal(200, result.Length); - for (int i = 0; i < 200; i++) - { - Assert.Null(result.Span[i]); - } } [Fact] public async Task DeserializeMemoryFromStreamWithManyNulls() { // Regression test for https://github.com/dotnet/runtime/issues/118346 - // Tests Memory converters handle null elements during streaming with small buffer if (StreamingSerializer is null) { return; @@ -177,15 +169,9 @@ public async Task DeserializeMemoryFromStreamWithManyNulls() // Create an array of ~200 null elements to force resumptions with smallest buffer size string json = $"[{string.Join(",", Enumerable.Repeat("null", 200))}]"; using var stream = new Utf8MemoryStream(json); + Memory result = await StreamingSerializer.DeserializeWrapper>(stream); - JsonSerializerOptions options = new() { DefaultBufferSize = 1 }; - Memory result = await StreamingSerializer.DeserializeWrapper>(stream, options); - Assert.Equal(200, result.Length); - for (int i = 0; i < 200; i++) - { - Assert.Null(result.Span[i]); - } } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/CollectionTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/CollectionTests.cs index cb88437b6b552a..0551d24f6438cb 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/CollectionTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/CollectionTests.cs @@ -461,6 +461,8 @@ public async Task DeserializeAsyncEnumerable() [JsonSerializable(typeof(StackWrapper))] [JsonSerializable(typeof(ClassWithRecursiveCollectionTypes))] [JsonSerializable(typeof(MemoryOfTClass))] + [JsonSerializable(typeof(ReadOnlyMemory))] + [JsonSerializable(typeof(Memory))] [JsonSerializable(typeof(ReadOnlyMemoryOfTClass))] [JsonSerializable(typeof(MemoryOfTClass))] [JsonSerializable(typeof(ReadOnlyMemoryOfTClass))] @@ -883,6 +885,8 @@ public CollectionTests_Default() [JsonSerializable(typeof(StackWrapper))] [JsonSerializable(typeof(ClassWithRecursiveCollectionTypes))] [JsonSerializable(typeof(MemoryOfTClass))] + [JsonSerializable(typeof(ReadOnlyMemory))] + [JsonSerializable(typeof(Memory))] [JsonSerializable(typeof(ReadOnlyMemoryOfTClass))] [JsonSerializable(typeof(MemoryOfTClass))] [JsonSerializable(typeof(ReadOnlyMemoryOfTClass))]