zeroshade commented on issue #371:
URL: https://github.com/apache/arrow-go/issues/371#issuecomment-2863580764

   Based on what you described and taking a look at 
https://github.com/marcboeker/go-duckdb/blob/main/arrow.go#L251, it looks like 
the problem is the release function that go-duckdb is providing:
   
   ```go
        release = func() {
                C.free(stream)
        }
   ```
   
   It needs to call the release callback on the exported ArrowArrayStream 
object, as you discovered when you said:
   
   > Therefore I went on to see what would happen if I added in to cdata a new 
ReleaseCArrowArrayStream() function only calling `C.ArrowArrayStreamRelease()` 
(similarly to the [ReleaseCArrowArray 
here](https://github.com/apache/arrow-go/blob/main/arrow/cdata/interface.go#L282))
 and then calling that in the `release()` function for `RegisterView()` in 
go-duckdb. This worked as I expected (no visible memory leak) ...
   
   One option would be to do precisely what you did (I didn't realize I hadn't 
made an equivalent release function for streams, that's a good point!). 
   
   Another option would be for `go-duckdb` to make the following change:
   
   ```go
         release = func() {
                   rdr, _ := 
cdata.ImportCRecordReader((*cdata.CArrowArrayStream)(stream), nil)
                   rdr.(array.RecordReader).Release()
                   C.free(stream)
         }
   ```
   
   Alternately, `go-duckdb` could add a C function to call the release callback 
and put a call to it inside of the release. It really just boils down to the 
fact that the release function being returned doesn't call the release callback 
of the `ArrowArrayStream` struct like it should.


-- 
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]

Reply via email to