What is the correct way to drop a mem::forgotten array of strings sent to C via an out pointer and then passed back to Rust for deletion?

Question

I would like to return some strings to C via a Rust FFI call. I also would like to ensure they're cleaned up properly.

I'm creating the strings on the Rust side and turning them into an address of an array of strings.

use core::mem;
use std::ffi::CString;

#[no_mangle]
pub extern "C" fn drop_rust_memory(mem: *mut ::libc::c_void) {
    unsafe {
        let boxed = Box::from_raw(mem);
        mem::drop(boxed);
    }
}

#[no_mangle]
pub extern "C" fn string_xfer(strings_out: *mut *mut *mut ::libc::c_char) -> usize {
    unsafe {
        let s1 = CString::new("String 1").unwrap();
        let s2 = CString::new("String 2").unwrap();
        let s1 = s1.into_raw();
        let s2 = s2.into_raw();

        let strs = vec![s1, s2];
        let len = strs.len();

        let mut boxed_slice = strs.into_boxed_slice();
        *strings_out = boxed_slice.as_mut_ptr() as *mut *mut ::libc::c_char;
        mem::forget(boxed_slice);
        len
    }
}

On the C side, I call the Rust FFI function, print the strings and then attempt to delete them via another Rust FFI call.

extern size_t string_xfer(char ***out);
extern void drop_rust_memory(void* mem);

int main() {
    char **receiver;
    int len = string_xfer(&receiver);

    for (int i = 0; i < len; i++) {
        printf("<%s>\n", receiver[i]);
    }

    drop_rust_memory(receiver);

    printf("# rust memory dropped\n");
    for (int i = 0; i < len; i++) {
        printf("<%s>\n", receiver[i]);
    }
    return 0;
}

This appears to work. For the second printing after the drop, I would expect to get a crash or some undefined behavior, but I get this

<String 1>
<String 2>
# rust memory dropped
<(null)>
<String 2>

which makes me less sure about the entire thing.

"or some undefined behavior" what do you expect from UB ? UB is UB, by definition you can't expect anything so what do you expect XD. This don't make sense. — Stargateur, Dec 17 '19 at 23:26

Stargateur · Answer 1 · 2019-12-18T15:26:13.210

First you may want take a look at Catching panic! when Rust called from C FFI, without spawning threads. Because panic will invoke undefined behaviour in this case. So you better catch the panic or avoid have code that can panic.

Secondly, into_boxed_slice() is primary use when you don't need vector feature any more so "A contiguous growable array type". You could also use as_mut_ptr() and forget the vector. That a choice either you want to carry the capacity information into C so you can make the vector grow or you don't want. (I think vector is missing a into_raw() method but I'm sure you can code one (just an example) to avoid critical code repetition). You could also use Box::into_raw() followed with a cast to transform the slice to pointer:

use std::panic;
use std::ffi::CString;

pub unsafe extern "C" fn string_xfer(len: &mut libc::size_t) -> Option<*mut *mut libc::c_char> {
    if let Ok(slice) = panic::catch_unwind(move || {
        let s1 = CString::new("String 1").unwrap();
        let s2 = CString::new("String 2").unwrap();

        let strs = vec![s1.into_raw(), s2.into_raw()];

        Box::into_raw(strs.into_boxed_slice())
    }) {
        *len = (*slice).len();
        Some(slice as _)
    } else {
        None
    }
}

Third, your drop_rust_memory() only drop a pointer, I think you are doing a total UB here. Rust memory allocation need the real size of the allocation (capacity). And you didn't give the size of your slice, you tell to Rust "free this pointer that contain a pointer to nothing (void so 0)" but that not the good capacity. You need to use from_raw_parts_mut(), your C code must give the size of the slice to the Rust code. Also, you need to properly free your CString you need to call from_raw() to do it (More information here):

use std::ffi::CString;

pub unsafe extern "C" fn drop_rust_memory(mem: *mut *mut libc::c_char, len: libc::size_t) {
    let slice = Box::from_raw(std::slice::from_raw_parts_mut(mem, len));
    for &x in slice.iter() {
        CString::from_raw(x);
    } // CString will free resource don't use mem/vec element after
}

To conclude, you should read more about undefined behaviour, it's not about "expect a crash" or "something" should happen. When your program trigger a UB, everything can happen, you go into a random zone, read more about UB on this amazing LLVM blog post

Note about C style prefer return the pointer and not the size because strings_out: *mut *mut *mut ::libc::c_char is a ugly thing so do pub extern fn string_xfer(size: &mut libc::size_t) -> *mut *mut libc::c_char. Also, How to check if function pointer passed from C is non-NULL

Thank you for your detailed explanation. One other thing, should I `mem:forget` each CString in the vec individually, too? Or are they properly moved as part of the vec even being raw pointers? — marathon, Dec 18 '19 at 01:03
@marathon I update the answer, no you used [`into_raw()`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.into_raw) as you can see it **consume** `self` so you don't have to forget it (in fact you can't this will not compile), the method already do it for you. — Stargateur, Dec 18 '19 at 01:24
`into_boxed_slice()` is indeed important here because it releases any excess capacity owned by the `Vec`. If you use `as_ptr` + `mem::forget` you also need to call `shrink_to_fit` first. In this case it probably doesn't matter because the `Vec` is *probably* initially created with `len` = `cap` = 2, but I don't know that the API guarantees that. — trent, Dec 18 '19 at 12:39
Also, combining `into_boxed_slice` with `Box::into_raw` obviates the need for `mem::forget`, which seems like a clear safety win to me over `shrink_to_fit` + `as_(mut_)_ptr` + `mem::forget` where if you forget either of the first or the last thing the behavior is undefined. — trent, Dec 18 '19 at 12:48
@trentcl I wanted to advise `Box::into_raw` but this is not possible for `Box<[T]>`, "If you use as_ptr + mem::forget you also need to call shrink_to_fit" no you don't you can also carry capacity information and give it back to rust when needed as I said it's a choice. — Stargateur, Dec 18 '19 at 13:21
[Not possible?](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=e552537f7a2721b3f414a42c1edcbf45) — trent, Dec 18 '19 at 13:32
@trentcl a cast feel wrong here, look like dark magic, I don't like it. I'm not even sure it does what it's suppose to do. — Stargateur, Dec 18 '19 at 13:36
It's just a cast from `*mut [T]` to `*mut T`. It drops the length. It can't do anything else because casts only change one thing at a time (e.g. if you try `as *mut *mut u8` instead of `c_char` you will get an error). Anyway, YMMV, but with how many questions we see on here abusing `as_ptr`, I feel `into_boxed_slice` + `into_raw` is less error-prone because you cannot forget to `forget` it. — trent, Dec 18 '19 at 14:06

What is the correct way to drop a mem::forgotten array of strings sent to C via an out pointer and then passed back to Rust for deletion?

1 Answers1