From d426f69ca20e7f817ac913ec3c8fda0699d4e817 Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Wed, 11 Nov 2020 21:51:08 -0800 Subject: [PATCH] sndio: avoid unsafe impl Sync on InnerState --- src/host/sndio/mod.rs | 20 +++++++++++--------- src/host/sndio/runner.rs | 14 +++++++------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index aa8f4e9f9..c63001744 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -113,12 +113,16 @@ impl Iterator for Devices { } } +struct SioHdl(*mut sndio_sys::sio_hdl); + +unsafe impl Send for SioHdl {} + /// The shared state between Device and Stream. Responsible for closing handle when dropped. struct InnerState { /// If device has been open with sio_open, contains a handle. Note that even though this is a /// pointer type and so doesn't follow Rust's borrowing rules, we should be careful not to copy /// it out because that may render Mutex ineffective in enforcing exclusive access. - hdl: Option<*mut sndio_sys::sio_hdl>, + hdl: Option, /// Buffer overrun/underrun behavior -- ignore/sync/error? behavior: BufferXrunBehavior, @@ -159,8 +163,6 @@ struct OutputCallbacks { error_callback: Box, } -unsafe impl Send for InnerState {} - #[derive(Debug, Copy, Clone, Eq, PartialEq)] enum Status { /// Initial state. No thread running. Device/Stream methods will start thread and change this @@ -205,7 +207,7 @@ impl InnerState { if hdl.is_null() { return Err(SndioError::DeviceNotAvailable); } - self.hdl = Some(hdl); + self.hdl = Some(SioHdl(hdl)); let mut sample_rate_to_par = HashMap::new(); for rate in SUPPORTED_SAMPLE_RATES { @@ -267,7 +269,7 @@ impl InnerState { // will wait for playback data to be provided (using the sio_write() // function). Once enough data is queued to ensure that play buffers will // not underrun, actual playback is started automatically." - sndio_sys::sio_start(self.hdl.unwrap()) // Unwrap OK because of check above + sndio_sys::sio_start(self.hdl.as_ref().unwrap().0) // Unwrap OK because of check above }; if status != 1 { return Err(backend_specific_error("failed to start stream")); @@ -286,7 +288,7 @@ impl InnerState { // playback. If samples to play are queued but playback hasn't started yet then // playback is forced immediately; playback will actually stop once the buffer is // drained. In no case are samples in the play buffer discarded. - sndio_sys::sio_stop(self.hdl.unwrap()) // Unwrap OK because of check above + sndio_sys::sio_stop(self.hdl.as_ref().unwrap().0) // Unwrap OK because of check above }; if status != 1 { return Err(backend_specific_error("error calling sio_stop")); @@ -385,7 +387,7 @@ impl InnerState { let status = unsafe { // Retrieve the actual parameters of the device. - sndio_sys::sio_getpar(self.hdl.unwrap(), par as *mut _) + sndio_sys::sio_getpar(self.hdl.as_ref().unwrap().0, par as *mut _) }; if status != 1 { return Err(backend_specific_error( @@ -439,7 +441,7 @@ impl InnerState { let status = unsafe { // Request the device using our parameters // unwrap OK because of the check at the top of this function. - sndio_sys::sio_setpar(self.hdl.unwrap(), &mut newpar as *mut _) + sndio_sys::sio_setpar(self.hdl.as_ref().unwrap().0, &mut newpar as *mut _) }; if status != 1 { return Err(backend_specific_error("failed to set parameters with sio_setpar").into()); @@ -452,7 +454,7 @@ impl Drop for InnerState { fn drop(&mut self) { if let Some(hdl) = self.hdl.take() { unsafe { - sndio_sys::sio_close(hdl); + sndio_sys::sio_close(hdl.0); } } } diff --git a/src/host/sndio/runner.rs b/src/host/sndio/runner.rs index f7db8bf4a..03b0c9553 100644 --- a/src/host/sndio/runner.rs +++ b/src/host/sndio/runner.rs @@ -114,7 +114,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { paused = false; } nfds = unsafe { - sndio_sys::sio_nfds(inner_state.hdl.unwrap()) // Unwrap OK because of open call above + sndio_sys::sio_nfds(inner_state.hdl.as_ref().unwrap().0) // Unwrap OK because of open call above }; if nfds <= 0 { inner_state.error(backend_specific_error(format!( @@ -133,7 +133,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { // Populate pollfd structs with sndio_sys::sio_pollfd nfds = unsafe { sndio_sys::sio_pollfd( - inner_state.hdl.unwrap(), // Unwrap OK because of open call above + inner_state.hdl.as_ref().unwrap().0, // Unwrap OK because of open call above pollfds.as_mut_ptr(), (libc::POLLOUT | libc::POLLIN) as i32, ) @@ -162,9 +162,9 @@ pub(super) fn runner(inner_state_arc: Arc>) { { let mut inner_state = inner_state_arc.lock().unwrap(); // Unwrap OK because of open call above - revents = - unsafe { sndio_sys::sio_revents(inner_state.hdl.unwrap(), pollfds.as_mut_ptr()) } - as i16; + revents = unsafe { + sndio_sys::sio_revents(inner_state.hdl.as_ref().unwrap().0, pollfds.as_mut_ptr()) + } as i16; if revents & libc::POLLHUP != 0 { inner_state.error(backend_specific_error("device disappeared")); break; @@ -222,7 +222,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { // unwrap OK because .open was called let bytes_written = unsafe { sndio_sys::sio_write( - inner_state.hdl.unwrap(), + inner_state.hdl.as_ref().unwrap().0, (output_data.data as *const u8).add(output_offset_bytes_into_buf as usize) as *const _, data_byte_size as u64 - output_offset_bytes_into_buf, @@ -271,7 +271,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { // unwrap OK because .open was called let bytes_read = unsafe { sndio_sys::sio_read( - inner_state.hdl.unwrap(), + inner_state.hdl.as_ref().unwrap().0, (input_data.data as *const u8).add(input_offset_bytes_into_buf as usize) as *mut _, data_byte_size as u64 - input_offset_bytes_into_buf,