This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-go.git
The following commit(s) were added to refs/heads/main by this push:
new 3886c0a0 fix(arrow/array): `Resize(0)` on non-empty builder causes
memory leak (#995)
3886c0a0 is described below
commit 3886c0a0162d17b9852b4bb8dc1872b4a0ee21c5
Author: Lucas Valente <[email protected]>
AuthorDate: Fri Jul 24 18:11:11 2026 +0200
fix(arrow/array): `Resize(0)` on non-empty builder causes memory leak (#995)
### Rationale for this change
I found a memory leak while working on the [JSON nullable parity
PR](https://github.com/apache/arrow-go/pull/833). It's triggered
whenever a non-empty builder gets resized to zero.
This memory leak can be achieved with public APIs only (`Append` and
`Resize`), meaning it's an actual bug in the public API and not a misuse
of internal APIs where I didn't check for an invariant.
In the specific case of the JSON PR, whenever we found an error in the
first row that would call `Resize(-1)`, which in turn calls `Resize(0)`
on all the previous fields, and the bug gets triggered.
You can checkout to each commit to see the test failing then passing
after the fix.
### What changes are included in this PR?
- A new unit test to repro the memory leak.
- The fix: always respect `minBuilderCapacity` when calling `Resize()`
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
---
arrow/array/builder.go | 5 +-
arrow/array/numericbuilder.gen_test.go | 183 ++++++++++++++++++++++++++++
arrow/array/numericbuilder.gen_test.go.tmpl | 17 +++
3 files changed, 203 insertions(+), 2 deletions(-)
diff --git a/arrow/array/builder.go b/arrow/array/builder.go
index ed4f08e7..517feec5 100644
--- a/arrow/array/builder.go
+++ b/arrow/array/builder.go
@@ -164,10 +164,11 @@ func (b *builder) resize(newBits int, init func(int)) {
return
}
- newBytesN := bitutil.CeilByte(newBits) / 8
+ allocBits := max(newBits, minBuilderCapacity)
+ newBytesN := bitutil.CeilByte(allocBits) / 8
oldBytesN := b.nullBitmap.Len()
b.nullBitmap.Resize(newBytesN)
- b.capacity = newBits
+ b.capacity = allocBits
if oldBytesN < newBytesN {
// TODO(sgc): necessary?
memory.Set(b.nullBitmap.Buf()[oldBytesN:], 0)
diff --git a/arrow/array/numericbuilder.gen_test.go
b/arrow/array/numericbuilder.gen_test.go
index 2ca56329..c54d8cf2 100644
--- a/arrow/array/numericbuilder.gen_test.go
+++ b/arrow/array/numericbuilder.gen_test.go
@@ -230,6 +230,18 @@ func TestInt64Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestInt64Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewInt64Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestInt64BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -457,6 +469,18 @@ func TestUint64Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestUint64Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewUint64Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestUint64BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -684,6 +708,18 @@ func TestFloat64Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestFloat64Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewFloat64Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestFloat64BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -909,6 +945,18 @@ func TestInt32Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestInt32Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewInt32Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestInt32BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -1136,6 +1184,18 @@ func TestUint32Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestUint32Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewUint32Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestUint32BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -1363,6 +1423,18 @@ func TestFloat32Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestFloat32Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewFloat32Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestFloat32BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -1588,6 +1660,18 @@ func TestInt16Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestInt16Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewInt16Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestInt16BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -1815,6 +1899,18 @@ func TestUint16Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestUint16Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewUint16Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestUint16BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -2042,6 +2138,18 @@ func TestInt8Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestInt8Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewInt8Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestInt8BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -2269,6 +2377,18 @@ func TestUint8Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestUint8Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewUint8Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestUint8BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -2501,6 +2621,19 @@ func TestTime32Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestTime32Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ dtype := &arrow.Time32Type{Unit: arrow.Second}
+ ab := array.NewTime32Builder(mem, dtype)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestTime32BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -2734,6 +2867,19 @@ func TestTime64Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestTime64Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ dtype := &arrow.Time64Type{Unit: arrow.Second}
+ ab := array.NewTime64Builder(mem, dtype)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestTime64BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -2962,6 +3108,18 @@ func TestDate32Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestDate32Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewDate32Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestDate32BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -3196,6 +3354,18 @@ func TestDate64Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestDate64Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ab := array.NewDate64Builder(mem)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestDate64BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
@@ -3428,6 +3598,19 @@ func TestDurationBuilder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func TestDurationBuilder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ dtype := &arrow.DurationType{Unit: arrow.Second}
+ ab := array.NewDurationBuilder(mem, dtype)
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func TestDurationBuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
diff --git a/arrow/array/numericbuilder.gen_test.go.tmpl
b/arrow/array/numericbuilder.gen_test.go.tmpl
index 07d5fc8a..e3422496 100644
--- a/arrow/array/numericbuilder.gen_test.go.tmpl
+++ b/arrow/array/numericbuilder.gen_test.go.tmpl
@@ -272,6 +272,23 @@ func Test{{.Name}}Builder_Resize(t *testing.T) {
assert.Equal(t, 5, ab.Len())
}
+func Test{{.Name}}Builder_ResizeToZeroThenAppend(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+{{if .Opt.Parametric -}}
+ dtype := &arrow.{{.Name}}Type{Unit: arrow.Second}
+ ab := array.New{{.Name}}Builder(mem, dtype)
+{{else}}
+ ab := array.New{{.Name}}Builder(mem)
+{{end -}}
+ defer ab.Release()
+
+ ab.Append(0)
+ ab.Resize(0)
+ ab.Append(0)
+}
+
func Test{{.Name}}BuilderUnmarshalJSON(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
defer mem.AssertSize(t, 0)
