[Rust][FFI] Add Arrow C Data ingestion - zero copy - #673
Conversation
21b9465 to
46aa83c
Compare
| #[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); | ||
| } | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
What changes are proposed in this pull request?
How is this tested?
arrow_c_data_ffi_tests added