Skip to content

Commit

Permalink
Make safer. (#209)
Browse files Browse the repository at this point in the history
* Make shutdown unsafe.
* Don't return client,processor,notification if there was a panic.
* Bump major version.
  • Loading branch information
wmedrano authored Sep 13, 2024
1 parent 02a5936 commit b25d6d4
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 16 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 2 additions & 3 deletions examples/playback_capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
10 changes: 8 additions & 2 deletions src/client/async_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}
}

Expand Down
18 changes: 9 additions & 9 deletions src/client/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
2 changes: 1 addition & 1 deletion src/client/test_callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit b25d6d4

Please sign in to comment.