ARROW-10149: [Rust] Improved support for externally owned memory regions

[jorgecarleitao]

Background

Currently, a memory region (arrow::buffer::BufferData) always knows its capacity, that it uses to drop itself once it is no longer needed. It also knows whether it needs to be dropped or not via BufferData::owner: bool.

However, this is insufficient for the purposes of supporting the C Data Interface, which requires informing the owner that the region is no longer needed, typically via a function call (release), for reference counting by the owner of the region.

This PR

This PR generalizes BufferData (and renames it to Bytes, which is more natural name for this structure, a-la bytes::Bytes) to support foreign deallocators. Specifically, it accepts two deallocation modes:

• Native(usize): the current implementation
• Foreign(Fn): an implementation that calls a function (which can be used to call a FFI)

FYI @pitrou , @nevi-me @paddyhoran @sunchao

Related to #8052 , which IMO is blocked by this functionality.

[github-actions]

https://issues.apache.org/jira/browse/ARROW-10149

[jorgecarleitao]

This was not being used, and thus I dropped it.

[pitrou]

Perhaps using a closure that mutates a captured variable?
Apparently you need to use either FnMut or a Cell:
https://stackoverflow.com/questions/38677736/passing-a-closure-that-modifies-its-environment-to-a-function-in-rust

[jorgecarleitao]

Done. I had to introduce a Mutex, because a FnMut is now mutable, while the data itself is not. I am not very happy with this, but it makes sense as we cannot assume that the C data interface is thread-safe, right?

[jorgecarleitao]

I reverted this. I do not think that that Fn should be mutable, as it is just performing an FFI call, over which Rust does not need to know about mutability. I am still trying to test it, but I think that something like Arc<dyn Fn(&mut Bytes)> is a better signature,.

[pitrou]

Why don't you use a Cell?

[jorgecarleitao]

        let b = Cell::new(false);
        let dealloc = Arc::new(|bytes: &mut Bytes| {
            *b.get_mut() = true;
            assert_eq!(bytes.as_slice(), &b"hello"[1..4]);
        });

does not compile because it requires moving b to inside the closure: if we move b to inside the closure (using move), the closure is no longer Fn, but FnMut. If the closure is FnMut, we can no longer wrap it inside an Arc, as Arc is immutable. To make it immutable, we need to wrap it around a Mutex.

The difference between the code we are going here and the example in SO is that we have an immutable function, as the underlying resource that this function acts upon is outside rust. I.e. from rust's perspective, the function is Fn.

One way to test this would be to make the closure to write something to a file, and verify that that was written. I.e. test that the function mutated something outside of Rust.

[pitrou]

After trying out varying things, I got the following to work:

use std::cell::Cell;
use std::sync::Arc;

pub type VoidFn = Arc<dyn Fn()>;

fn main() {
    let integer = Arc::new(Cell::new(5));
    let inner = integer.clone();
    let closure = Arc::new(move || {
        inner.set(inner.get() + 1);
    });
    execute_closure(closure);
    println!("After closure: {}", integer.get());
}

fn execute_closure(func: VoidFn)
{
    func();
}

I may be missing something though.

[pitrou]

By the way, by definition a destructor will mutate some state (visible or not), so it seems FnMut may be fine too.

[jorgecarleitao]

That is awesome! Thanks a lot for the help and insight @pitrou .

The destructor mutates state. Shouldn't that state be only the state of self? IMO that is the reason rust's Drop requires a mutable ref of self, fn drop(&mut self).

[pitrou]

I have no idea, I am out of my depth here (not a Rust developer :-)).

[jorgecarleitao]

Closing in favor of #8401