Skip to content

Commit adabae6

Browse files
brianrobCopilot
andcommitted
Fix NullReferenceException in SegmentedList.Add during large heap snapshots
SegmentedMemoryStreamWriter.Clear() was replacing the backing bytes list with a brand-new empty SegmentedList, discarding the pre-allocated buffer (potentially ~1GB for 128M-node graphs). The replacement started with items=null and capacity=0, requiring re-growth from scratch under extreme memory pressure, which caused NullReferenceException in SegmentedList.Add. The fix resets bytes.Count to 0 instead, preserving the pre-allocated segments. This is safe because SegmentedList.Add() checks count < capacity and writes directly into existing segments. This pattern is already used in Graph.ClearWorker() (m_nodes.Count = 0, m_types.Count = 0). Also fixes a segment-size inconsistency: the constructor used 65_536 but the old Clear() was creating a new list with 131_072. Fixes #951 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5ef79ab commit adabae6

2 files changed

Lines changed: 113 additions & 1 deletion

File tree

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
using FastSerialization;
2+
using Xunit;
3+
4+
namespace FastSerializationTests
5+
{
6+
/// <summary>
7+
/// Tests for SegmentedMemoryStreamWriter, focusing on Clear() semantics.
8+
/// </summary>
9+
public class SegmentedMemoryStreamWriterTests
10+
{
11+
[Fact]
12+
public void Clear_ResetsLengthToZero()
13+
{
14+
var writer = new SegmentedMemoryStreamWriter(64, SerializationSettings.Default);
15+
16+
writer.Write((byte)1);
17+
writer.Write((byte)2);
18+
writer.Write((byte)3);
19+
Assert.Equal(3, writer.Length);
20+
21+
writer.Clear();
22+
Assert.Equal(0, writer.Length);
23+
}
24+
25+
[Fact]
26+
public void Clear_AllowsWritingNewData()
27+
{
28+
var writer = new SegmentedMemoryStreamWriter(64, SerializationSettings.Default);
29+
30+
writer.Write((int)100);
31+
writer.Write((int)200);
32+
33+
writer.Clear();
34+
writer.Write((byte)42);
35+
writer.Write((int)999);
36+
37+
var reader = writer.GetReader();
38+
Assert.Equal(42, reader.ReadByte());
39+
Assert.Equal(999, reader.ReadInt32());
40+
}
41+
42+
[Fact]
43+
public void Clear_PreservesBufferAndDoesNotThrowNRE()
44+
{
45+
// This is the core scenario from issue #951: after Clear(), writing
46+
// large amounts of data must not throw NullReferenceException.
47+
var writer = new SegmentedMemoryStreamWriter(1024, SerializationSettings.Default);
48+
49+
for (int i = 0; i < 100; i++)
50+
{
51+
writer.Write((byte)i);
52+
}
53+
54+
writer.Clear();
55+
Assert.Equal(0, writer.Length);
56+
57+
// Write enough to span multiple segments (segment size is 65_536).
58+
for (int i = 0; i < 200_000; i++)
59+
{
60+
writer.Write((byte)(i & 0xFF));
61+
}
62+
63+
Assert.Equal(200_000, writer.Length);
64+
65+
var reader = writer.GetReader();
66+
for (int i = 0; i < 200_000; i++)
67+
{
68+
Assert.Equal((byte)(i & 0xFF), reader.ReadByte());
69+
}
70+
}
71+
72+
[Fact]
73+
public void Clear_ResetsGetLabel()
74+
{
75+
var writer = new SegmentedMemoryStreamWriter(64, SerializationSettings.Default);
76+
77+
writer.Write((byte)1);
78+
writer.Write((int)42);
79+
Assert.Equal((StreamLabel)5, writer.GetLabel());
80+
81+
writer.Clear();
82+
Assert.Equal((StreamLabel)0, writer.GetLabel());
83+
}
84+
85+
[Fact]
86+
public void Clear_MultipleCycles()
87+
{
88+
var writer = new SegmentedMemoryStreamWriter(64, SerializationSettings.Default);
89+
90+
for (int round = 0; round < 5; round++)
91+
{
92+
writer.Clear();
93+
Assert.Equal(0, writer.Length);
94+
95+
writer.Write((byte)round);
96+
writer.Write((int)(round * 100));
97+
Assert.Equal(5, writer.Length);
98+
99+
var reader = writer.GetReader();
100+
Assert.Equal((byte)round, reader.ReadByte());
101+
Assert.Equal(round * 100, reader.ReadInt32());
102+
}
103+
}
104+
}
105+
}

src/FastSerialization/SegmentedMemoryStreamWriter.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,14 @@ public SegmentedMemoryStreamWriter(long initialSize, SerializationSettings setti
3232
}
3333

3434
public virtual long Length { get { return bytes.Count; } }
35-
public virtual void Clear() { bytes = new SegmentedList<byte>(131_072); }
35+
/// <summary>
36+
/// Resets the stream to empty, preserving the pre-allocated buffer.
37+
/// We set Count to 0 rather than creating a new SegmentedList so that the existing
38+
/// segment allocations are reused. This avoids discarding potentially large pre-allocations
39+
/// (up to ~1GB for 128M-node graphs) and prevents NullReferenceException in SegmentedList.Add
40+
/// that can occur when re-growing from zero under extreme memory pressure.
41+
/// </summary>
42+
public virtual void Clear() { bytes.Count = 0; }
3643

3744
public void Write(byte value)
3845
{

0 commit comments

Comments
 (0)