[vlc-devel] [PATCH 1/2] libvlc: Add Rust API for writing modules in rust

[Romain Vimont]

On Sun, Sep 20, 2020 at 02:39:33AM +0530, Kartik Ohri wrote:
>        - Added a utils module to vlccore-rs
>                This module can contain commonly used utilities for Rust
>                modules. For instance, passing Rust allocated strings to
>                C code. This is an easy thing to mess up. Rust allocated
>                pointers should not be free()`ed in C code. Hence, a
>                utility method is provided in vlccore-rs::utils to
>                ensure such issues are avoided.

I encounter the same problem (not just for Strings): in addition to
correctly transfer ownership between C and Rust, the memory must be
deallocated on the same side as it was allocated (either both in C or
both in Rust).

In particular, passing a Rust-allocated memory to C by just calling
mem::forget() in Rust (so that C free() it) is incorrect:
https://doc.rust-lang.org/std/boxed/struct.Box.html#method.into_raw

The recommended solution is to pass the pointer back to Rust so that it
deallocates the memory. But in practice, if we create a VLC core struct
from Rust with allocated fields (in my case es_format_t) and transfer
its ownership to C, we don't want to release each pointer from Rust (the
core will just call es_format_Clean() and free them directly).

To solve the problem, you propose to call libc::malloc() from Rust and
free() from C (or malloc() from C and libc::free() from Rust). I like
this solution, but IIUC, it is not guaranteed to be correct, because the
allocators used by Rust and C may be different:

> To free the data behind the C pointer you should call a C function
> that does it, so that definitely the same C’s allocator is used. If
> the pointer comes from C’s malloc, then Rust’s libc::free may work,
> except it’s dangerous to do so if Windows DLLs are involved, since
> each DLL may have separate runtime.

https://users.rust-lang.org/t/idiomatic-way-to-handle-pointer-to-values-returned-by-ffi-call/17085/3

> Rust 0.11.0 through 1.31.1 used jemalloc. Rust 1.32.0 changed to use
> the system's default allocator.
>
> Additionally, Rust 1.28.0 introduced a mechanism that applications can
> use to replace the global allocator with one of their choosing.
>
> It's important to note that, although Rust now uses the system's
> default allocator by default, that doesn't mean that C libraries use
> the same allocator, even if it's literally malloc. For example, on
> Windows, if you use a C library that's been compiled with Visual C++
> 2008 while your Rust binary has been compiled with Visual Studio 2019
> Build Tools, there will be two C runtime libraries loaded in your
> process: the C library will use msvcr90.dll while your Rust binary
> will use ucrtbase.dll. Each C runtime library manages its own heap, so
> memory allocated by one cannot be freed by the other.

https://stackoverflow.com/a/57879688/1987178

(Now that Rust uses the system allocator by default, my understanding is
that it might be safe for VLC, but in theory they could change it again
in future versions.)

One possible (and generic) way to solve the issue could be to expose
(an equivalent of) vlc_alloc() via FFI, so that we could call
ffi::vlc_alloc() from Rust. That way, we could safely free() the pointer
from C.

And we could just use libc::memcpy() to fill the C-allocated memory with
any Rust #[repr(C)] data.

What do you think?

> diff --git a/src/vlccore-rs/src/utils.rs b/src/vlccore-rs/src/utils.rs
> new file mode 100644
> index 0000000000..3d9e25ac31
> --- /dev/null
> +++ b/src/vlccore-rs/src/utils.rs
> @@ -0,0 +1,19 @@
> +use libc::{c_char, malloc};
> +use std::ffi::CString;
> +use std::ptr::null_mut;
> +use std::slice::from_raw_parts_mut;
> +
> +pub unsafe fn convert_string_to_ptr(string: String) -> *mut c_char {
> +    if string.is_empty() {
> +        return std::ptr::null_mut();
> +    }
> +    let size = string.len() + 1;
> +    CString::new(string)
> +        .map(|ptr| {
> +            let c_ptr = malloc(size * std::mem::size_of::<u8>()) as *mut u8;
                           ^ this malloc

> +            let slice = from_raw_parts_mut(c_ptr, size);
> +            slice[..size].copy_from_slice(ptr.as_bytes_with_nul());
> +            c_ptr as *mut c_char
> +        })
> +        .unwrap_or(null_mut())
> +}

Regards