Trying to understand how this is FFI-safe

[marioortizmanero]

I was trying to add support for abi_stable to this crate, which would be very helpful for more people as well. Not sure if you've ever used it, maybe you can help me a bit. Anyway, adding support should be as easy as deriving the StableAbi trait, which contains information about the type's layout and similars for safety guarantees on top of repr(C). I was simply adding #[cfg_attr(feature = "sabi", derive(abi_stable::StableAbi))] to each type.

Unfortunately, this strategy stopped working as soon as I reached FfiContext. It points to a FfiWaker, which contains a WakerUnion, which in turn has a Waker. The problem here is that Waker is not defined in async_ffi; it's from std. And as far as I know, it's not FFI-safe at all by itself. In its implementation, it's a wrapper for RawWaker, which contains a vtable with Rust functions, which are obviously not repr(C).

The only sound way to use a Waker would be, as far as I know, to have a pointer to it, which is then accessed only from the runtime, and not the plugin. This is what this crate seems to do; it implements the FfiWaker as a pointer + vtable, and uses it as an opaque type, which will work just fine. What I didn't fully understand is how FfiWaker has a waker: ManuallyDrop<Waker> field, which is not FFI-safe. I'm guessing that it should never be accessed from the plugin, and be kept as a private field.

Because of this, generating bindings with cbindgen to see what the actual interface would look like showed warnings. This is exactly where abi_stable failed to continue as well.

$ cbindgen src/lib.rs --config cbindgen.toml
WARN: Can't find Waker. This usually means that this type was incompatible or not found.
(...)

Since FfiWaker is not fully FFI-safe by itself, I propose the FfiWakerBase struct for the public interface, which doesn't have access to internal, non FFI-safe data such as WakerUnion. It's simply this:

// Inspired by Gary Guo (github.com/nbdd0121)
//
// The base is what can be accessed through FFI, and the regular struct contains
// internal data (the original waker).
#[repr(C)]
#[cfg_attr(feature = "sabi", derive(abi_stable::StableAbi))]
struct FfiWakerBase { // Contains all the FFI-safe data
    vtable: *const FfiWakerVTable,
}
#[repr(C)]
struct FfiWaker { // Contains the internal data and also the base type, to which it can be casted
    base: FfiWakerBase,
    waker: WakerUnion,
}

This currently worked in the C plugin tests because their definition of FfiContext doesn't include the WakerUnion field, but that's kind of a hack, I think. Or at least impossible for stable_abi to pick up. FfiWakerBase lets us use abi_stable and generate C bindings automatically without warnings, at the small cost of a cast to/from FfiWaker when the internal data is going to be used. As per the C standard, this will always be safe.

Is my reasoning correct? Any ideas?

[oxalica]

Close via #10