From 2b4e565833b38ab4a6374a9395e696f88540e5eb Mon Sep 17 00:00:00 2001 From: Will Medrano Date: Thu, 12 Sep 2024 20:16:00 -0700 Subject: [PATCH 1/4] Make shutdown unsafe. --- examples/playback_capture.rs | 5 ++--- src/client/callbacks.rs | 17 ++++++++--------- src/client/test_callback.rs | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/examples/playback_capture.rs b/examples/playback_capture.rs index 528154a9..4e24b15f 100644 --- a/examples/playback_capture.rs +++ b/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!( diff --git a/src/client/callbacks.rs b/src/client/callbacks.rs index f6e95434..31bb9b19 100644 --- a/src/client/callbacks.rs +++ b/src/client/callbacks.rs @@ -16,15 +16,14 @@ 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. + /// + /// See https://man7.org/linux/man-pages/man7/signal-safety.7.html for details about + /// async-signal-safe. + 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) {} diff --git a/src/client/test_callback.rs b/src/client/test_callback.rs index 28da777b..fe0f9ae8 100644 --- a/src/client/test_callback.rs +++ b/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); From 39a3af98a37f41d2607fb9573097ef73657f39dc Mon Sep 17 00:00:00 2001 From: Will Medrano Date: Thu, 12 Sep 2024 20:22:18 -0700 Subject: [PATCH 2/4] Don't return client,processor,notification if there was a panic. --- src/client/async_client.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/client/async_client.rs b/src/client/async_client.rs index 52fdc028..973aa535 100644 --- a/src/client/async_client.rs +++ b/src/client/async_client.rs @@ -111,7 +111,7 @@ impl AsyncClient { 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 AsyncClient { 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) + } } } From 50b170e494f1f93a095468e906c8dfdd0d4f15ed Mon Sep 17 00:00:00 2001 From: Will Medrano Date: Thu, 12 Sep 2024 20:25:31 -0700 Subject: [PATCH 3/4] Add safety section. --- src/client/callbacks.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/client/callbacks.rs b/src/client/callbacks.rs index 31bb9b19..a273a3e0 100644 --- a/src/client/callbacks.rs +++ b/src/client/callbacks.rs @@ -21,8 +21,9 @@ pub trait NotificationHandler: Send { /// 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 - /// async-signal-safe. + /// 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. From c0db4e7304bf29ccd8ecf1be997b44d0ff089799 Mon Sep 17 00:00:00 2001 From: Will Medrano Date: Fri, 13 Sep 2024 07:32:27 -0700 Subject: [PATCH 4/4] Bump major version. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 954a8f1b..7f6c57c9 100644 --- a/Cargo.toml +++ b/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"