Skip to content

Conversation

@timsaucer
Copy link
Member

@timsaucer timsaucer commented Aug 14, 2025

Which issue does this PR close?

Rationale for this change

When using the foreign function interface, a Box<RecordBatchStreamPrivateData> is created but it is never properly freed. This leads to a growth in memory over time as these are used.

What changes are included in this PR?

Adds a release function to FFI_RecordBatchStream and implements Drop.

Are these changes tested?

Tested against datafusion-python where it is easy to track memory growth and to implement FFI based table providers.

Are there any user-facing changes?

None

@timsaucer timsaucer self-assigned this Aug 14, 2025
@timsaucer timsaucer added bug Something isn't working ffi Changes to the ffi crate labels Aug 14, 2025
timsaucer added a commit to rerun-io/rerun that referenced this pull request Aug 14, 2025
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @timsaucer -- this makes sense to me

}

unsafe extern "C" fn release_fn_wrapper(provider: &mut FFI_RecordBatchStream) {
let private_data =
Copy link
Contributor

Choose a reason for hiding this comment

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

I double checked that the private_data is actually a RecordBatchStreamPrivateData that is created with the constructor

pub fn new(stream: SendableRecordBatchStream, runtime: Option<Handle>) -> Self {
let private_data = Box::into_raw(Box::new(RecordBatchStreamPrivateData {
rbs: stream,
runtime,
})) as *mut c_void;

@alamb
Copy link
Contributor

alamb commented Aug 18, 2025

I also merged up from main

@alamb alamb merged commit f27b103 into apache:main Aug 19, 2025
27 checks passed
timsaucer added a commit to timsaucer/datafusion that referenced this pull request Aug 21, 2025
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@timsaucer timsaucer deleted the bugfix/drop-ffi-rbs-private-data branch August 21, 2025 11:14
alamb added a commit that referenced this pull request Aug 21, 2025
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ffi Changes to the ffi crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak in FFI_RecordBatchStream

2 participants