Skip to content

Commit ab3d9ae

Browse files
Change exception for overflow in ArrayBufferWriter (#32587)
* Fix #609 - Part 2 We throw a OutOfMemoryException instead of "Arithmetic operation resulted in an overflow" * Test attributes. We don't need to limit to 64bit process, since the memory is never allocated. * Remove platform restriction * Explicitly throw the OutOfMemoryException instead of rely on Array.Resize * Move ThrowOutOfMemoryException method * Missing message, BufferMaximumSizeExceeded * Code Coverage * Resource file * Missing Resx update * Restrict test to 64bits environment * Simple test so that we have some test coverage in inner loop Since we are trying to avoid the OOM, don't run the GetMemory_ExceedMaximumBufferSize on Linux * Test comment * Move OutOfMemory test to byte specific test
1 parent 25fdaa8 commit ab3d9ae

File tree

6 files changed

+54
-6
lines changed

6 files changed

+54
-6
lines changed

src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,24 @@ private void CheckAndResizeBuffer(int sizeHint)
167167

168168
if (sizeHint > FreeCapacity)
169169
{
170-
int growBy = Math.Max(sizeHint, _buffer.Length);
170+
int currentLength = _buffer.Length;
171+
int growBy = Math.Max(sizeHint, currentLength);
171172

172-
if (_buffer.Length == 0)
173+
if (currentLength == 0)
173174
{
174175
growBy = Math.Max(growBy, DefaultInitialBufferSize);
175176
}
176177

177-
int newSize = checked(_buffer.Length + growBy);
178+
int newSize = currentLength + growBy;
179+
180+
if ((uint)newSize > int.MaxValue)
181+
{
182+
newSize = currentLength + sizeHint;
183+
if ((uint)newSize > int.MaxValue)
184+
{
185+
ThrowOutOfMemoryException((uint)newSize);
186+
}
187+
}
178188

179189
Array.Resize(ref _buffer, newSize);
180190
}
@@ -186,5 +196,10 @@ private static void ThrowInvalidOperationException_AdvancedTooFar(int capacity)
186196
{
187197
throw new InvalidOperationException(SR.Format(SR.BufferWriterAdvancedTooFar, capacity));
188198
}
199+
200+
private static void ThrowOutOfMemoryException(uint capacity)
201+
{
202+
throw new OutOfMemoryException(SR.Format(SR.BufferMaximumSizeExceeded, capacity));
203+
}
189204
}
190205
}

src/libraries/System.Memory/src/Resources/Strings.resx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
<root>
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<root>
23
<!--
34
Microsoft ResX Schema
45
@@ -146,4 +147,7 @@
146147
<data name="BufferWriterAdvancedTooFar" xml:space="preserve">
147148
<value>Cannot advance past the end of the buffer, which has a size of {0}.</value>
148149
</data>
149-
</root>
150+
<data name="BufferMaximumSizeExceeded" xml:space="preserve">
151+
<value>Cannot allocate a buffer of size {0}.</value>
152+
</data>
153+
</root>

src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.Byte.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,5 +89,21 @@ public async Task WriteAndCopyToStreamAsync()
8989
Assert.Equal(outputMemory.Length, streamOutput.Length);
9090
Assert.True(outputMemory.Span.SequenceEqual(streamOutput));
9191
}
92+
93+
// NOTE: GetMemory_ExceedMaximumBufferSize test is constrained to run on Windows and MacOSX because it causes
94+
// problems on Linux due to the way deferred memory allocation works. On Linux, the allocation can
95+
// succeed even if there is not enough memory but then the test may get killed by the OOM killer at the
96+
// time the memory is accessed which triggers the full memory allocation.
97+
[PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)]
98+
[ConditionalFact(nameof(IsX64))]
99+
[OuterLoop]
100+
public void GetMemory_ExceedMaximumBufferSize()
101+
{
102+
var output = new ArrayBufferWriter<byte>(int.MaxValue / 2 + 1);
103+
output.Advance(int.MaxValue / 2 + 1);
104+
Memory<byte> memory = output.GetMemory(1); // Validate we can't double the buffer size, but can grow by sizeHint
105+
Assert.Equal(1, memory.Length);
106+
Assert.Throws<OutOfMemoryException>(() => output.GetMemory(int.MaxValue));
107+
}
92108
}
93109
}

src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,13 @@ public void GetMemory_DefaultCtor(int sizeHint)
205205
Assert.Equal(sizeHint <= 256 ? 256 : sizeHint, memory.Length);
206206
}
207207

208+
[Fact]
209+
public void GetMemory_ExceedMaximumBufferSize_WithSmallStartingSize()
210+
{
211+
var output = new ArrayBufferWriter<T>(256);
212+
Assert.Throws<OutOfMemoryException>(() => output.GetMemory(int.MaxValue));
213+
}
214+
208215
[Fact]
209216
public void GetMemory_InitSizeCtor()
210217
{

src/libraries/System.Text.Json/src/Resources/Strings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,4 +503,7 @@
503503
<data name="ExtensionDataCannotBindToCtorParam" xml:space="preserve">
504504
<value>The extension data property '{0}' on type '{1}' cannot bind with a parameter in constructor '{2}'.</value>
505505
</data>
506+
<data name="BufferMaximumSizeExceeded" xml:space="preserve">
507+
<value>Cannot allocate a buffer of size {0}.</value>
508+
</data>
506509
</root>

src/libraries/System.Text.Json/tests/Resources/Strings.resx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20278,4 +20278,7 @@ tiline\"another\" String\\"],"str":"\"\""}</value>
2027820278
<data name="LoremIpsum40WordsBase64" xml:space="preserve">
2027920279
<value>TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQsIGNvbnNlY3RldHVyIGFkaXBpc2NpbmcgZWxpdC4gQ3VyYWJpdHVyIHZpdmVycmEgbmVxdWUgYXQgZXJhdCBsYW9yZWV0LCBhdCBzYWdpdHRpcyBhcmN1IGVmZmljaXR1ci4gVXQgcHVsdmluYXIgZXJvcyBuZWMgb2RpbyBjdXJzdXMgZWxlaWZlbmQuIE1hZWNlbmFzIHZpdmVycmEgZWxlbWVudHVtIHBvcnR0aXRvci4gTnVsbGFtIG1pIHZlbGl0LCBtYWxlc3VhZGEgY29tbW9kbyB0cmlzdGlxdWUgZXUsIG1vbGxpcyBhIHZlbGl0LiBNb3JiaS4=</value>
2028020280
</data>
20281-
</root>
20281+
<data name="BufferMaximumSizeExceeded" xml:space="preserve">
20282+
<value>Cannot allocate a buffer of size {0}.</value>
20283+
</data>
20284+
</root>

0 commit comments

Comments
 (0)