Idiomatic FFI callbacks that assume mutable global state

Hi,

I’m trying to write an rlib that will provide a nice interface to a fairly old chunk of C code. The way this code is structured is making it rather difficult and I could use some advice. Apologies about all the words and code up front.

The C code I’m looking to wrap minifies down to something like this:

// problem.c
int a = 0;

extern void foo(int);
extern void bar(int);

void quux(void) {
        ++a;
        foo(a);
        // 20 years of optimized C code here that does stuff
        bar(a);
}

foo and bar will be rust functions I will provide and link in the resulting rlib. I cannot control parameters to foo or bar. Additionally they’re on a hot path and very performance sensitive.

I am interested in consumers of my library to be able to provide a callbacks which will execute on foo and bar, effectively muxing foo and bar. E.g

// quux.rs
extern "C" {
    fn quux();
}

static mut FOO_HOOKS = Vec<Box<FnMut(u32)>> = Vec::new();

pub unsafe fn hook_foo<T: FnMut(u32) + 'static>(h : T) {
    FOO_HOOKS.push(Box::new(h));
}

#[no_mangle]
extern "C" foo(a: u32) {
    unsafe { FOO_HOOKS.iter_mut().for_each(|x| x(a)) }
}

pub unsafe fn do_quux() {
    quux()
}
// consumer.rs
use quux::{hook_foo, hook_bar, do_quux};

fn main() {
    unsafe {
        hook_foo(|a| {
            println!("{}", a);
        });
        do_quux();
    }
}

I understand this is wildly race-y, however the C code is not threaded and will call foo and bar synchronously meaning this code currently should be no worse than the corresponding C code I would have written.

My issue comes when I want to have the hooks share mutable state, essentially something like this rust pseudocode:

// my-wishlist.rs
let mut baz = Vec::new();
unsafe {
    hook_foo(|a| {
        baz.push(a);
    });
    hook_bar(|a| {
        baz.push(a);
    });
    do_quux();
}

This (again assuming do_quux is only called once at any given time and nothing else reads or writes baz) should be safe as foo and bar cannot be executed concurrently. Obviously it wont compile for a few reasons.

My main question is how to best implement something roughly similar to this, given the constraints that we can’t modify the C or introduce unneeded perf hits. I don’t think there is a good solution, given the way the C code is designed, so I’m mostly looking for whatever’s least bad. Right now I’m just passing it through with raw pointers, but would welcome any improvements!

Hopefully I haven’t missed anything obvious, and thanks for taking the time to read all this!

If you know that foo and bar are never called concurrently, perhaps what you want is to define a trait

pub trait FooBar {
    fn foo(&mut self);
    fn bar(&mut self);
}

This makes it possible for foo and bar implementors to share mutable state.

Completely and utterly off-topic, but this topic contains the most hilarious bug I’ve ever seen: https://youtu.be/O3Nuf2_bLHg (recorded on the phone because screen capture makes it disappear). I wonder if this is UB-related

5 Likes

Sorry, I’m not sure I understand the trait suggestion. Could you explain a bit more what you mean?

If you know it won’t be multithreaded, use RefCell. You will also need thread_local! statics to fulfill the F : 'static bound:

Note: since panic!-ing accross FFI is UB, I’ve taken the liberty to add an abort-on-unwind bomb guard.

use ::std::{*,
    cell::RefCell,
};

/// Ideally this should be in the implementation of hook_foo and hook_bar
fn ffi_wrap<Ret, F : FnMut(u32) -> Ret> (
    mut f: F,
) -> impl FnMut(u32) -> Ret
{
    struct AbortOnDrop;
    impl Drop for AbortOnDrop { fn drop(&mut self) { process::abort() } }
    move |x| {
        let anti_unwind = AbortOnDrop;
        let ret = f();
        mem::forget(anti_unwind);
        ret
    }   
}
thread_local! {
    static BAZ: RefCell<Vec<u32>> = RefCell::new(Vec::new());
}
unsafe {
    hook_foo(ffi_wrap(|a| BAZ.with(|baz| {
        baz.borrow_mut().push(a);
    })));
    hook_bar(ffi_wrap(|a| BAZ.with(|baz| {
        baz.borrow_mut().push(a);
    })));
    do_quux();
}
1 Like

Great, thank you. This is roughly what I ended up implementing, except I just set panic=abort and did it via UnsafeCell. I’ll profile the RefCell and see if it introduces a measurable difference, and if not go wit that. Thanks for taking the time to read and understand all of that.

Thread locals are not 'static, as they’ll be dropped when the thread exit, which obviously can happen before the process terminates.

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.