Should FFI callbacks into Rust be marked `unsafe`?

Suppose I am working with a C library that has hooks that pass a pointer to my code.

typedef struct { ... } PapayaInfo;
typedef void (*PapayaHook)(PapayaInfo* info);
void set_papaya_hook(PapayaHook hook);

The Rust FFI for the PapayaHook type might look like this:

type PapayaHook = extern "C" fn(*mut PapayaInfo);

I'm asking why it's not unsafe extern "C" instead. It seems like the type should allow unsafe callbacks, since any callback function that dereferences the pointer is unsafe in the Rust sense.

But what I see "in the wild" is that unsafe is not present (example 1, example 2). Users' callback functions have to be declared as safe, even though in practice they're full of unsafe code.

Are the rules different for FFI code? Is it simply no big deal in this case? How strictly should we interpret Rust's safety rules?

1 Like

I agree that unsafe extern fn is more correct in these cases. It's probably overlooked because the unsafe qualifier only makes a difference in practice to Rust code that calls the function. In my experience with FFI code, these callback functions are typically never called from Rust.

1 Like

I think the current documentation for unsafe allows for multiple interpretations. One can take a mechanical view and tag a function unsafe when it meets certain criterion (raw pointer deref... bam! unsafe), or one can take a holistic view and ask "If I tag this unsafe, will I reduce the chance of errors?". Most of the time both views arrive at the same answer, but not for callbacks. The callback definitely derefs a raw pointer, but what errors can be avoided by marking the function unsafe? Is a distraction (the unsafe tag) being added to solve a problem that's not there?

What do others think?

This doesn't match my understanding of unsafe. Callbacks are safe, even if they do a lot of unsafe things inside.

How much of the code inside the function is unsafe is uninteresting for the unsafety of a function.

unsafe in the function signature means that the caller should take care and that the function does not guarantee that Rusts guarantees hold even after the call to the function has finished. Take for example std::mem::transmute: it happily returns you values of any type - and crash happily if you unsafely transmuted to the wrong type and later access it.

Another example of a function that is internally unsafe, but not externally is slice::split_at_mut: it uses unsafety internally, but it guarantees that Rusts guarantees about the input reference and the two output references hold - it is safe to call.

Finally, unsafety doesn't propagate outwards - just because it's a piece of C code calling that function doesn't make it unsafer: it's the callers job to pass in the right values to call the function. For that reasons, callbacks do a lot of unsafe operations inside (mostly because they dereference a lot of raw pointers), but are by themselves not very unsafe: as long as the caller correctly calls the function, we're all good. The fact that the implementor must take care with the input values comes from the raw input values not the chance that the calling code might call the function wrong.

P.S.: I read through the book and haven't found where it's necessarily ambiguous. unsafe is for the points where you have to uphold invariants within Rust code. A callback passed to C can uphold all invariants if it were called from Rust code. The example given std::mem::transmute can't. Acquiring a raw pointer through any means is also strictly safe in Rust, as long as you don't dereference it.

2 Likes

In my opinion it doesn't matter whether it is Rust or C calling the function. If the function has any sort of contract that isn't checked by the compiler, but required for safety, such as taking a raw pointer and assuming it to be valid, then that function must be marked unsafe.

On the flip side there is nothing inherently unsafe about user defined callbacks, and if the callback can be called with any value safely, then the callback doesn't necessarily need to be unsafe. In winapi however I typically just end up marking every single function pointer everywhere as unsafe because I can't be bothered figuring out which one has a safe contract and which doesn't. Also making them all unsafe is consistent and reinforces the unsafety of working with winapi.

In the examples you provided, I'd definitely put unsafe on those function pointers, because they clearly have an unsafe contract that has to be upheld. I think in many cases authors simply forgot to mark function pointers as unsafe because they're used to extern blocks automatically marking functions as unsafe, so they probably expected extern function pointers to be unsafe by default as well, which they totally aren't.

3 Likes

This doesn't match my understanding of unsafe. Callbacks are safe, even if they do a lot of unsafe things inside.

It's subtle, but this is definitely incorrect. The motivating example here is the following actual code (note the comment):

/// Drop a T that lives in an Erlang resource. (erlang_nif-sys requires us to declare this
/// function safe, but it is of course thoroughly unsafe!)
extern "C" fn resource_destructor<T>(_env: NIF_ENV, handle: MUTABLE_NIF_RESOURCE_HANDLE) {
    unsafe {
        let aligned = align_alloced_mem_for_struct::<T>(handle);
        let res = aligned as *mut T;
        ptr::read(res);
    }
}

Of course this function is safe as long as every caller (Rust and C alike) always passes correct arguments to it. It'll only trigger undefined behavior if you pass incorrect arguments. But that is the definition of an unsafe function.

1 Like