callbacks are unsafe due to panicking

[eeeeeta]

Currently, rust-jack does not catch panics at the FFI boundary, resulting in possible unsafety / undefined behaviour.

(c.f. the docs for std::panic::catch_unwind:)

It is currently undefined behavior to unwind from Rust code into foreign code, so this function is particularly useful when Rust is called from another language (normally C). This can run arbitrary Rust code, capturing a panic and allowing a graceful handling of the error.

[wmedrano]

Makes sense, to make safe, I want to ensure

1. Low runtime impact for normal callbacks
2. Allow for debugging of code that would panic
3. Define behavior for when a rust panic would happen, deactivate client perhaps?

[HadrienG2]

In my application, I maintain an AtomicBool that indicates if the audio thread is alive in the shared state between the audio threads and other application threads:

    // Check if the audio thread is alive
    fn is_alive(&self) -> bool {
        self.0.alive.load(Ordering::Relaxed)
    }

    // Mark the audio thread as dead
    fn mark_dead(&self) {
        self.0.alive.store(false, Ordering::Relaxed);
    }

Then I wrap all my JACK callbacks with the following...

    fn callback_guard<F>(&self, callback: F) -> Control
        where F: FnMut() -> Control + panic::UnwindSafe
    {
        if !self.is_alive() { return Control::Quit; }
        let result = panic::catch_unwind(callback);
        // FIXME: Store error somewhere so it can be processed, something based
        //        on AtomicPtr could do the trick and be async signal safe.
        let output = result.unwrap_or(Control::Quit);
        if output == Control::Quit { self.mark_dead(); }
        output
    }

...which ensures the following properties:

• Unhandled panics and Control::Quit returns mark the audio thread dead and stop audio work
• If the JACK callback supports it, JACK is notified by bubbling up a Control::Quit
• If the JACK callback does not support it, then the output can be ignored
• Application can poll is_alive() periodically to check audio thread health

This has the minor drawback of adding a Control return where there is none in the JACK API, which could be fixed by making another version of callback_guard for functions that don't return Control.

Another thing that could be improved is to differentiate Control::Quit returns from panics so that JACK callbacks continue to be processed normally during a graceful exit. This would require using an AtomicU8 instead of an AtomicBool in order to discriminate between a "working" state, a "panicked" state, and a "graceful exit" state when needed.

Maybe some variant of this design could be useful to you, and upstreamed as the official solution?