This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new c4f286c4f fix(rust/ffi): fix release_ffi_error to properly null values
(#4181)
c4f286c4f is described below
commit c4f286c4f0cedbb9426d14bf7b55be51a0fbc52e
Author: Matt Topol <[email protected]>
AuthorDate: Sat Apr 4 04:10:48 2026 -0400
fix(rust/ffi): fix release_ffi_error to properly null values (#4181)
Fixes #4178
---
rust/ffi/src/types.rs | 71 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 68 insertions(+), 3 deletions(-)
diff --git a/rust/ffi/src/types.rs b/rust/ffi/src/types.rs
index 716114d2d..7e0e6a2e8 100644
--- a/rust/ffi/src/types.rs
+++ b/rust/ffi/src/types.rs
@@ -560,9 +560,12 @@ unsafe extern "C" fn release_ffi_error(error: *mut
FFI_AdbcError) {
match error.as_mut() {
None => (),
Some(error) => {
- // SAFETY: `error.message` was necessarily obtained with
`CString::into_raw`.
- // Additionally, C should not modify the string's length.
- drop(CString::from_raw(error.message));
+ if !error.message.is_null() {
+ // SAFETY: `error.message` was necessarily obtained with
`CString::into_raw`.
+ // Additionally, C should not modify the string's length.
+ drop(CString::from_raw(error.message));
+ error.message = null_mut();
+ }
if !error.private_data.is_null() {
// SAFETY: `error.private_data` was necessarily obtained with
`Box::into_raw`.
@@ -570,7 +573,9 @@ unsafe extern "C" fn release_ffi_error(error: *mut
FFI_AdbcError) {
// Finally, C should call the release function only once.
let private_data = Box::from_raw(error.private_data as *mut
ErrorPrivateData);
drop(private_data);
+ error.private_data = null_mut();
}
+ error.release = None;
}
}
}
@@ -626,4 +631,64 @@ mod tests {
let partitions_actual: Partitions = partitions_ffi.into();
assert_eq!(partitions_expected, partitions_actual);
}
+
+ #[test]
+ fn test_release_ffi_error_nulls_fields() {
+ // Use details to ensure private_data is non-null before release.
+ let error = Error {
+ message: "test error".into(),
+ status: Status::Unknown,
+ vendor_code: 0,
+ sqlstate: [0; 5],
+ details: Some(vec![("key".to_string(), b"value".to_vec())]),
+ };
+ let mut ffi_error: FFI_AdbcError = error.try_into().unwrap();
+ assert!(!ffi_error.message.is_null());
+ assert!(!ffi_error.private_data.is_null());
+
+ unsafe { release_ffi_error(&mut ffi_error) };
+
+ assert!(ffi_error.message.is_null());
+ assert!(ffi_error.private_data.is_null());
+ assert!(ffi_error.release.is_none());
+ // Drop here is a no-op because release is None.
+ }
+
+ #[test]
+ fn test_release_ffi_error_idempotent() {
+ // Calling release twice must not double-free the message pointer.
+ let error = Error {
+ message: "test error".into(),
+ status: Status::Unknown,
+ vendor_code: 0,
+ sqlstate: [0; 5],
+ details: None,
+ };
+ let mut ffi_error: FFI_AdbcError = error.try_into().unwrap();
+
+ unsafe {
+ release_ffi_error(&mut ffi_error);
+ release_ffi_error(&mut ffi_error);
+ }
+ // Drop here is a no-op because release is None.
+ }
+
+ #[test]
+ fn test_release_ffi_error_null_message() {
+ // A null message must not cause UB or a panic during release.
+ let mut ffi_error = FFI_AdbcError {
+ message: null_mut(),
+ vendor_code: 0,
+ sqlstate: [0; 5],
+ release: Some(release_ffi_error),
+ private_data: null_mut(),
+ private_driver: null(),
+ };
+
+ unsafe { release_ffi_error(&mut ffi_error) };
+
+ assert!(ffi_error.message.is_null());
+ assert!(ffi_error.release.is_none());
+ // Drop here is a no-op because release is None.
+ }
}
