Merge branch 'fix/ffi-callback-panic-containment' into 'master'

Contain panics in FFI callbacks

See merge request videolan/vlc-rs!26

src/core.rs

@@ -10,7 +10,7 @@ use std::i32;
 use std::convert::TryInto;
 use libc::{c_void, c_char, c_int};
 use vlc_sys as sys;
-use crate::tools::{to_cstr, from_cstr, from_cstr_ref};
+use crate::tools::{to_cstr, from_cstr, from_cstr_ref, invoke_callback};
 use crate::enums::*;
 
 /// Retrieve libvlc version.
@@ -131,7 +131,9 @@ impl Instance {
         }
     }
 
-    /// Set logging callback
+    /// Set logging callback.
+    ///
+    /// Panics from this callback are caught and do not unwind into LibVLC.
     pub fn set_log<F: Fn(LogLevel, Log, Cow<str>) + Send + 'static>(&self, f: F) {
         let cb: Box<Box<dyn Fn(LogLevel, Log, Cow<str>) + Send + 'static>> = Box::new(Box::new(f));
 
@@ -157,13 +159,14 @@ impl Drop for Instance {
 const BUF_SIZE: usize = 1024; // Write log message to the buffer by vsnprintf.
 unsafe extern "C" fn logging_cb(
     data: *mut c_void, level: c_int, ctx: *const sys::libvlc_log_t, fmt: *const c_char, args: sys::VaList) {
+    invoke_callback(|| {
+        let f: &Box<dyn Fn(LogLevel, Log, Cow<str>) + Send + 'static> = ::std::mem::transmute(data);
+        let mut buf: [c_char; BUF_SIZE] = [0; BUF_SIZE];
 
-    let f: &Box<dyn Fn(LogLevel, Log, Cow<str>) + Send + 'static> = ::std::mem::transmute(data);
+        sys::vsnprintf(buf.as_mut_ptr(), BUF_SIZE.try_into().unwrap(), fmt, args);
-    let mut buf: [c_char; BUF_SIZE] = [0; BUF_SIZE];
 
-    sys::vsnprintf(buf.as_mut_ptr(), BUF_SIZE.try_into().unwrap(), fmt, args);
+        f((level as u32).into(), Log{ptr: ctx}, from_cstr_ref(buf.as_ptr()).unwrap());
-
+    });
-    f((level as u32).into(), Log{ptr: ctx}, from_cstr_ref(buf.as_ptr()).unwrap());
 }
 
 /// List of module description.
@@ -328,6 +331,9 @@ impl<'a> EventManager<'a> {
         unsafe { sys::libvlc_event_detach(self.ptr, event_type as i32, Some(event_manager_callback), registered_callback) }
     }
 
+    /// Attach an event callback.
+    ///
+    /// Panics from this callback are caught and do not unwind into LibVLC.
     pub fn attach<F>(&self, event_type: EventType, callback: F) -> Result<*mut c_void, ()>
         where F: Fn(Event, VLCObject) + Send + 'static
     {
@@ -357,9 +363,11 @@ impl<'a> EventManager<'a> {
 }
 
 unsafe extern "C" fn event_manager_callback(pe: *const sys::libvlc_event_t, data: *mut c_void) {
-    let f: &Box<dyn Fn(Event, VLCObject) + Send + 'static> = ::std::mem::transmute(data);
+    invoke_callback(|| {
+        let f: &Box<dyn Fn(Event, VLCObject) + Send + 'static> = ::std::mem::transmute(data);
 
-    f(conv_event(pe), VLCObject{ ptr: (*pe).p_obj });
+        f(conv_event(pe), VLCObject{ ptr: (*pe).p_obj });
+    });
 }
 
 // Convert c-style libvlc_event_t to Event
@@ -579,4 +587,3 @@ impl Log {
         self.ptr
     }
 }
-

src/media_player.rs

@@ -8,6 +8,7 @@ use crate::Media;
 use crate::EventManager;
 use libc::{c_void, c_uint};
 use crate::enums::{State, Position};
+use crate::tools::invoke_callback;
 use std::mem::transmute;
 
 /// A LibVLC media player plays one media (usually in a custom drawable).
@@ -87,6 +88,9 @@ impl MediaPlayer {
         unsafe{ sys::libvlc_media_player_stop(self.ptr) };
     }
 
+    /// Set audio callbacks.
+    ///
+    /// Panics from these callbacks are caught and do not unwind into LibVLC.
     pub fn set_callbacks<F>(
         &self,
         play: F,
@@ -337,29 +341,38 @@ struct AudioCallbacksData {
 
 unsafe extern "C" fn audio_cb_play(
     data: *mut c_void, samples: *const c_void, count: c_uint, pts: i64) {
-    let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+    invoke_callback(|| {
-    (data.play)(samples, count, pts);
+        let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
-
+        (data.play)(samples, count, pts);
+    });
 }
 
 unsafe extern "C" fn audio_cb_pause(data: *mut c_void, pts: i64) {
-    let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+    invoke_callback(|| {
-    (data.pause.as_ref().unwrap())(pts);
+        let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+        (data.pause.as_ref().unwrap())(pts);
+    });
 }
 
 unsafe extern "C" fn audio_cb_resume(data: *mut c_void, pts: i64) {
-    let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+    invoke_callback(|| {
-    (data.resume.as_ref().unwrap())(pts);
+        let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+        (data.resume.as_ref().unwrap())(pts);
+    });
 }
 
 unsafe extern "C" fn audio_cb_flush(data: *mut c_void, pts: i64) {
-    let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+    invoke_callback(|| {
-    (data.flush.as_ref().unwrap())(pts);
+        let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+        (data.flush.as_ref().unwrap())(pts);
+    });
 }
 
 unsafe extern "C" fn audio_cb_drain(data: *mut c_void) {
-    let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+    invoke_callback(|| {
-    (data.drain.as_ref().unwrap())();
+        let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData);
+        (data.drain.as_ref().unwrap())();
+    });
 }
 
 #[derive(Clone, PartialEq, Eq, Hash, Debug)]
@@ -367,4 +380,3 @@ pub struct TrackDescription {
     pub id: i32,
     pub name: Option<String>,
 }
-

src/tools.rs

@@ -5,6 +5,7 @@
 use std::ffi::{CString, CStr, NulError};
 use std::path::Path;
 use std::borrow::Cow;
+use std::panic::{catch_unwind, AssertUnwindSafe};
 use libc::c_char;
 
 // Convert String to CString.
@@ -41,3 +42,28 @@ pub fn path_to_cstr(path: &Path) -> Result<CString, NulError> {
 
     Ok(path)
 }
+
+/// Invoke a Rust callback from a foreign callback without unwinding into C.
+///
+/// LibVLC callback APIs do not provide an error channel, so panics are caught
+/// and ignored after Rust's panic hook has run.
+pub(crate) fn invoke_callback<F>(callback: F)
+    where F: FnOnce()
+{
+    let _ = catch_unwind(AssertUnwindSafe(callback));
+}
+
+#[cfg(test)]
+mod tests {
+    use super::invoke_callback;
+    use std::panic::{catch_unwind, AssertUnwindSafe};
+
+    #[test]
+    fn invoke_callback_contains_panics() {
+        let result = catch_unwind(AssertUnwindSafe(|| {
+            invoke_callback(|| panic!("callback panic"));
+        }));
+
+        assert!(result.is_ok());
+    }
+}