zeroshade commented on code in PR #38314:
URL: https://github.com/apache/arrow/pull/38314#discussion_r1362671300
##########
go/arrow/cdata/cdata.go:
##########
@@ -418,26 +409,28 @@ func (imp *cimporter) doImportChildren() error {
func (imp *cimporter) initarr() {
imp.arr = C.get_arr()
+ if imp.alloc == nil {
+ imp.alloc = &importAllocator{arr: imp.arr}
+ }
+}
+
+func (imp *cimporter) doImportArr(src *CArrowArray) error {
+ imp.arr = C.get_arr()
+ C.ArrowArrayMove(src, imp.arr)
+ if imp.alloc == nil {
+ imp.alloc = &importAllocator{arr: imp.arr}
+ }
+ return imp.doImport()
}
// import is called recursively as needed for importing an array and its
children
// in order to generate array.Data objects
-func (imp *cimporter) doImport(src *CArrowArray) error {
- imp.initarr()
+func (imp *cimporter) doImport() error {
// move the array from the src object passed in to the one referenced by
// this importer. That way we can set up a finalizer on the created
// arrow.ArrayData object so we clean up our Array's memory when
garbage collected.
- C.ArrowArrayMove(src, imp.arr)
defer func(arr *CArrowArray) {
- if imp.data != nil {
- runtime.SetFinalizer(imp.data, func(arrow.ArrayData) {
- defer C.free(unsafe.Pointer(arr))
- C.ArrowArrayRelease(arr)
- if C.ArrowArrayIsReleased(arr) != 1 {
- panic("did not release C mem")
- }
- })
Review Comment:
We don't actually expose any API to externally check the current refcount on
the objects so there's not really a way for the finalizer to check that. That's
why I added the new tests to confirm that `Release` does actually free the
memory, and users can use the `CheckedAllocator` if they want to ensure that
things are being properly released
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
