How to properly mutex lock a struct used by trampolines called from C

I'm using this technique for C code, called from Rust, call a Rust's method back.

pub type OnVpnLog = std::sync::Arc<std::sync::Mutex<dyn Fn(*const libc::c_char) + Send + Sync>>;
pub type OnVpnSomething = std::sync::Arc<std::sync::Mutex<dyn Fn(i32) + Send + Sync>>;

struct OVPNClientInner {
    on_vpn_log: Option<OnVpnLog>,
    on_vpn_something: Option<OnVpnSomething>,
}

unsafe extern "C" fn on_log_trampoline(
    buffer: *const libc::c_char,
    user_data: *mut libc::c_void,
) -> libc::c_int {
    let ovpn_client_inner = &mut *(user_data as *mut OVPNClientInner);

    (ovpn_client_inner.on_vpn_log.as_ref().unwrap().lock().unwrap())(buffer);
    0
}

unsafe extern "C" fn on_something_trampoline(
    user_data: *mut libc::c_void,
) -> libc::c_int {
    let ovpn_client_inner = &mut *(user_data as *mut OVPNClientInner);

    (ovpn_client_inner.on_vpn_something.as_ref().unwrap().lock().unwrap())(0);
    0
}

In this specific case, I'm calling on_vpn_log which is behing a Mutex. However, this is not sufficient. Suppose that I call on_vpn_log while the function on_vpn_log is being replaced by another one. This would lead to undefined behaviour.

Also, the 2 trampolines could in the future with a code update try to access something inside OVPNClientInner. We must ensure that OVPNClientInner is accessed only once at a time.

One option would be a global static mutex, but that would block one instance of OVPNClientInner while another was being updated.

Another option would be to put a mutex inside OVPNClientInner:

struct OVPNClientInner {
    on_vpn_log: Option<OnVpnLog>,
    on_vpn_something: Option<OnVpnSomething>,
    mutex: Mutex
}

But OVPNClientInner is shared between C and Rust, so no ffi-unsafe types can be present. Also that mutex could be changed while we try to lock it (maybe we could make it immutable, I don't know)

What would be a good solution here to provide good safety?

There are already FFI-unsafe types present in it. Option (most of the time) and Arc are FFI-unsafe.

If these trampoline functions can be called concurrently, it is unsound to create an &mut OVPNClientInner inside them as having multiple mutable references at once is instantly UB. You should be able to just create an &OVPNClientInner and the Rust type system can guide you from there to make sure no unsoundness occurs.

you mean doing let ovpn_client_inner = & *(user_data as *mut OVPNClientInner); would guarantee that, even if the code is called from C, no undefined behaviour would happen?

It alone wouldn't guarantee the absence of UB - the caller could still pass in an invalid pointer. But assuming the pointer is valid it would completely protect you from any data races, yes.

Also, you should write it as &*user_data.cast::<OVPNClientInner>(), because .cast is a strictly less powerful (and thus less dangerous) API.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.