Skip to content

[Rust][FFI] Add Arrow C Data ingestion - zero copy - #673

Merged
nikolaobradovic-db merged 2 commits into
mainfrom
nikola-obradovic_data/zerobus/zerocopy_arrow_ffi_base
Aug 5, 2026
Merged

[Rust][FFI] Add Arrow C Data ingestion - zero copy#673
nikolaobradovic-db merged 2 commits into
mainfrom
nikola-obradovic_data/zerobus/zerocopy_arrow_ffi_base

Conversation

@nikolaobradovic-db

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

  • Add zerobus_arrow_stream_ingest_c_data, an additive C API that transfers canonical ArrowArray/ArrowSchema ownership without an IPC encode/decode round trip while still be able to use existing Flight encoding (compression, chunking, dictionary hydration, ACK, recovery, and error behavior is preserved)
  • Add the disabled-by-default internal-arrow-c-data SDK feature so Zerobus native bindings can share one importer, Arrow type identity, validation, and ownership implementation.
  • Retain C-specific pointer handling, panic containment, runtime bridging, and ABI marker types in zerobus-ffi.
  • Add rlib output only for workspace integration tests that call the real C ABI; shipped FFI artifacts remain static/dynamic libraries.
  • Add dedicated C ABI lifetime coverage for release-after-ACK and terminal retention, plus wire-level compression, chunking, dictionary payload, and nested ownership tests.

How is this tested?

arrow_c_data_ffi_tests added

@nikolaobradovic-db
nikolaobradovic-db force-pushed the nikola-obradovic_data/zerobus/zerocopy_arrow_ffi_base branch from 21b9465 to 46aa83c Compare August 5, 2026 09:26
Comment thread rust/ffi/src/arrow.rs
Comment on lines 295 to 304
#[no_mangle]
pub extern "C" fn zerobus_arrow_stream_free(stream: *mut CArrowStream) {
ffi_guard(ptr::null_mut(), (), move || {
if !stream.is_null() {
unsafe {
let _ = Box::from_raw(stream as *mut ZerobusArrowStream);
}
}
})
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only relevant thing that I found:
zerobus_arrow_stream_free does not wait for the background task the way close() does
close() stops the supervisor and waits for it to finish. free() (via Drop) only asks it to stop, then returns. After ingest_c_data, the SDK may still hold the batch in that task, so the caller’s release callback can run later on a runtime thread — after free() has already returned.

If the caller destroys state the callback needs at free() time (allocator, wrapper context, etc.), the late callback can crash. I managed to reproduce this with a test and my code panicked.

This is unlikely after a normal flush → close → free, but realistic on failure/cleanup paths where the stream is freed while batches are still retained.
Also, this behavior existed before this PR but it matters more now because C Data retains caller-owned buffers and release callbacks, not just SDK-owned IPC copies.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I need @teodordelibasic-db to finish #677 to properly fix this use-after-free, and it can be done as a follow-up.

@nikolaobradovic-db
nikolaobradovic-db added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit 2cc8250 Aug 5, 2026
46 checks passed
@nikolaobradovic-db
nikolaobradovic-db deleted the nikola-obradovic_data/zerobus/zerocopy_arrow_ffi_base branch August 5, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants