fix(python/adbc_driver_manager): don't leak array streams #2922
Conversation
The driver manager was leaking array streams in the case that you executed a query, *did not fetch the result*, then did another operation. In this case, because the stream was never imported, it would silently leak. Avoid this by explicitly/directly releasing any extant array stream before another driver operation.
| """ | ||
| if self.stream.release != NULL: | ||
| self.stream.release(&self.stream) | ||
| self.stream.release = NULL |
There was a problem hiding this comment.
is there a magic python magic func we can utilize here? like __del__?
There was a problem hiding this comment.
It's already implemented on the Cursor which will implicitly free everything else now. We could perhaps add it here too?
There was a problem hiding this comment.
I also added the methods to ArrowArray/ArrowSchema even though they're not used
zeroshade
left a comment
zeroshade
left a comment
There was a problem hiding this comment.
Looks good to me, just one question beyond the CI failing
| if self._results is not None: | ||
| self._results.close() | ||
| self._results = None | ||
| self._clear() |
There was a problem hiding this comment.
Should _clear just imply close and closed=true?
There was a problem hiding this comment.
Er, no, because this object can be reused and _clear is used below, so the only thing it should do is free any allocated array stream
|
@zeroshade the CI failing is some bug with arrow-go where Flight SQL is returning an EOF from some internal reader as an actual error, which I've never managed to track down |
|
Ah wait I do need to update the PYI it appears |
2 participants
The driver manager was leaking array streams in the case that you executed a query, did not fetch the result, then did another operation. In this case, because the stream was never imported, it would silently leak. Avoid this by explicitly/directly releasing any extant array stream before another driver operation.