Using Rust's advanced memory management features without invalidating a bound C library's references

Hi folks! So, this is a continuation of this thread in a way, but the problem has changed enough at this point that I thought making a new thread was prudent. I was hoping to triumphantly return to that thread with news of my victory, but no such luck so far. :persevere: In short, I'm writing bindings to a C library that stores pointers inside of structs, passes other pointers to those structs in callbacks, and assumes that everything will stay put in memory while it's doing stuff like this. However, if I try to store the pointers it gives me in any kind of fancy memory management tool or other generic container that Rust provides (everything from Arc to Option), the Rust compiler starts moving things around in memory in ways that invalidate the C library's references, causing segfaults. I'm trying to address this by using Pin, but it's proving to be infuriatingly difficult: it seems like every single thing that crosses the FFI needs to be pinned, including a user-facing Rust struct that needs to be passed back and forth from C in a rather hairy way. I haven't actually managed to get a design with everything pinned to compile yet, though, so who knows if that will actually fix the crashes—it just seems like it probably will, looking at where things explode in a debugger over time.

I first encountered this problem early on, when I was trying to capture a pattern of usage of the library which involved circular references. I posted about it and got some nice advice involving types like Rc and UnsafeCell. With some work and study, I managed to use that approach to write something that would compile, but found that it provoked crashes in the C library left and right. I didn't really understand why at the time, but managed to deal with the problem by looking closer at the C library and realizing that it could actually work with much more constrained access between values than it is idiomatically used with in C. That allowed me to design a different way of interacting with it in Rust that didn't require anything like Rc, just normal references and a few raw pointers wrapped in structs. This kept the C library perfectly happy, allowed me to create a pleasant public API, and I went on my way finishing out the bindings and writing examples and the like.

Yesterday I at last decided to try using it to make a test audio plugin with vst-rs (my bindings are to a graphics library designed around the needs of audio plugins). That crate wants you to store stuff pertaining to your GUI inside of types like Box and Option, so the old rug-out-from-under-the-C-library problem reared its head again. Trying to solve it, I came across this thread which seems to suggest that Rust will just tend to move things around when going from a raw mutable pointer to a safe mutable reference so you shouldn't depend on that not happening. I also read this blog post which suggests using handles instead of raw pointers so you don't have to worry about things being moved, which seems reasonable in pure Rust but would require the C library to follow suit. Last, I found std::pin, which seemed like exactly what I needed.

I managed to get all the pointers the C library gives me pinned and haven't seen a crash involving them since, so that's great. However, I'm still having a horrible mess of trouble with the Rust struct that gets passed back and forth across the FFI. The big trouble there is that the C library doesn't give me a pointer to the Rust struct, but rather to the inner C struct that the Rust struct wraps, so I need a way to go from the C struct pointer back to the Rust struct. I was using an internal HashMap that maps inner pointer addresses to pointers to their respective Rust structs, but that stopped working once Rust started moving stuff around because the addresses stopped being accurate. So, I figured I need to also pin the Rust struct, but I really have no desire to present users with an API where the main user-facing struct needs to be stored in a Pin<Box<>> and requires them to do all kinds of awkward ceremony to use it. I've tried doing stuff with wrapper structs and Deref and such but I haven't found anything that pleases the borrow checker and keeps things looking neat from the outside. I also still need some way to go from a pointer to the inner C struct back to the Rust struct it maps to; I've had some luck storing pinned references in the HashMap but it's not entirely clear how I should relate that to the rest of the application with the design of the main struct in flux. I have a feeling that none of this is quite the right way to approach this problem, so if anyone knows what is I would be overjoyed.

Also…I'm really trying to be a good sport and keep my head up, but I have to admit that the three months I've spent with Rust so far just haven't been pleasant on the whole. :anguished: I'm really starting to question if it was a good idea to write these bindings at all. It's taken far longer than I was expecting and I'm starting to think that it was a mistake not to just go ahead and keep writing my audio plugins in C or C++ like I have been. I was hoping that Rust would give me comparable speed to those languages with better error-checking, less boilerplate, and a more user-friendly atmosphere; so far I feel like it has delivered on the first two accounts but honestly been worse on the latter two, even if it really shines in certain places and seems like it has a lot of potential. I love the influence it takes from Ruby and Haskell, but I wish it took more; those languages both feel like they save me a lot of keystrokes and permit very readable code, whereas I often find myself writing lots of boilerplate to please the Rust compiler, even in cases where I feel like it should be able to infer what I'm telling it. That, and trying to interact with C code has been fraught with all kinds of surprising heavy problems in ways that have tended to be super-opaque, which I've been disappointed by since it's touted as having good C interop. I don't mind if a language is challenging or has a steep learning curve, I just want the learning process to be rewarding; so much of what I've learned about Rust is that things that I would guess are easy are actually hard and require quite wooly code, and things that you would think work just don't, sometimes in very nasty, UB-type ways. So, I kind of feel like I was happiest about it at the very beginning, and the more I've learned, the more disheartened I've felt. I feel like I should press on and finish these bindings because they seem nearly done and they fill an empty niche in the ecosystem, but it's getting to the point that I dread working on them because of how frustrated I anticipate feeling.

Don't give up! You just have started interacting with Rust through one of the most difficult and annoying ways possible, so I understand the feeling. Indeed, the whole point of Rust is to be using abstractions that guarantee safety, and abstract the implementation details (such as where exactly the memory is), whereas C does the opposite. Each side has its pros and cons, but by interacting with the two languages at the same time you mostly have to deal with the cons of both languages, which is a bummer. And far more difficult to do correctly than most people imagine.

Rust is making efforts to offer well-defined constructs that interact with this low-level stuff, such as:

  • Pin<Ptr>, when wanting to use unmoveable types (impl !Unpin);

  • MaybeUninit<_>, when wanting to deal with uninitialized memory;

  • UnsafeCell<_> when wanting to deal with aliased mutation.

but the ecosystem is not mature enough to make using those easier, so these three things are currently dangerously easy to get wrong :grimacing:


Luckily, thanks to places like this forum, we can help you get there! :slightly_smiling_face:

So, for this case, I'll ask that:

  • you tell us whether this whole thing is supposed to be multi-threaded or not (which will lead to having to design a Sync or !Sync abstractions, the latter being far simpler and/or faster)

  • you give us concrete code examples of idiomatic C-only usages of the library (with the C functions signatures)

With this, we'll be able to give you more concrete pointers (no pun intended) to design a Rust abstraction able to soundly wrap the C library. It will either require:

  • worst case scenario, an unsafe fn API, if the invariants of the C library are too complex to uphold. At which point the interest of using Rust is somewhat diminished (but not completely);

  • runtime-checks, that the C library did not initially have, but thanks to which we'll be able to offer a sound non-unsafe and ergonomic API (e.g., using RefCell instead of UnsafeCell, or Rc<_> and Weak<_> instead of Pin<Box<_>> and raw pointers);

  • or "weird" APIs, mainly callback-based (CPS), that by virtue of their restrictive design can enforce usage patterns that compile-time lifetimes alone cannot, and that would thus otherwise require "runtime" lifetimes / c.f. the previous point.

6 Likes

Luckily, thanks to places like this forum, we can help you get there! :slight_smile:

Thanks for the offer :smile_cat: This forum is definitely a saving grace of the language—I really appreciate how nice everyone has been so far.

Each side has its pros and cons, but by interacting with the two languages at the same time you mostly have to deal with the cons of both languages, which is a bummer. And far more difficult to do correctly than most people imagine.

Yeah—I do kind of wish that unsafe had a Rust subset that promised to play by C's rules, even just as a stopgap. The approach of introducing types like Pin and UnsafeCell to talk to low-level code in a highly structured way seems more attractive in the long run, but a fresh and unvarnished complex design is often more work to use than a simple and crude one I think, even if the complex design would save you work once it's more established.

There's a lot of places in Rust that feel that way to me in one way or another—I admire them for their ambition and big dreams, but they have a very "devil in the details" atmosphere one you start using them. I'm hoping that will diminish as the language keeps evolving.

So, for this case, I'll ask that:

  • you tell us whether this whole thing is supposed to be multi-threaded or not (which will lead to having to design a Sync or !Sync abstractions, the latter being far simpler and/or faster)

  • you give us concrete code examples of idiomatic C-only usages of the library (with the C functions signatures)

Okay. I'm going to go into detail—I'll try to focus only on the essentials, I just think there's a lot of essentials in this case. :stuck_out_tongue: Using real code samples, I'll show how the library is used in C, the approach I took in Rust that played nice with the C code, and the manner in which that approach breaks. I'll discuss threading as well.

The C library is called Pugl. Its two most significant structures are PuglWorld and PuglView. Here's how they're declared in Pugl's headers along with their instantiation functions (Pugl's code is ISC licensed, comments are as they appear in the original source):

/**
   The "world" of application state.

   The world represents things that are not associated with a particular view.
   Several worlds can be created in a process (which is the case when many
   plugins use Pugl, for example), but code using different worlds must be
   isolated so they are never mixed.  Views are strongly associated with the
   world they were created for.
*/
typedef struct PuglWorldImpl PuglWorld;

/**
   Create a new world.

   @return A newly created world.
*/
PUGL_API PuglWorld*
puglNewWorld(void);

/**
   @}
   @anchor view
   @name View
   A view is a drawing region that receives events, which may correspond to a
   top-level window or be embedded in some other window.
   @{
*/

/**
   A Pugl view.
*/
typedef struct PuglViewImpl PuglView;

/**
   Create a new view.

   A view represents a window, but a window will not be shown until configured
   with the various puglInit functions and shown with puglShowWindow().
*/
PUGL_API PuglView*
puglNewView(PuglWorld* world);

As you can see, their implementation details are hidden from the user, but here they are just for the sake of clarity:

/** Cross-platform world definition. */
struct PuglWorldImpl {
	PuglWorldInternals* impl;
	char*               className;
	double              startTime;
	size_t              numViews;
	PuglView**          views;
};

/** Cross-platform view definition. */
struct PuglViewImpl {
	PuglWorld*         world;
	const PuglBackend* backend;
	PuglInternals*     impl;
	PuglHandle         handle;
	PuglEventFunc      eventFunc;
	char*              title;
	PuglBlob           clipboard;
	PuglNativeWindow   parent;
	uintptr_t          transientParent;
	PuglHints          hints;
	PuglRect           frame;
	int                minWidth;
	int                minHeight;
	int                minAspectX;
	int                minAspectY;
	int                maxAspectX;
	int                maxAspectY;
	bool               visible;
};

The PuglView documentation is being a bit glib in describing it as representing a window, because it doesn't necessarily represent a platform window. It can, but it can also represent a drawing context within a platform window; multiple PuglViews can be parented to the same platform window at once and displayed simultaneously. This is a screenshot from one of Pugl's application tests:

That's one platform window displaying two PuglViews. One view is the grey rectangle with the cube inside. The other is the gradient, which is partially concealed by the view with the cube. So, views can be drawn on top of each other, moved around, etc. within a single platform window.

Here's a very simple usage example in C that hits on all the main points. It's the simplest application test in Pugl's repo. All it does is display a blank window and print a description of the events it registers to the console. I've added comments describing what's happening.

/*
 *  These are utility functions for Pugl's tests that don't make
 *  up part of its public API. The only function being used from
 *  it here is printEvent.
 */
#include "test_utils.h"

#include "pugl/pugl.h"

/*
 *  Pugl supports two backends, OpenGL and Cairo. This header
 *  presents a stubbed-out backend that only pretends to create a
 *  drawing context.
 */
#include "pugl/pugl_stub.h"

#include <stdbool.h>
#include <stdio.h>

/*
 *  Views can hold a void pointer to a user-defined struct that
 *  Pugl calls a PuglHandle. An instance of this struct can then
 *  be retrieved from the view by calling puglGetHandle(view) and
 *  casting the result to the appropriate type. This is used in
 *  event callbacks, as we'll see.
 *
 *  You can put whatever you want in this struct. In Pugl's
 *  application tests, it typically holds pointers to the world
 *  and all of the views, so views have access to the entire
 *  application state at any time. I found this level of access
 *  challenging to preserve in Rust, and opted instead for a
 *  design where views only have access to the handle in
 *  callbacks and use it to pass messages to other parts of the
 *  application indirectly.
 */
typedef struct
{
	PuglWorld* world;
	PuglView*  view;
	int        quit;
} PuglPrintEventsApp;

/*
 *  This is the view's event callback, as you would probably
 *  guess. Each view can have its own callback. The approach
 *  taken here is typical: the handle is retrieved from the view,
 *  events are handled in a switch statement, and a status is
 *  returned.
 *
 *  Event callbacks have this type:
 *
 *  PuglStatus (*PuglEventFunc)(PuglView* view, const PuglEvent* event)
 *
 *  This is significant because it constrains what Pugl makes
 *  available in a callback. All you get is a pointer to the view
 *  and a const pointer to the event, so anything else you need
 *  you have to make available in the handle. This leads to one
 *  of the thorniest problems in the high-level bindings: going
 *  from the pointer to the C-level PuglView back to the
 *  Rust-level View when the callback is called.
 */
static PuglStatus
onEvent(PuglView* view, const PuglEvent* event)
{
	PuglPrintEventsApp* app = (PuglPrintEventsApp*)puglGetHandle(view);

	printEvent(event, "Event: ", true);

	switch (event->type) {
	case PUGL_CLOSE: app->quit = 1; break;
	default: break;
	}

	return PUGL_SUCCESS;
}

int
main(void)
{
	PuglPrintEventsApp app = {NULL, NULL, 0};

	/*
	 *  Views hold a pointer to their associated world which can
	 *  be retrieved with puglGetWorld(*view).
	 */
	app.world = puglNewWorld();
	app.view  = puglNewView(app.world);

	puglSetClassName(app.world, "Pugl Print Events");
	puglSetBackend(app.view, puglStubBackend());
	puglSetHandle(app.view, &app);
	puglSetEventFunc(app.view, onEvent);

	/*
	 *  Creating and showing the window are different. Creating
	 *  it sets up the right context with the underlying
	 *  platform, whereas showing it makes the window visible to
	 *  the user. Creation must be done first and is where most
	 *  of the heavy lifting happens.
	 */
	if (puglCreateWindow(app.view, "Pugl Event Printer")) {
		return logError("Failed to create window\n");
	}

	puglShowWindow(app.view);

	/*
	 *  Here's the main loop. By design, Pugl doesn't force a
	 *  loop on you or want to take full control of the process,
	 *  so you don't have to do things this way. It's the easiest
	 *  approach if you're using Pugl on its own, though.
	 *
	 *  You can either block the thread and wait for events, or
	 *  you can dispatch events continuously; it's your choice.
	 *  This application waits for them. The -1 argument to
	 *  puglPollEvents tells it to wait forever; a positive
	 *  number tells it how many seconds to wait before
	 *  returning. If you want to dispatch events continuously,
	 *  you call puglPostRedisplay(view) in the main loop to
	 *  request a redraw instead of puglPollEvents. If using
	 *  puglPollEvents, calls to puglPostRedisplay can be made in
	 *  the view's event callback if the event in question merits
	 *  a redraw.
	 */
	while (!app.quit) {
		puglPollEvents(app.world, -1);
		puglDispatchEvents(app.world);
	}

	/*
	 *  Since these are opaque structs from the user's standpoint,
	 *  Pugl makes teardown functions available for the world and
	 *  views. As they traverse much of the internals of these
	 *  structs and also try to free things, they are a common
	 *  source of memory corruption errors if Rust has moved
	 *  anything around behind Pugl's back.
	 */
	puglFreeView(app.view);
	puglFreeWorld(app.world);

	return 0;
}

Here's the approach I took in Rust that resulted in stable interactions with Pugl. Unfortunately, it doesn't get along with idiomatic Rust code, which is the problem I'm facing now. I'll show the main types and their most significant methods, with commentary:

// World

pub struct World {
    inner: *mut PuglWorld,
    class_name: &'static str,
}

impl Drop for World {
    fn drop(&mut self) {
        unsafe { puglFreeWorld(self.inner); }
    }
}

impl World {
    pub fn new() -> World {
        World {
            inner: unsafe { puglNewWorld() },
            class_name: "",
        }
    }

    // Views are created through the World's interface. This
    // function is generic over the user's handle, which is wrapped
    // in a Handle struct. More on that in a sec.
    pub fn new_view<T>(&self) -> View {
        View::new::<Handle<T>>(self.inner)
    }

    #[must_use]
    pub fn poll_events(&self, timeout: f64) -> Result<Status,Status> {
        Status::wrap_c_in_result(unsafe {
            puglPollEvents(self.inner, timeout)
        })
    }

    #[must_use]
    pub fn dispatch_events(&self) -> Result<Status,Status> {
        Status::wrap_c_in_result(unsafe {
            puglDispatchEvents(self.inner)
        })
    }

    //...
}

// Handle

// This is a wrapper struct for the user's handle—the one that can
// be retrieved from views in their event callbacks. Its internals
// are kept private because it does some hacky stuff to relate
// C-level PuglViews with their Rust-level Views, a relation which
// is set up internally during view instantiation. This is done
// with a ViewMap, which is coming in a moment.
pub struct Handle<T> {
    view_map: ViewMap,
    pub inner: T
}

impl<T> Handle<T> {
    pub fn new(inner: T) -> Handle<T> {
        Handle {
            view_map: ViewMap::new(),
            inner,
        }
    }
}

// Handles deref to their inner type so that users don't have to
// worry about them after instantiation.

impl<T> Deref for Handle<T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        &self.inner
    }
}

impl<T> DerefMut for Handle<T> {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.inner
    }
}

// ViewMap

// A mapping between C-level PuglView addresses and raw pointers to
// Rust-level Views. The Rust view pointers are stored as *const,
// but converted to *mut and then &mut before being given to the
// user. This happens internally during event callback setup, so
// it's invisible to the user, which we'll see shortly. Obviously,
// this is not a very Rust-friendly design, and is nastily brittle
// if Rust starts moving PuglViews or Views around.
struct ViewMap {
    map: HashMap<usize, *const View>,
}

impl ViewMap {
    fn new() -> ViewMap {
        ViewMap { map: HashMap::new() }
    }

    fn set(&mut self, pugl_view: *const PuglView, view: *const View) {
        self.map.insert(pugl_view as usize, view);
    }

    fn get(&self, pugl_view: *const PuglView) -> *mut View {
        match self.map.get(&(pugl_view as usize)) {
            Some(view) => *view as *mut View,
            None => panic!(
                "C-level view ptr {} has no associated view in map",
                pugl_view as usize
            ),
        }
    }
}

// event callback

pub type EventFunc = fn(&mut View, Event) -> Status;

// View

pub struct View {
    inner: *mut PuglView,
    backend: Backend,
    event_func: EventFunc,
}

impl Drop for View {
    fn drop(&mut self) {
        unsafe { puglFreeView(self.inner); }
    }
}

impl View {
    // Views are initialized with a stubbed-out backend and event
    // callback in case the user doesn't need one or the other.
    // This can be changed later. Of note, this function is
    // private; you may recall that views are instantiated through
    // World's interface, both to associate them with the world and
    // to perform handle-related setup.
    fn new<T>(world: *mut PuglWorld) -> View {
        let inner = unsafe { puglNewView(world) };
        let status = Status::from_c(unsafe {
            puglSetBackend(inner, puglStubBackend())
        });
        Self::check_property_init_status(status, "stub backend");
        let status = Status::from_c(unsafe {
            puglSetEventFunc(inner, Some(Self::event_func_wrapper::<T>))
        });
        Self::check_property_init_status(status, "event func wrapper");
        View {
            inner,
            backend: Backend::Stub,
            event_func: Self::event_func_stub,
        }
    }

    // After creating a Handle, you can attach it to a View using
    // this method. Under the hood, it tracks an assocation between
    // the View and its inner PuglView in its ViewMap, then attaches
    // a pointer to the Rust-level Handle to the C-level PuglView.
    pub fn set_handle<T>(&self, handle: &mut Handle<T>) {
        handle.view_map.set(self.inner, self as *const View);
        let handle_ptr: *mut Handle<T> = handle;
        unsafe {
            puglSetHandle(self.inner, handle_ptr as PuglHandle);
        }
    }

    pub fn get_handle<T>(&self) -> &mut Handle<T> {
        unsafe {
            let handle_ptr = puglGetHandle(self.inner) as *mut Handle<T>;
            &mut*handle_ptr
        }
    }

    // This is what gets registered as the event callback on the
    // C-level PuglView. It gets the Rust-level Handle back out of
    // the PuglView, then uses the address of the PuglView to look
    // up the Rust-level View in the Handle's ViewMap. After that,
    // it calls the Rust-level View's callback, passing in the
    // Rust-level View and a Rustified Event.
    unsafe extern "C" fn event_func_wrapper<T>(
        c_view: *mut PuglView,
        c_event: *const PuglEvent
    ) -> PuglStatus {
        let view_ptr: *mut View = {
            let handle_ptr = puglGetHandle(c_view) as *mut Handle<T>;
            let handle = handle_ptr.as_mut().unwrap();
            handle.view_map.get(c_view)
        };
        let view = view_ptr.as_mut().unwrap();
        let callback = view.event_func;
        let event = Event::from_c(*(c_event.as_ref().unwrap()));
        callback(view, event) as u32
    }

    //...
}

// Timer

// The only thing that Pugl's views do with their world in
// callbacks is use it to access a monotonically-increasing clock
// through a const reference. This is provided as a convenience
// struct that users can store in their Handle instead of dealing
// with a shared reference to the World. It stores a pointer to a
// PuglWorld and knows how to ask it for the time.
pub struct Timer {
    world: *const PuglWorld,
}

impl Timer {
    pub fn new(world: &World) -> Timer {
        Timer { world: world.inner }
    }

    pub fn get_time(&self) -> f64 {
        unsafe { puglGetTime(self.world) }
    }
}

I think that's the essentials from my bindings. Here's my version of the event printing program from earlier. I imagine it's self-explanatory at this point:

use pugl_rs::*;
use pugl_rs::events::Event;

struct App {
    quit: bool,
    timer: Timer,
}

fn on_event(view: &mut View, event: Event) -> Status {
    let app: &mut App = view.get_handle();
    println!("Time: {}", app.timer.get_time());
    println!("Event: {:#?}", event);

    match event {
        Event::Close(_) => app.quit = true,
        _ => (),
    }

    Status::Success
}

fn main() {
    let mut world = World::new();
    let mut view = world.new_view::<App>();
    let mut app = Handle::new(App {
        timer: Timer::new(&world),
        quit: false
    });

    world.set_class_name("Pugl Print Events").unwrap();
    view.set_backend(Backend::OpenGL).unwrap();
    view.set_handle(&mut app);
    view.set_event_func(on_event);

    view.create_window("event printer").unwrap();
    view.show_window().unwrap();

    while !app.quit {
        world.poll_events(-1.0).unwrap();
        world.dispatch_events().unwrap();
    }
}

I've written much more complex examples than this against both the OpenGL and Cairo backends and haven't encountered any trouble with this design in isolation. However, problems emerge if you try to pass smart pointers to Worlds or Views around, store them in enums, etc., because Rust starts moving things around in memory in ways that invalidate both the references being stored internally in the Rust structs as well as the references stored in Pugl's C-level structs. At an earlier point, I was using Rust's smart pointers instead of all these plain wrappers around raw pointers, but moved to this design to get around that issue. I've been experimenting with std::pin as a way to more gracefully handle Rust's desire to move things, but have yet to find a design that works.

As far as threading goes, it's a bit thorny and I'm not sure there's a clear right answer. Pugl's docs make no mention one way or another about thread safety and much of its code is not inherently thread-safe. I've also found that PuglWorld instantiation crashes if it's not done serially, at least on X11. These things being said, I think Pugl gives the user enough control that you could build a thread-safe interface to it if you were careful—it doesn't really do anything you don't tell it to directly.

Plugin standards vary somewhat. LV2 does not require plugin UIs to be threadsafe. VST2 and VST3 seem to leave threading more-or-less entirely up to the host, but I think hosts generally don't try to talk to a single plugin's UI from multiple threads simultaneously. In strict terms, though, no assumptions can be made about that if you're shipping a VST. I can't find any information about the threading model for AAX and RTAS if one has ever been explicitly articulated, so same thing goes in that case I guess.

Based on all this, I can see two arguments. One of them goes that Pugl isn't thread-safe, so my bindings shouldn't bother with that either, and hosts should know better than to assume that plugins have a thread-safe UI. The other says that hosts are unpredictable, so better to err on the side of caution and make the bindings thread-safe. My inclination is to go with the first option for the time being and not worry about thread safety, then make some plugins with it and test them in a variety of hosts to see if they're stable. If they misbehave and threading issues seem to be the culprit, that might be a good time to take on the extra burden of thread safety. Even aside from the programming difficulties, performance is paramount in audio plugin development, even for the UI—the UI needs to be responsive enough for realtime use and cycles can be at a premium with all the different stuff going on in a DAW, so I'd rather prioritize speed if I have the option.

1 Like

Thanks for your detailed presentation, it is now far easier to help you.

So first of all, here is a Playground link to toy with Pugl's raw API:

use pugl::*;

#[allow(unused)]
struct PrintEventsApp {
    world: *const PuglWorld,
    view: *const PuglView,
    quit: Cell<bool>,
}

unsafe extern "C"
fn on_event (view: *mut PuglView, event: *const PuglEvent)
  -> PuglStatus::T
{
    abort_on_unwind!();

    let app =
        puglGetHandle(view)
        .cast::<PrintEventsApp>()
        .as_ref()
        .expect("Got NULL Handle")
    ;
    let event_type = *(& /*raw*/ (*event).type);
    println!("Callback called on: {}", event_type);
    match event_type {
        | PuglEventType::CLOSE => app.quit.set(true),
        | _ => {},
    }
    PuglStatus::SUCCESS
}

fn main ()
{ unsafe {
    let world = puglNewWorld();
    let view = puglNewView(world);
    let app = PrintEventsApp {
        world,
        view,
        quit: Cell::new(false),
    };
    puglSetHandle(view, &app as *const _ as _);
    puglSetEventFunc(view, Some(on_event));
    while app.quit.not() { // opinionated stance against the `!` sigil
        puglPollEvents(world, -1.);
        puglDispatchEvents(world);
    }
    puglFreeView(view);
    puglFreeWorld(world);
}}

This will help us test and discuss about possible designs.


What the API shows, is that there are indeed many back-references, resulting in:

  • less trivial ownership;

    • By aliasing, I mean aliased mutation, i.e., mutating something despite there being multiple (active) pointers to it.

Because of that, I think there are two sensible choices to make since the beginning:

  1. Drop multi-threaded support, and thus make all our abstractions !Sync. And since we do not know if pugl is using thread-local globals, we'll also need to ensure that is is !Send too, for guaranteed soundness.

  2. Do not use &mut-based mutation nor &mut references in general anywhere. Indeed, those assume the lack of aliasing, which is clearly not the case here.

    Given 1., the most sensible choice here is to thus rely on Cell and RefCell. And given that incorrect usage of RefCell can lead to panic!s, which are not good in presence of extern "C" functions (such as with the callbacks provided), I will be mainly (ab)using Cell. This is usually a very right choice when interfacing with C APIs, since C's non-NULL *mut _ pointer has the semantics of Rust's &Cell<_>.

    This can already be seen with the App definition, which expects to mutate the quit field. We thus wrap in a Cell, thus always dealing with &App and never with &mut App.


Now, the real challenge, as you mentioned, is that all these active pointers mean that:

  • either the different structs are always borrowed, meaning they cannot move; not even be returned from a function!

  • or the different structs ought to be !Unpin, and then be Pin-ned, either through a heap Rc-allocation (negligeable cost w.r.t. Box, given that we won't be exploiting &mut-ness anywhere, and it will let us have ref-counted owned pointer copies (Rc::clone), and ref-counted non-owned-and-thus-potentially-stale pointer copies (Rc::downgrade)), or through stack-pinning in some particular cases.

    • Note that the Pin abstraction will only work with back-references to clear the references on Drop.

To be continued...

2 Likes

Here is a version using ref-counted references to ensure no pointer gets to gangle (e.g., because someone moved the memory). As you see, no Pin was used, just rc::{Rc, Weak} with Cell for the mutation:

mod lib {
    mod c_pugl { ... }
    use c_pugl::*;

    use ::std::{
        cell::Cell,
        mem,
        ptr,
        rc::{self, Rc},
    };

    type PhantomLifetime<'lt> = ::core::marker::PhantomData<&'lt ()>;

    #[derive(Debug)]
    pub enum Error {
        Failure,              //< Non-fatal failure
        UnknownError,         //< Unknown system error
        BadBackend,           //< Invalid or missing backend
        BackendFailed,        //< Backend initialisation failed
        RegistrationFailed,   //< Window class registration failed
        CreateWindowFailed,   //< Window creation failed
        SetFormatFailed,      //< Failed to set pixel format
        CreateContextFailed,  //< Failed to create drawing context
        UnsupportedType,      //< Unsupported data type
    }
    pub type Result<T> = ::core::result::Result<T, Error>;

    fn result_of_pugl_status (status: PuglStatus::T) -> Result<()>
    {
        Err(match status {
            | PuglStatus::SUCCESS => return Ok(()),
            | PuglStatus::FAILURE => Error::Failure,
            | PuglStatus::UNKNOWN_ERROR => Error::UnknownError,
            | PuglStatus::BAD_BACKEND => Error::BadBackend,
            | PuglStatus::BACKEND_FAILED => Error::BackendFailed,
            | PuglStatus::REGISTRATION_FAILED => Error::RegistrationFailed,
            | PuglStatus::CREATE_WINDOW_FAILED => Error::CreateWindowFailed,
            | PuglStatus::SET_FORMAT_FAILED => Error::SetFormatFailed,
            | PuglStatus::CREATE_CONTEXT_FAILED => Error::CreateContextFailed,
            | PuglStatus::UNSUPPORTED_TYPE => Error::UnsupportedType,
            | _ => panic!("Got invalid {:?}", status),
        })
    }
    fn pugl_status_of_result (res: Result<()>) -> PuglStatus::T
    {
        match res {
            | Ok(()) => PuglStatus::SUCCESS,
            | Err(Error::Failure) => PuglStatus::FAILURE,
            | Err(Error::UnknownError) => PuglStatus::UNKNOWN_ERROR,
            | Err(Error::BadBackend) => PuglStatus::BAD_BACKEND,
            | Err(Error::BackendFailed) => PuglStatus::BACKEND_FAILED,
            | Err(Error::RegistrationFailed) => PuglStatus::REGISTRATION_FAILED,
            | Err(Error::CreateWindowFailed) => PuglStatus::CREATE_WINDOW_FAILED,
            | Err(Error::SetFormatFailed) => PuglStatus::SET_FORMAT_FAILED,
            | Err(Error::CreateContextFailed) => PuglStatus::CREATE_CONTEXT_FAILED,
            | Err(Error::UnsupportedType) => PuglStatus::UNSUPPORTED_TYPE,
        }
    }

    #[derive(Debug, Clone)]
    pub
    struct World /* = */ (
        Rc<WorldInner>,
    );

    #[derive(Debug)]
    struct WorldInner {
        pugl: ptr::NonNull<PuglWorld>,
    }

    impl WorldInner {
        pub
        fn new () -> Option<Self>
        {
            Some(Self {
                pugl: ptr::NonNull::new(unsafe { puglNewWorld() })?,
            })
        }
    }

    impl Drop for WorldInner {
        fn drop (self: &'_ mut Self)
        {
            unsafe { puglFreeWorld(self.pugl.as_ptr()) }
        }
    }

    impl World {
        pub
        fn new () -> Option<Self>
        {
            WorldInner::new().map(|it| Self(Rc::new(it)))
        }

        pub
        fn poll_events (self: &'_ Self, timeout: ::libc::c_double)
        {
            unsafe {
                puglPollEvents(self.0.pugl.as_ptr(), timeout)
            }
        }

        pub
        fn dispatch_events (self: &'_ Self)
          -> Result<()>
        {
            result_of_pugl_status(unsafe {
                puglDispatchEvents(self.0.pugl.as_ptr())
            })
        }
    }

    struct ViewInner {
        pugl: ptr::NonNull<PuglView>,
        _world: World, // owning pointer to ensure World is kept alive
        event_handler: Cell< // mutable
            Option< // nullable
                Box<dyn // owned trait object
                    // with a callable API
                    Fn(&'_ View, Event<'_>) -> Result<()>
                >
            >
        >,
    }
    /* >> The PuglView's handle will be a (weak) reference to this `ViewInner` << */

    #[derive(Clone)]
    pub
    struct View /* = */ (
        Rc<ViewInner>,
    );

    impl View {
        pub
        fn new (world: World) -> Option<View>
        {
            let view = Rc::new(ViewInner {
                pugl: ptr::NonNull::new(
                    unsafe { puglNewView(world.0.pugl.as_ptr()) }
                )?,
                _world: world,
                event_handler: Cell::new(None),
            });
            unsafe {
                puglSetHandle(
                    view.pugl.as_ptr(),
                    // hack while waiting for rc::Weak::into_raw to stabilize
                    mem::transmute(Rc::downgrade(&view)),
                )
            };
            Some(View(view))
        }

        pub
        fn on_event (
            self: &'_ Self,
            event_handler: impl 'static + Fn(&'_ View, Event<'_>) -> Result<()>,
        )
        {
            unsafe {
                self.0.event_handler.set(Some(Box::new(event_handler)));
                puglSetEventFunc(self.0.pugl.as_ptr(), {
                    unsafe extern "C" fn on_event (
                        view: *mut PuglView,
                        event: *const PuglEvent,
                    ) -> PuglStatus::T
                    {
                        ::scopeguard::defer_on_unwind!({
                            eprintln!(concat!(
                                "Caught Rust attempting to unwind across an extern \"C\" function",
                                ", which is Undefined Behavior.\n",
                                "Aborting to prevent unsoundness",
                            ));
                            ::std::process::abort();
                        });
                        macro_rules! unwrap {($expr:expr) => (
                            if let Some(it) = $expr { it } else {
                                return PuglStatus::FAILURE;
                            }
                        )}
                        let event = Event(
                            unwrap!(ptr::NonNull::new(event as _)),
                            PhantomLifetime::default(),
                        );
                        let view = unwrap!(view.as_ref());
                        let ref view: View = View(unwrap!(
                            mem::ManuallyDrop::new( // avoid dropping the weak count
                                mem::transmute::<_, rc::Weak<ViewInner>>( // Rc::Weak::from_raw()
                                    puglGetHandle(view)
                                )
                            ).upgrade()
                        ));
                        let event_handler = view.0.event_handler.replace(None);
                        if let Some(event_handler) = event_handler {
                            let ret = pugl_status_of_result(event_handler(view, event));
                            view.0.event_handler.set(Some(event_handler));
                            ret
                        } else {
                            PuglStatus::SUCCESS
                        }
                    }
                    Some(on_event)
                });
            }
        }
    }
    impl Drop for ViewInner {
        fn drop (self: &'_ mut Self)
        {
            let pugl_view: *mut PuglView = self.pugl.as_ptr();
            unsafe {
                drop::<rc::Weak<Self>>(mem::transmute(
                    puglGetHandle(pugl_view)
                ));
                puglFreeView(pugl_view);
            }
        }
    }

    #[derive(Clone, Copy)]
    pub
    struct Event<'event> /* = */ (
        ptr::NonNull<PuglEvent>,
        PhantomLifetime<'event>,
    );

    #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
    pub
    enum EventType {
        /// No event
        Nothing,
        /// Mouse button press
        ButtonPress,
        /// Mouse button release
        ButtonRelease,
        /// View moved and/or resized
        Configure,
        /// View exposed, redraw required
        Expose,
        /// Close view
        Close,
        /// Key press
        KeyPress,
        /// Key release
        KeyRelease,
        /// Character entry
        Text,
        /// Pointer entered view
        EnterNotify,
        /// Pointer left view
        LeaveNotify,
        /// Pointer motion
        MotionNotify,
        /// Scroll
        Scroll,
        /// Keyboard focus entered view
        FocusIn,
        /// Keyboard focus left view
        FocusOut,
    }

    impl Event<'_> {
        pub
        fn get_type (self: Self) -> EventType
        {
            match *unsafe { & /* raw */ (*self.0.as_ptr()).r#type } {
                | PuglEventType::NOTHING => EventType::Nothing,
                | PuglEventType::BUTTON_PRESS => EventType::ButtonPress,
                | PuglEventType::BUTTON_RELEASE => EventType::ButtonRelease,
                | PuglEventType::CONFIGURE => EventType::Configure,
                | PuglEventType::EXPOSE => EventType::Expose,
                | PuglEventType::CLOSE => EventType::Close,
                | PuglEventType::KEY_PRESS => EventType::KeyPress,
                | PuglEventType::KEY_RELEASE => EventType::KeyRelease,
                | PuglEventType::TEXT => EventType::Text,
                | PuglEventType::ENTER_NOTIFY => EventType::EnterNotify,
                | PuglEventType::LEAVE_NOTIFY => EventType::LeaveNotify,
                | PuglEventType::MOTION_NOTIFY => EventType::MotionNotify,
                | PuglEventType::SCROLL => EventType::Scroll,
                | PuglEventType::FOCUS_IN => EventType::FocusIn,
                | PuglEventType::FOCUS_OUT => EventType::FocusOut,
                | event_type => panic!("Got invalid {:?}", event_type),
            }
        }
    }
}
use lib as pugl;

use ::std::{
    cell::Cell,
    ops::Not,
    rc::Rc,
};

struct PrintEventsApp {
    quit: Cell<bool>,
}

fn main ()
{
    let world = pugl::World::new().expect("Got NULL");
    let view = pugl::View::new(world.clone()).expect("Got NULL");
    let ref app = Rc::new(PrintEventsApp {
        quit: Cell::new(false),
    });
    view.on_event({
        let app = Rc::clone(app);
        move |_view: &_, event: pugl::Event<'_>| Ok({
            if dbg!(event.get_type()) == pugl::EventType::Close {
                app.quit.set(true);
            }
        })
    });
    while app.quit.get().not() {
        world.poll_events(-1.);
        world.dispatch_events().unwrap();
    }
}
3 Likes

Thanks so much for your help! I've been working on adapting my bindings to the guidelines you laid out. In the course of doing that, I've ended up with a question I'm having a hand time finding answers to—if you know the answer and would we willing to explain, I'd be very appreciative. Basically, is it necessary to wrap *mut pointers in something like Rc<Cell<_>> if there's aliased mutation to the value going on on the C side, even if there's a single owner on the Rust side?

For instance, in this case, a C-side PuglWorld holds a list of pointers to all the PuglViews associated with it, but a pointer to one of those views is also passed into its event callback. Mutation is allowed through both of these pointers. In Rust terms, that seems like two mut references existing at once, which would tend to be wrapped in a combination of a shared-reference smart pointer and a cell of some sort to prevent UB. How far do I need to go to accommodate a pattern like this in Rust if I know it exists in C? If I acquire a *mut pointer to the same PuglView on the Rust side, does that automatically open me up to UB simply by holding onto it somewhere? Am I protected if I immediately wrap the pointer inside of an Rc<Cell<_>> or the like after acquiring it, or is the mere existence of it at any point in my Rust code hazardous? If a single pointer to something non-const exists on the C side and I acquire a *mut pointer to it on the Rust side, does that count as two mut references?

On a related note, I see in your example that you hold references to PuglViews and PuglWorlds inside of Cell<_> in your mock implementation of Pugl's API. Is that just a memory-safe way of mocking up what Pugl is doing in C, or is that how you would actually write low-level bindings to Pugl? Does Rust automatically coerce a cell into a raw pointer when passing it across the FFI? On the other hand, if I get a raw pointer from C, how am I supposed to get it inside a Cell? I know how to make a smart pointer from it, but Cell doesn't provide a from_ptr method or anything, only a from_mut method—and you recommended avoiding &mut references. If I dereference the C pointer, store the value in a Cell, then starting passing pointers to that value from the Cell back to C, I worry it would invalidate older C-side pointers to that value.

In a larger sense, do you know of a resource where these kinds of questions are answered systematically? The Book says, "...other languages don’t enforce Rust’s rules and guarantees, and Rust can’t check them, so responsibility falls on the programmer to ensure safety," but it doesn't go into detail about what would result in errors or UB in Rust from the perspective of another language. The Nomicon has some useful tips and recipes, but I don't see a systematic discussion of these issues there either. The reference has a list of undefined behavior, but it's all in Rust terms; all they say about C is that UB in C is UB in Rust, but clearly there's well-defined behavior in C that can also result in UB in Rust. To make matters worse, they also note the list is unexhaustive. There's the Unsafe Code Guidelines, but they seem very much like a work in progress—important parts like a precise definition of interior mutability are still up-in-the-air. They do say, "Finding live shared references propagates recursively through references, but not through raw pointers," which would make me think that the things I've been doing with raw pointers might be fine as long as I don't turn them into references, but it's hard to know the specifics just from a sentence like that—and it gets more complex when things like smart pointers and cells are introduced, because I'm not sure to what extent those count as references in this particular context compared to & and &mut. It would be really nice if there was some resource I could consult to learn about this stuff so that I wouldn't have to bug people on a forum about it—does one exist? Maybe I'm not reading the existing resources closely enough?

1 Like

The Cell / RefCell (when single-threaded), or Atomic{Integer} / Mutex / RwLock wrapping is almost always necessary, at least as a sane default.

This is because there are only two ways to soundly do aliased (but non-simultaneous!) mutation in Rust:

  • either through a Rust reference (which needs to be a shared one: &, because of the aliasing / sharing), and then the value being mutated needs to be wrapped in a special (for the compiler) wrapper: UnsafeCell (it is the magical wrapper that makes it legal for mutation to happen over some T even when there are being multiple &'_ UnsafeCell<T> around). All the wrappers I have mentioned above use UnsafeCell internally.

    • UnsafeCell can be used directly, but so doing is, as its name indicates, unsafe. I'd even say that it is more dangerous than just using raw pointers to begin with.
  • or by always remaining at the "raw pointer" (*mut T) level, which means avoiding to ever create a Rust reference.

(You can look at this blog post of mine for more info about this)

Note, however, that the smart pointer plays no role w.r.t. mutation. The only (but paramount) thing that Rc does is preventing the pointers from ever dangling and causing UB (c.f., your "structs were moved around and it broke things" case).

One of the first things that we could do with my working example, for instance, is to try and replace the Rc part with borrows and lifetimes (to move the anti-dangling checks to compile-time), although so doing will surely lead to a more cumbersome API

No, it does not, given that it is a raw pointer. If you were, however, to go and upgrade that *mut PuglView to a &mut PuglView, then that could be UB.

Everything private to the mock_c_lib module (body of functions, non-exported struct definitions) is indeed just a (hopefully) UB-free way of mocking what C would do, so besides curiosity, it should not serve as a model. The exported contents of c_pugl, however, correspond to my manually writing what an automatic tool such as bindgen would have generated for this C API (I think). Seeing it well formatted and with helper macros should improve the readability: I think it is important to know / understand what bindgen generates.

You can upgrade (by transmuting) a *mut T from C to a Option< &UnsafeCell<T> > (i.e., a nullable-but-well-aligned aliased pointer to an area of memory that may be mutated by one of these pointers aliasing it). If you know it is not NULL, this gives us &UnsafeCell<T>. Using that reference, however, relies on the properties that C gives you.

If, for instance, you know that the C code is not multi-threaded, you could go and upgrade (by transmuting) that &UnsafeCell to a &Cell, and use .get() and .set() to read/write directly that T (usually that T will be an integer).

  • if it is a more complex struct, you can have Cells wrapping each "field": &StructWithCellFields, which is what I have done in your pugl example with, for instance, PrintEventsApp.

If the C API guarantees that within a given pattern, the other aliasing raw pointers / &UnsafeCell<T> references will not be used (e.g., the C library offers a manual locking mechanism, or there is simply no concurrency / reentrancy going on), you can even go and upgrade that &UnsafeCell<T> to a &mut T, and use it as you please. Since the C library is rarely that ill-designed / ill-intentioned, this guarantee often holds and is the reason why many people that upgrade a *mut T to a &mut T without thinking about the consequences get along just fine.

I think this is best seen with an example.

Imagine a C library offering the following API:

// always returns a valid (non-NULL, aligned, writeable ...) pointer
uint8_t * get_pointer (void);

// Does not spawn any threads
void do_stuff (void);

// i.e.,

extern "C" {
    fn get_pointer () -> *mut u8;
    fn do_stuff ();
}

Now imagine that we want to get the pointer, clear it, call do_stuff(), and then read-dereference that pointer again (the only reason we may do this is that we suspect that pointer to be stored in some C global that do_stuff will use).

If you go and do:

fn foo () -> u8
{ unsafe {
    let p: &mut u8 = &mut *get_pointer();
    *p = 0;
    loop {
        do_stuff();
        if *p != 0 { return *p; }
    }
}}

then this function is allowed to never return, independently of the behavior of do_stuff. If do_stuff does not overwrite the value pointed to by get_pointer(), then the infinite loop is expected and all is fine, although given the body of the function we can imagine that the author expected otherwise.

If, however, do_stuff() ever reads or writes to the value pointed to by get_pointer() (as I said, thanks to a global), then this code is UB: indeed, given the exclusivity that &mut u8 expresses, Rust will assume that do_stuff cannot interact with the value p points to, so it may:

  • change the body of the loop to loop { do_stuff(); } ("given the exclusivity of p, the value it points to cannot possibly have changed, --Rust is allowed to think-- thus we know its value is the same as the one we wrote, that is, 0").

  • at which point it could maybe even remove the *p = 0 write altogether, since by virtue of looping indefinitely with an exclusive pointer, there is no way no one could see the difference.

This example also illustrate something else: the problem with aliasing is not exclusive (pun intended) to *mut T. If get_pointer() returned a uint8_t const *, and if we had written

fn foo () -> u8
{ unsafe {
    let p: &u8 = &*get_pointer();
    while *p == 0 {
        do_stuff();
    }
    *p
}}

we would have had the same issue: Rust could optimize that into

fn foo () -> u8
{ unsafe {
    let p: &u8 = &*get_pointer();
    if *p != 0 { return *p; }
    loop {
        do_stuff();
    }
}}

To prevent such optimizations, UnsafeCell is the wrapper that expresses that other code may mutate something despite there being some Rust reference to an UnsafeCell containing that thing:

fn foo () -> u8
{ unsafe {
    let p: &Cell<u8> = mem::transmute(get_pointer());
    p.set(0);
    loop {
        do_stuff();
        if p.get() != 0 { return p.get(); }
    }
}}

or we just avoid the Rust references and keep using raw pointers (since they only express properties at usage site, contrary to Rust references' "existence spans").

fn foo () -> u8
{ unsafe {
    let p: *mut u8 = get_pointer();
    p.write(0);
    loop {
        do_stuff();
        if *p != 0 { return *p; }
    }
}}

That part of the Rust ecosystem is not mature yet (we mainly need for the Unsafe Code Working Group to come up with an official reference of what properties the Rust abstract machine relies on, and given how it will not be possible to backtrack the moment they so do, it takes time to try to think of all the possible future compiler optimizations they prevent if they define too much behavior).

I personally plan on working on developing an FFI / safer unsafe framework with guides and libraries about all this, but that also requires time :sweat_smile:

1 Like