fix(arrow): retain pools for stream buffers - #182
Merged
SteNicholas merged 1 commit intoAug 6, 2026
Conversation
mrdrivingduck
marked this pull request as ready for review
August 5, 2026 01:56
mrdrivingduck
force-pushed
the
codex/fix-arrow-buffer-pool-lifetime
branch
3 times, most recently
from
August 6, 2026 03:00
c430694 to
b35d136
Compare
zjw1111
reviewed
Aug 6, 2026
| return arrow::Status::OK(); | ||
| } | ||
|
|
||
| struct BufferWithMemoryPool { |
Collaborator
There was a problem hiding this comment.
could you please modify the PR title to conventional commits style, and add a TODO assigned to xinyu, noting that performance will be optimized later.
Contributor
Author
There was a problem hiding this comment.
Done with PR title. TODO means an issue or simply cc from current PR?
Collaborator
There was a problem hiding this comment.
add a line in code:
// TODO(lxy): xxxxxx
Contributor
Author
There was a problem hiding this comment.
Done in KeepMemoryPoolAlive.
mrdrivingduck
force-pushed
the
codex/fix-arrow-buffer-pool-lifetime
branch
from
August 6, 2026 07:35
b35d136 to
4ea9719
Compare
Keep Arrow memory pools alive while returned stream buffers remain referenced and note the allocation overhead for a future performance improvement. Add deterministic coverage for synchronous and asynchronous reads after adapter teardown. Co-authored-by: GPT-5.6 Terra <codex@users.noreply.github.com>
mrdrivingduck
force-pushed
the
codex/fix-arrow-buffer-pool-lifetime
branch
from
August 6, 2026 08:22
4ea9719 to
3f0d672
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I found this while debugging an S3 crash. I initially suspected the S3 filesystem, but the issue was in the Arrow input-stream adapter: a returned buffer can outlive its memory pool when a reader is closed while a caller still holds a buffer. This is more likely with network filesystems, where asynchronous reads may finish after the reader is closed.
This patch uses
shared_ptr's aliasing constructor to keep the pool alive with the returned buffer. It also adds deterministic coverage forRead,ReadAt, andReadAsyncusing a smallInputStreammock. Without the change, the tests crash; with it, they pass.