CODEWITHSUNDEEP
rust
Gatonito
Gatonito

Reputation: 1894

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?

Upvotes: 0

Views: 287

Answers (1)

kmdreko
kmdreko

Reputation: 60597

If on_vpn_log and on_vpn_something can be updated and used separately, I'd recommend:

pub type OnVpnLog = dyn Fn(*const libc::c_char) + Send + Sync;
pub type OnVpnSomething = dyn Fn(i32) + Send + Sync;

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

If not, use a single Mutex when creating and using the user_data:

struct OVPNClientInner {
    on_vpn_log: Option<Box<OnVpnLog>>,
    on_vpn_something: Option<Box<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 = &*(user_data as *mut Mutex<OVPNClientInner>); // <--------
    
    (ovpn_client_inner.lock().unwrap().on_vpn_log.as_ref().unwrap())(buffer);
    0
}

But OVPNClientInner is shared between C and Rust, so no ffi-unsafe types can be present.

I'm not familiar with this API, but typically an opaque void* called "user data" is unused by the framework itself. The ffi boundary being void* and unused on the C side means there's no concern with using Mutex in your structure, you can put almost anything actually.

The only thing you should be concerned about is if what you send is Sync, which means its safe to use from multiple threads. Both the implementations above are safe in that regard.

As an aside, you currently have an Option that you're assuming is present. You should either remove it or be sure to handle its absence properly.

Upvotes: 1

Related Questions