From 0ca64698e97776ef1754da97b4856bc6ef9190ad Mon Sep 17 00:00:00 2001 From: Matthew Collins Date: Sun, 19 Aug 2018 23:39:26 +0100 Subject: [PATCH] Fix a segfault when `get_callback` panics --- src/sdl2/audio.rs | 63 ++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/src/sdl2/audio.rs b/src/sdl2/audio.rs index c4bc2612..ca020dee 100644 --- a/src/sdl2/audio.rs +++ b/src/sdl2/audio.rs @@ -373,13 +373,15 @@ extern "C" fn audio_callback_marshall use std::slice::from_raw_parts_mut; use std::mem::size_of; unsafe { - let cb_userdata: &mut CB = &mut *(userdata as *mut CB); + let cb_userdata: &mut Option = &mut *(userdata as *mut _); let buf: &mut [CB::Channel] = from_raw_parts_mut( stream as *mut CB::Channel, len as usize / size_of::() ); - cb_userdata.callback(buf); + if let Some(cb) = cb_userdata { + cb.callback(buf); + } } } @@ -394,14 +396,13 @@ pub struct AudioSpecDesired { } impl AudioSpecDesired { - fn convert_to_ll(freq: F, channels: C, samples: S, userdata: *mut CB) -> sys::SDL_AudioSpec + fn convert_to_ll(freq: F, channels: C, samples: S, userdata: *mut Option) -> sys::SDL_AudioSpec where CB: AudioCallback, F: Into>, C: Into>, S: Into>, { - use std::mem::transmute; let freq = freq.into(); let channels = channels.into(); @@ -413,22 +414,20 @@ impl AudioSpecDesired { // A value of 0 means "fallback" or "default". - unsafe { - sys::SDL_AudioSpec { - freq: freq.unwrap_or(0), - format: ::audio_format().to_ll(), - channels: channels.unwrap_or(0), - silence: 0, - samples: samples.unwrap_or(0), - padding: 0, - size: 0, - callback: Some(audio_callback_marshall:: - as extern "C" fn - (arg1: *mut c_void, - arg2: *mut uint8_t, - arg3: c_int)), - userdata: transmute(userdata) - } + sys::SDL_AudioSpec { + freq: freq.unwrap_or(0), + format: ::audio_format().to_ll(), + channels: channels.unwrap_or(0), + silence: 0, + samples: samples.unwrap_or(0), + padding: 0, + size: 0, + callback: Some(audio_callback_marshall:: + as extern "C" fn + (arg1: *mut c_void, + arg2: *mut uint8_t, + arg3: c_int)), + userdata: userdata as *mut _, } } @@ -596,7 +595,7 @@ pub struct AudioDevice { device_id: AudioDeviceID, spec: AudioSpec, /// Store the callback to keep it alive for the entire duration of `AudioDevice`. - userdata: Box + userdata: Box> } impl AudioDevice { @@ -607,14 +606,8 @@ impl AudioDevice { D: Into>, { - // SDL_OpenAudioDevice needs a userdata pointer, but we can't initialize the - // callback without the obtained AudioSpec. - // Create an uninitialized box that will be initialized after SDL_OpenAudioDevice. - let userdata: *mut CB = unsafe { - let b: Box = Box::new(mem::uninitialized()); - mem::transmute(b) - }; - let desired = AudioSpecDesired::convert_to_ll(spec.freq, spec.channels, spec.samples, userdata); + let mut userdata: Box> = Box::new(None); + let desired = AudioSpecDesired::convert_to_ll(spec.freq, spec.channels, spec.samples, &mut *userdata); let mut obtained = unsafe { mem::uninitialized::() }; unsafe { @@ -636,10 +629,8 @@ impl AudioDevice { id => { let device_id = AudioDeviceID::PlaybackDevice(id); let spec = AudioSpec::convert_from_ll(obtained); - let mut userdata: Box = mem::transmute(userdata); - let garbage = mem::replace(&mut userdata as &mut CB, get_callback(spec)); - mem::forget(garbage); + *userdata = Some(get_callback(spec)); Ok(AudioDevice { subsystem: a.clone(), @@ -713,7 +704,7 @@ impl AudioDevice { /// but the callback data will be dropped. pub fn close_and_get_callback(self) -> CB { drop(self.device_id); - *self.userdata + self.userdata.expect("Missing callback") } } @@ -725,11 +716,11 @@ pub struct AudioDeviceLockGuard<'a, CB> where CB: AudioCallback, CB: 'a { impl<'a, CB: AudioCallback> Deref for AudioDeviceLockGuard<'a, CB> { type Target = CB; - fn deref(&self) -> &CB { &self.device.userdata } + fn deref(&self) -> &CB { (*self.device.userdata).as_ref().expect("Missing callback") } } impl<'a, CB: AudioCallback> DerefMut for AudioDeviceLockGuard<'a, CB> { - fn deref_mut(&mut self) -> &mut CB { &mut self.device.userdata } + fn deref_mut(&mut self) -> &mut CB { (*self.device.userdata).as_mut().expect("Missing callback") } } impl<'a, CB: AudioCallback> Drop for AudioDeviceLockGuard<'a, CB> { @@ -835,7 +826,7 @@ mod test { assert_eq!(new_buffer.len(), new_buffer_expected.len(), "capacity must be exactly equal to twice the original vec size"); // // this has been commented, see https://discourse.libsdl.org/t/change-of-behavior-in-audiocvt-sdl-convertaudio-from-2-0-5-to-2-0-6/24682 - // // to maybe re-enable it someday + // // to maybe re-enable it someday // assert_eq!(new_buffer, new_buffer_expected); } }