Rename and comment Deallocation variants

arrow/src/alloc/mod.rs

@@ -125,28 +125,30 @@ pub unsafe fn reallocate<T: NativeType>(
     })
 }
 
-/// The owner of an allocation, that is not natively allocated.
+/// The owner of an allocation.
 /// The trait implementation is responsible for dropping the allocations once no more references exist.
 pub trait Allocation: RefUnwindSafe {}
 
 impl<T: RefUnwindSafe> Allocation for T {}
 
 /// Mode of deallocating memory regions
-pub enum Deallocation {
-    /// Native deallocation, using Rust deallocator with Arrow-specific memory alignment
-    Native(usize),
-    /// Foreign interface, via a callback
-    Foreign(Arc<dyn Allocation>),
+pub(crate) enum Deallocation {
+    /// An allocation of the given capacity that needs to be deallocated using arrows's cache aligned allocator.
+    /// See [allocate_aligned] and [free_aligned].
+    Arrow(usize),
+    /// An allocation from an external source like the FFI interface or a Rust Vec.
+    /// Deallocation will happen
+    Custom(Arc<dyn Allocation>),
 }
 
 impl Debug for Deallocation {
     fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
         match self {
-            Deallocation::Native(capacity) => {
-                write!(f, "Deallocation::Native {{ capacity: {} }}", capacity)
+            Deallocation::Arrow(capacity) => {
+                write!(f, "Deallocation::Arrow {{ capacity: {} }}", capacity)
             }
-            Deallocation::Foreign(_) => {
-                write!(f, "Deallocation::Foreign {{ capacity: unknown }}")
+            Deallocation::Custom(_) => {
+                write!(f, "Deallocation::Custom {{ capacity: unknown }}")
             }
         }
     }

arrow/src/array/data.rs

@@ -2603,21 +2603,21 @@ mod tests {
         let mut bitmap = vec![0b1110_u8];
 
         let strings_buffer = unsafe {
-            Buffer::from_foreign(
+            Buffer::from_custom_allocation(
                 NonNull::new_unchecked(strings.as_mut_ptr()),
                 strings.len(),
                 Arc::new(strings),
             )
         };
         let offsets_buffer = unsafe {
-            Buffer::from_foreign(
+            Buffer::from_custom_allocation(
                 NonNull::new_unchecked(offsets.as_mut_ptr() as *mut u8),
                 offsets.len() * std::mem::size_of::<i32>(),
                 Arc::new(offsets),
             )
         };
         let null_buffer = unsafe {
-            Buffer::from_foreign(
+            Buffer::from_custom_allocation(
                 NonNull::new_unchecked(bitmap.as_mut_ptr()),
                 bitmap.len(),
                 Arc::new(bitmap),

arrow/src/buffer/immutable.rs

@@ -22,6 +22,7 @@ use std::sync::Arc;
 use std::{convert::AsRef, usize};
 
 use crate::alloc::{Allocation, Deallocation};
+use crate::ffi::FFI_ArrowArray;
 use crate::util::bit_chunk_iterator::{BitChunks, UnalignedBitChunk};
 use crate::{bytes::Bytes, datatypes::ArrowNativeType};
 
@@ -73,7 +74,7 @@ impl Buffer {
     /// bytes. If the `ptr` and `capacity` come from a `Buffer`, then this is guaranteed.
     pub unsafe fn from_raw_parts(ptr: NonNull<u8>, len: usize, capacity: usize) -> Self {
         assert!(len <= capacity);
-        Buffer::build_with_arguments(ptr, len, Deallocation::Native(capacity))
+        Buffer::build_with_arguments(ptr, len, Deallocation::Arrow(capacity))
     }
 
     /// Creates a buffer from an existing memory region (must already be byte-aligned), this
@@ -83,18 +84,41 @@ impl Buffer {
     ///
     /// * `ptr` - Pointer to raw parts
     /// * `len` - Length of raw parts in **bytes**
-    /// * `owner` - A [crate::alloc::Allocation] which is responsible for freeing that data
+    /// * `data` - An [ffi::FFI_ArrowArray] with the data
     ///
     /// # Safety
     ///
     /// This function is unsafe as there is no guarantee that the given pointer is valid for `len`
     /// bytes and that the foreign deallocator frees the region.
-    pub unsafe fn from_foreign(
+    #[deprecated(
+        note = "use from_custom_allocation instead which makes it clearer that the allocation is in fact owned"
+    )]
+    pub unsafe fn from_unowned(
+        ptr: NonNull<u8>,
+        len: usize,
+        data: Arc<FFI_ArrowArray>,
+    ) -> Self {
+        Self::from_custom_allocation(ptr, len, data)
+    }
+
+    /// Creates a buffer from an existing memory region. Ownership of the memory is tracked via reference counting
+    /// and the memory will be freed using the `drop` method of [crate::alloc::Allocation] when the reference count reaches zero.
+    ///
+    /// # Arguments
+    ///
+    /// * `ptr` - Pointer to raw parts
+    /// * `len` - Length of raw parts in **bytes**
+    /// * `owner` - A [crate::alloc::Allocation] which is responsible for freeing that data
+    ///
+    /// # Safety
+    ///
+    /// This function is unsafe as there is no guarantee that the given pointer is valid for `len` bytes
+    pub unsafe fn from_custom_allocation(
         ptr: NonNull<u8>,
         len: usize,
         owner: Arc<dyn Allocation>,
     ) -> Self {
-        Buffer::build_with_arguments(ptr, len, Deallocation::Foreign(owner))
+        Buffer::build_with_arguments(ptr, len, Deallocation::Custom(owner))
     }
 
     /// Auxiliary method to create a new Buffer
@@ -535,7 +559,7 @@ mod tests {
     fn test_from_foreign_vec() {
         let mut vector = vec![1_i32, 2, 3, 4, 5];
         let buffer = unsafe {
-            Buffer::from_foreign(
+            Buffer::from_custom_allocation(
                 NonNull::new_unchecked(vector.as_mut_ptr() as *mut u8),
                 vector.len() * std::mem::size_of::<i32>(),
                 Arc::new(vector),

arrow/src/buffer/mutable.rs

@@ -267,7 +267,7 @@ impl MutableBuffer {
     #[inline]
     pub(super) fn into_buffer(self) -> Buffer {
         let bytes = unsafe {
-            Bytes::new(self.data, self.len, Deallocation::Native(self.capacity))
+            Bytes::new(self.data, self.len, Deallocation::Arrow(self.capacity))
         };
         std::mem::forget(self);
         Buffer::from_bytes(bytes)

arrow/src/bytes.rs

@@ -59,7 +59,7 @@ impl Bytes {
     /// This function is unsafe as there is no guarantee that the given pointer is valid for `len`
     /// bytes. If the `ptr` and `capacity` come from a `Buffer`, then this is guaranteed.
     #[inline]
-    pub unsafe fn new(
+    pub(crate) unsafe fn new(
         ptr: std::ptr::NonNull<u8>,
         len: usize,
         deallocation: Deallocation,
@@ -92,10 +92,10 @@ impl Bytes {
 
     pub fn capacity(&self) -> usize {
         match self.deallocation {
-            Deallocation::Native(capacity) => capacity,
+            Deallocation::Arrow(capacity) => capacity,
             // we cannot determine this in general,
             // and thus we state that this is externally-owned memory
-            Deallocation::Foreign(_) => 0,
+            Deallocation::Custom(_) => 0,
         }
     }
 }
@@ -104,11 +104,11 @@ impl Drop for Bytes {
     #[inline]
     fn drop(&mut self) {
         match &self.deallocation {
-            Deallocation::Native(capacity) => {
+            Deallocation::Arrow(capacity) => {
                 unsafe { alloc::free_aligned::<u8>(self.ptr, *capacity) };
             }
-            // foreign interface knows how to deallocate itself.
-            Deallocation::Foreign(_) => (),
+            // The automatic drop implementation will free the memory once the reference count reaches zero
+            Deallocation::Custom(_allocation) => (),
         }
     }
 }

arrow/src/ffi.rs

@@ -538,7 +538,8 @@ unsafe fn create_buffer(
     assert!(index < array.n_buffers as usize);
     let ptr = *buffers.add(index);
 
-    NonNull::new(ptr as *mut u8).map(|ptr| Buffer::from_foreign(ptr, len, owner))
+    NonNull::new(ptr as *mut u8)
+        .map(|ptr| Buffer::from_custom_allocation(ptr, len, owner))
 }
 
 fn create_child(