Skip to content

Fix Arrow query parameter memory leak - #853

Draft
mlafeldt wants to merge 1 commit into
mainfrom
fix/arrow-param-memory-retention
Draft

Fix Arrow query parameter memory leak#853
mlafeldt wants to merge 1 commit into
mainfrom
fix/arrow-param-memory-retention

Conversation

@mlafeldt

@mlafeldt mlafeldt commented Sep 9, 2026

Copy link
Copy Markdown
Member

Since a141084, every RecordBatch passed to ArrowVTab is retained for the process lifetime so stored views stay safe to rebind. Any loop that builds parameters per batch leaks without bound.

Replace the three arrow_*_to_query_params helpers with ArrowBatchRegistration, which releases its batch on drop. Callers borrow the registration, making the lifetime change a compile-time break. A query that already bound the batch owns a cheap clone and may outlive the registration.

Following #852

Since a141084, every RecordBatch passed to ArrowVTab was retained for the
process lifetime so stored views stayed safe to rebind. Any loop that built
parameters per batch leaked without bound.

Replace the three arrow_*_to_query_params helpers with
ArrowBatchRegistration, which releases its batch on drop. Callers borrow the
registration, making the lifetime change a compile-time break. A query that
already bound the batch owns a cheap clone and may outlive the registration.

Batches now live in a keyed store rather than behind a raw address, so the
table function takes a single opaque UBIGINT ID and the layout marker that
guarded the pointer dereference is gone. Seed the ID counter from the
standard library RandomState: its random keys suit collision reduction with
a view persisted by an earlier process, without promising uniform output or
authorization. Reserved endpoints are clamped and exhaustion stays checked,
so an ID is never reused within a store.

Drop a removed batch after the store's mutex guard ends, so a foreign
release callback can register or release other batches without deadlocking.

DuckDB rejects mismatched argument counts and types before invoking the bind
callback, so only the reachable null and unknown-ID checks remain.
@mlafeldt
mlafeldt force-pushed the fix/arrow-param-memory-retention branch from 7d87842 to c092925 Compare September 9, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant