This is an automated email from the ASF dual-hosted git repository.
CurtHagenlocher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-dotnet.git
The following commit(s) were added to refs/heads/main by this push:
new db7f1e4 fix: Saturate NativeBuffer growth instead of overflowing past
half of int.MaxValue (#419)
db7f1e4 is described below
commit db7f1e4118cbc29f871b82bbb3e6c2aa642cb50f
Author: Curt Hagenlocher <[email protected]>
AuthorDate: Sat Aug 22 08:13:12 2026 -0700
fix: Saturate NativeBuffer growth instead of overflowing past half of
int.MaxValue (#419)
## What's Changed
`Grow` doubled the current length in a `checked` context without
saturating:
```csharp
int newCount = Math.Max(newElementCount, checked(Length * 2));
```
So once a buffer passed half of the addressable maximum, its **next**
grow threw `OverflowException`
however small the requested increase, and even though the requested size
still fit. For a
`NativeBuffer<byte, …>` that is a hard ceiling near 1 GiB, with no way
for a caller to work around it:
asking for a smaller increment does not help, because the overflow is in
the doubling rather than in
the request.
Growth now saturates at the largest addressable element count, so it
stays amortised right up to the
ceiling. A request that genuinely cannot be addressed still fails at the
byte-size calculation, as it
did before — behaviour is unchanged for anything that could not have
worked.
This is what the `TODO` those lines carried proposed:
> There might be a size that's big enough to work for this case but not
too big to overflow. We could
> use that instead of blindly doubling.
### On testing it
Reaching the boundary through `Grow` means allocating more than a
gigabyte, which does not belong in
a unit test. The count arithmetic is extracted to `ComputeGrowCount` so
the boundary can be tested
directly and exhaustively, including the per-element-size ceiling — the
limit is a byte count, so a
wider element type saturates at proportionally fewer elements.
Verified the new tests fail against the previous arithmetic before
fixing it: with `checked(Length *
2)` restored, `ComputeGrowCountSaturatesInsteadOfOverflowing` and
`ComputeGrowCountSaturatesPerElementSize` both fail; the rest pass
either way.
`Apache.Arrow.Tests` is green on net8.0: 1870 passed, 28 skipped (the
Python interop cases).
### Scope
This does not change the 2 GiB ceiling on `ArrowBuffer` itself
(`ReadOnlyMemory<byte>`, `int Length`)
— it only stops buffers failing at half of it.
Closes #418.
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
src/Apache.Arrow/Memory/NativeBuffer.cs | 24 ++++++++---
test/Apache.Arrow.Tests/NativeBufferTests.cs | 63 ++++++++++++++++++++++++++++
2 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/src/Apache.Arrow/Memory/NativeBuffer.cs
b/src/Apache.Arrow/Memory/NativeBuffer.cs
index f5b8f61..b042999 100644
--- a/src/Apache.Arrow/Memory/NativeBuffer.cs
+++ b/src/Apache.Arrow/Memory/NativeBuffer.cs
@@ -15,6 +15,7 @@
using System;
using System.Buffers;
+using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Threading;
@@ -90,15 +91,11 @@ namespace Apache.Arrow.Memory
if (newElementCount <= Length)
return;
- // Exponential growth (2x) to amortise repeated grows
- // TODO: There might be a size that's big enough to work for this
case but not too big to overflow.
- // We could use that instead of blindly doubling.
- int newCount = Math.Max(newElementCount, checked(Length * 2));
int elementSize = Unsafe.SizeOf<TItem>();
+ int newCount = ComputeGrowCount(Length, newElementCount,
elementSize);
int needed = checked(newCount * elementSize);
- var owner = _owner ?? throw new
ObjectDisposedException(nameof(NativeBuffer<TItem, TTracker>));
- owner.Reallocate(needed);
+ _owner.Reallocate(needed);
if (zeroFill)
{
@@ -109,6 +106,21 @@ namespace Apache.Arrow.Memory
Length = newCount;
}
+ /// <summary>
+ /// The element count to grow to: double the current length to
amortise repeated grows, but never
+ /// past the largest buffer that can be addressed, and never below
what the caller asked for.
+ /// </summary>
+ internal static int ComputeGrowCount(int length, int newElementCount,
int elementSize)
+ {
+ // Always Unsafe.SizeOf<TItem>() for an unmanaged TItem, so never
below one; the parameter
+ // exists so the boundary can be tested without allocating a
buffer of that size.
+ Debug.Assert(elementSize > 0);
+
+ int maxCount = int.MaxValue / elementSize;
+ long doubled = (long)length * 2;
+ return (int)Math.Max(newElementCount, Math.Min(doubled, maxCount));
+ }
+
public void Dispose()
{
IDisposable disposable = _owner;
diff --git a/test/Apache.Arrow.Tests/NativeBufferTests.cs
b/test/Apache.Arrow.Tests/NativeBufferTests.cs
index 84d050c..87ea440 100644
--- a/test/Apache.Arrow.Tests/NativeBufferTests.cs
+++ b/test/Apache.Arrow.Tests/NativeBufferTests.cs
@@ -84,6 +84,69 @@ namespace Apache.Arrow.Tests
Assert.Equal(42, buf.Span[0]);
}
+ // Growth doubles to stay amortised, but must saturate rather than
overflow. Doubling used to be
+ // unconditional and checked, so a buffer past half the maximum threw
OverflowException on its
+ // next grow however little was asked for — a byte buffer could not
grow beyond about 1 GiB.
+ //
+ // The arithmetic is tested directly: reproducing it through Grow
would mean allocating more than
+ // a gigabyte, which is not something to put in a unit test.
+ [Theory]
+ // length, requested, elementSize, expected
+ [InlineData(0, 1, 1, 1)] // nothing to double yet
+ [InlineData(3, 10, 4, 10)] // request exceeds the
doubling
+ [InlineData(8, 10, 4, 16)] // doubling exceeds the
request
+ [InlineData(5, 5, 4, 10)] // equal: doubling still
wins
+ public void ComputeGrowCountDoublesWhileItFits(
+ int length, int requested, int elementSize, int expected)
+ {
+ Assert.Equal(
+ expected,
+ NativeBuffer<byte,
NoOpAllocationTracker>.ComputeGrowCount(length, requested, elementSize));
+ }
+
+ [Fact]
+ public void ComputeGrowCountSaturatesInsteadOfOverflowing()
+ {
+ // Past half the maximum, doubling would overflow. The result
saturates at the largest
+ // addressable count and still covers the request.
+ const int elementSize = 1;
+ int overHalf = (int.MaxValue / 2) + 1000;
+
+ int grown = NativeBuffer<byte,
NoOpAllocationTracker>.ComputeGrowCount(
+ overHalf, overHalf + 1, elementSize);
+
+ Assert.Equal(int.MaxValue, grown);
+ Assert.True(grown >= overHalf + 1);
+ }
+
+ [Fact]
+ public void ComputeGrowCountSaturatesPerElementSize()
+ {
+ // The ceiling is a byte count, so a wider element saturates at
proportionally fewer of them.
+ const int elementSize = 8;
+ int maxCount = int.MaxValue / elementSize;
+ int overHalf = (maxCount / 2) + 1000;
+
+ int grown = NativeBuffer<long,
NoOpAllocationTracker>.ComputeGrowCount(
+ overHalf, overHalf + 1, elementSize);
+
+ Assert.Equal(maxCount, grown);
+ Assert.True((long)grown * elementSize <= int.MaxValue);
+ }
+
+ [Fact]
+ public void ComputeGrowCountNeverReturnsLessThanRequested()
+ {
+ // A request larger than the ceiling is not silently truncated;
Grow still refuses it when it
+ // works out the byte size.
+ const int elementSize = 8;
+ int beyond = (int.MaxValue / elementSize) + 1;
+
+ Assert.Equal(
+ beyond,
+ NativeBuffer<long, NoOpAllocationTracker>.ComputeGrowCount(0,
beyond, elementSize));
+ }
+
[Fact]
public void BuildTransfersOwnershipToArrowBuffer()
{
