Skip to content

Commit b2b3ff5

Browse files
committed
fix memory double free
Signed-off-by: Cai Zhang <[email protected]>
1 parent 613e760 commit b2b3ff5

File tree

2 files changed

+9
-31
lines changed

2 files changed

+9
-31
lines changed

arrow/cdata/cdata.go

Lines changed: 9 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -408,11 +408,6 @@ func (imp *cimporter) doImportChildren() error {
408408
for i, c := range children {
409409
imp.children[i].dt = st.Field(i).Type
410410
if err := imp.children[i].importChild(imp, c); err != nil {
411-
for j := 0; j < i; j++ {
412-
if imp.children[j].data != nil {
413-
imp.children[j].data.Release()
414-
}
415-
}
416411
return err
417412
}
418413
}
@@ -436,11 +431,6 @@ func (imp *cimporter) doImportChildren() error {
436431
for i, c := range children {
437432
imp.children[i].dt = dt.Fields()[i].Type
438433
if err := imp.children[i].importChild(imp, c); err != nil {
439-
for j := 0; j < i; j++ {
440-
if imp.children[j].data != nil {
441-
imp.children[j].data.Release()
442-
}
443-
}
444434
return err
445435
}
446436
}
@@ -449,11 +439,6 @@ func (imp *cimporter) doImportChildren() error {
449439
for i, c := range children {
450440
imp.children[i].dt = dt.Fields()[i].Type
451441
if err := imp.children[i].importChild(imp, c); err != nil {
452-
for j := 0; j < i; j++ {
453-
if imp.children[j].data != nil {
454-
imp.children[j].data.Release()
455-
}
456-
}
457442
return err
458443
}
459444
}
@@ -484,8 +469,10 @@ func (imp *cimporter) doImportArr(src *CArrowArray) error {
484469
// memory that we have to track the lifetime of.
485470
defer func() {
486471
if imp.alloc.bufCount.Load() == 0 {
487-
C.ArrowArrayRelease(imp.arr)
488-
C.free(unsafe.Pointer(imp.arr))
472+
if C.ArrowArrayIsReleased(imp.arr) == 0 {
473+
C.ArrowArrayRelease(imp.arr)
474+
C.free(unsafe.Pointer(imp.arr))
475+
}
489476
}
490477
}()
491478

@@ -495,20 +482,13 @@ func (imp *cimporter) doImportArr(src *CArrowArray) error {
495482
// import is called recursively as needed for importing an array and its children
496483
// in order to generate array.Data objects
497484
func (imp *cimporter) doImport() error {
498-
// move the array from the src object passed in to the one referenced by
499-
// this importer. That way we can set up a finalizer on the created
500-
// arrow.ArrayData object so we clean up our Array's memory when garbage collected.
501-
defer func(arr *CArrowArray) {
502-
// this should only occur in the case of an error happening
503-
// during import, at which point we need to clean up the
504-
// ArrowArray struct we allocated.
505-
if imp.data == nil {
506-
C.free(unsafe.Pointer(arr))
507-
}
508-
}(imp.arr)
509-
510485
// import any children
511486
if err := imp.doImportChildren(); err != nil {
487+
for _, c := range imp.children {
488+
if c.data != nil {
489+
c.data.Release()
490+
}
491+
}
512492
return err
513493
}
514494

arrow/cdata/cdata_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -834,7 +834,6 @@ func TestImportDenseUnionWithInvalidSchema(t *testing.T) {
834834
defer freeTestMallocatorArr(carr, mem)
835835

836836
unionSc := testUnion([]string{"+ud:5,10", "i", "u"}, []string{"", "u0", "u1"}, []int64{0, flagIsNullable, flagIsNullable})
837-
defer freeMallocedSchemas(unionSc)
838837

839838
structSc := testStruct([]string{"+s", "+ud:5,10"}, []string{"", "union_field"}, []int64{0, 0})
840839
defer freeMallocedSchemas(structSc)
@@ -886,7 +885,6 @@ func TestImportSPARSEUnionWithInvalidSchema(t *testing.T) {
886885
defer freeTestMallocatorArr(carr, mem)
887886

888887
unionSc := testUnion([]string{"+us:5,10", "i", "u"}, []string{"", "u0", "u1"}, []int64{0, flagIsNullable, flagIsNullable})
889-
defer freeMallocedSchemas(unionSc)
890888

891889
structSc := testStruct([]string{"+s", "+us:5,10"}, []string{"", "union_field"}, []int64{0, 0})
892890
defer freeMallocedSchemas(structSc)

0 commit comments

Comments
 (0)