Make safer. (#209)

* Make shutdown unsafe.
* Don't return client,processor,notification if there was a panic.
* Bump major version.

Author: wmedrano

Cargo.toml

@@ -8,7 +8,7 @@ license = "MIT"
 name = "jack"
 readme = "README.md"
 repository = "https://github.com/RustAudio/rust-jack"
-version = "0.12.2"
+version = "0.13.0"
 
 [dependencies]
 bitflags = "2"

examples/playback_capture.rs

@@ -51,9 +51,8 @@ impl jack::NotificationHandler for Notifications {
         println!("JACK: thread init");
     }
 
-    fn shutdown(&mut self, status: jack::ClientStatus, reason: &str) {
-        println!("JACK: shutdown with status {status:?} because \"{reason}\"",);
-    }
+    /// Not much we can do here, see https://man7.org/linux/man-pages/man7/signal-safety.7.html.
+    unsafe fn shutdown(&mut self, _: jack::ClientStatus, _: &str) {}
 
     fn freewheel(&mut self, _: &jack::Client, is_enabled: bool) {
         println!(

src/client/async_client.rs

@@ -111,7 +111,7 @@ impl<N, P> AsyncClient<N, P> {
         if self.callback.is_none() {
             return Err(Error::ClientIsNoLongerAlive);
         }
-        let cb = self.callback.take().unwrap();
+        let cb = self.callback.take().ok_or(Error::ClientIsNoLongerAlive)?;
         let client = cb.client.raw();
 
         // deactivate
@@ -124,7 +124,13 @@ impl<N, P> AsyncClient<N, P> {
         sleep_on_test();
         clear_callbacks(client)?;
         // done, take ownership of callback
-        Ok(cb)
+        if cb.is_valid.load(std::sync::atomic::Ordering::Relaxed) {
+            Ok(cb)
+        } else {
+            std::mem::forget(cb.notification);
+            std::mem::forget(cb.process);
+            Err(Error::ClientIsNoLongerAlive)
+        }
     }
 }
 

src/client/callbacks.rs

@@ -16,15 +16,15 @@ pub trait NotificationHandler: Send {
     /// It does not need to be suitable for real-time execution.
     fn thread_init(&self, _: &Client) {}
 
-    /// Called when the JACK server shuts down the client thread. The function
-    /// must be written as if
-    /// it were an asynchronous POSIX signal handler --- use only async-safe
-    /// functions, and remember
-    /// that it is executed from another thread. A typical funcion might set a
-    /// flag or write to a
-    /// pipe so that the rest of the application knows that the JACK client
-    /// thread has shut down.
-    fn shutdown(&mut self, _status: ClientStatus, _reason: &str) {}
+    /// Called when the JACK server shuts down the client thread. The function must be written as if
+    /// it were an asynchronous POSIX signal handler --- use only async-safe functions, and remember
+    /// that it is executed from another thread. A typical funcion might set a flag or write to a
+    /// pipe so that the rest of the application knows that the JACK client thread has shut down.
+    ///
+    /// # Safety
+    /// See https://man7.org/linux/man-pages/man7/signal-safety.7.html for details about
+    /// what is legal in an async-signal-safe callback.
+    unsafe fn shutdown(&mut self, _status: ClientStatus, _reason: &str) {}
 
     /// Called whenever "freewheel" mode is entered or leaving.
     fn freewheel(&mut self, _: &Client, _is_freewheel_enabled: bool) {}

src/client/test_callback.rs

@@ -86,7 +86,7 @@ fn client_cback_has_proper_default_callbacks() {
     let ps = unsafe { ProcessScope::from_raw(0, ptr::null_mut()) };
     // check each callbacks
     ().thread_init(&wc);
-    ().shutdown(client_status::ClientStatus::empty(), "mock");
+    unsafe { ().shutdown(client_status::ClientStatus::empty(), "mock") };
     assert_eq!(().process(&wc, &ps), Control::Continue);
     ().freewheel(&wc, true);
     ().freewheel(&wc, false);