diff --git a/CHANGELOG.md b/CHANGELOG.md index e6674626..cc52579c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - On X11, fixed panic caused by dropping the window before running the event loop. - Introduce `WindowBuilderExt::with_app_id` to allow setting the application ID on Wayland. +- On Windows, catch panics in event loop child thread and forward them to the parent thread. This prevents an invocation of undefined behavior due to unwinding into foreign code. - On Windows, fix issue where resizing or moving window combined with grabbing the cursor would freeze program. - On Windows, fix issue where resizing or moving window would eat `Awakened` events. diff --git a/Cargo.toml b/Cargo.toml index 1e06f5e8..3b0dcc13 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,9 @@ cocoa = "0.18.4" core-foundation = "0.6" core-graphics = "0.17.3" +[target.'cfg(target_os = "windows")'.dependencies] +backtrace = "0.3" + [target.'cfg(target_os = "windows")'.dependencies.winapi] version = "0.3.6" features = [ diff --git a/src/lib.rs b/src/lib.rs index 0d2b59fa..4ab8f44e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -98,6 +98,8 @@ extern crate serde; #[cfg(target_os = "windows")] extern crate winapi; +#[cfg(target_os = "windows")] +extern crate backtrace; #[cfg(any(target_os = "macos", target_os = "ios"))] #[macro_use] extern crate objc; diff --git a/src/platform/windows/events_loop.rs b/src/platform/windows/events_loop.rs index 96c9a41a..d94b1bfc 100644 --- a/src/platform/windows/events_loop.rs +++ b/src/platform/windows/events_loop.rs @@ -12,12 +12,14 @@ //! The closure passed to the `execute_in_thread` method takes an `Inserter` that you can use to //! add a `WindowState` entry to a list of window to be used by the callback. -use std::{mem, ptr, thread}; +use std::{mem, panic, ptr, thread}; +use std::any::Any; use std::cell::RefCell; use std::collections::HashMap; use std::os::windows::io::AsRawHandle; use std::sync::{Arc, mpsc, Mutex}; +use backtrace::Backtrace; use winapi::ctypes::c_int; use winapi::shared::minwindef::{ BOOL, @@ -138,9 +140,14 @@ pub struct EventsLoop { // Id of the background thread from the Win32 API. thread_id: DWORD, // Receiver for the events. The sender is in the background thread. - receiver: mpsc::Receiver, + receiver: mpsc::Receiver, // Sender instance that's paired with the receiver. Used to construct an `EventsLoopProxy`. - sender: mpsc::Sender, + sender: mpsc::Sender, +} + +enum EventsLoopEvent { + WinitEvent(Event), + Panic(PanicError), } impl EventsLoop { @@ -164,6 +171,7 @@ impl EventsLoop { let (init_tx, init_rx) = mpsc::sync_channel(0); let thread_sender = tx.clone(); + let panic_sender = tx.clone(); let thread = thread::spawn(move || { let tx = thread_sender; let thread_msg_target = thread_event_target_window(); @@ -174,6 +182,7 @@ impl EventsLoop { windows: HashMap::with_capacity(4), file_drop_handlers: HashMap::with_capacity(4), mouse_buttons_down: 0, + panic_error: None, }); }); @@ -191,6 +200,16 @@ impl EventsLoop { loop { if winuser::GetMessageW(&mut msg, ptr::null_mut(), 0, 0) == 0 { + // If a panic occurred in the child callback, forward the panic information + // to the parent thread. + let panic_payload_opt = CONTEXT_STASH.with(|stash| + stash.borrow_mut().as_mut() + .and_then(|s| s.panic_error.take()) + ); + if let Some(panic_payload) = panic_payload_opt { + panic_sender.send(EventsLoopEvent::Panic(panic_payload)).unwrap(); + }; + // Only happens if the message is `WM_QUIT`. debug_assert_eq!(msg.message, winuser::WM_QUIT); break; @@ -224,8 +243,12 @@ impl EventsLoop { { loop { let event = match self.receiver.try_recv() { - Ok(e) => e, - Err(_) => return + Ok(EventsLoopEvent::WinitEvent(e)) => e, + Ok(EventsLoopEvent::Panic(panic)) => { + eprintln!("resuming child thread unwind at: {:?}", Backtrace::new()); + panic::resume_unwind(panic) + }, + Err(_) => break, }; callback(event); @@ -237,8 +260,12 @@ impl EventsLoop { { loop { let event = match self.receiver.recv() { - Ok(e) => e, - Err(_) => return + Ok(EventsLoopEvent::WinitEvent(e)) => e, + Ok(EventsLoopEvent::Panic(panic)) => { + eprintln!("resuming child thread unwind at: {:?}", Backtrace::new()); + panic::resume_unwind(panic) + }, + Err(_) => break, }; let flow = callback(event); @@ -282,7 +309,7 @@ impl Drop for EventsLoop { #[derive(Clone)] pub struct EventsLoopProxy { thread_msg_target: HWND, - sender: mpsc::Sender, + sender: mpsc::Sender, } unsafe impl Send for EventsLoopProxy {} @@ -290,7 +317,7 @@ unsafe impl Sync for EventsLoopProxy {} impl EventsLoopProxy { pub fn wakeup(&self) -> Result<(), EventsLoopClosed> { - self.sender.send(Event::Awakened).map_err(|_| EventsLoopClosed) + self.sender.send(EventsLoopEvent::WinitEvent(Event::Awakened)).map_err(|_| EventsLoopClosed) } /// Executes a function in the background thread. @@ -407,45 +434,78 @@ fn thread_event_target_window() -> HWND { // in a thread-local variable. thread_local!(static CONTEXT_STASH: RefCell> = RefCell::new(None)); struct ThreadLocalData { - sender: mpsc::Sender, + sender: mpsc::Sender, windows: HashMap>>, file_drop_handlers: HashMap, // Each window has its own drop handler. mouse_buttons_down: u32, + panic_error: Option, } +type PanicError = Box; // Utility function that dispatches an event on the current thread. pub fn send_event(event: Event) { CONTEXT_STASH.with(|context_stash| { let context_stash = context_stash.borrow(); - let _ = context_stash.as_ref().unwrap().sender.send(event); // Ignoring if closed + let _ = context_stash.as_ref().unwrap().sender.send(EventsLoopEvent::WinitEvent(event)); // Ignoring if closed }); } /// Capture mouse input, allowing `window` to receive mouse events when the cursor is outside of /// the window. unsafe fn capture_mouse(window: HWND) { - CONTEXT_STASH.with(|context_stash| { + let set_capture = CONTEXT_STASH.with(|context_stash| { let mut context_stash = context_stash.borrow_mut(); if let Some(context_stash) = context_stash.as_mut() { context_stash.mouse_buttons_down += 1; - winuser::SetCapture(window); + true + } else { + false } }); + if set_capture { + winuser::SetCapture(window); + } } /// Release mouse input, stopping windows on this thread from receiving mouse input when the cursor /// is outside the window. unsafe fn release_mouse() { - CONTEXT_STASH.with(|context_stash| { + let release_capture = CONTEXT_STASH.with(|context_stash| { let mut context_stash = context_stash.borrow_mut(); if let Some(context_stash) = context_stash.as_mut() { context_stash.mouse_buttons_down = context_stash.mouse_buttons_down.saturating_sub(1); if context_stash.mouse_buttons_down == 0 { - winuser::ReleaseCapture(); + return true; } } + false }); + if release_capture { + winuser::ReleaseCapture(); + } +} + +pub unsafe fn run_catch_panic(error: R, f: F) -> R + where F: panic::UnwindSafe + FnOnce() -> R +{ + // If a panic has been triggered, cancel all future operations in the function. + if CONTEXT_STASH.with(|stash| stash.borrow().as_ref().map(|s| s.panic_error.is_some()).unwrap_or(false)) { + return error; + } + + let callback_result = panic::catch_unwind(f); + match callback_result { + Ok(lresult) => lresult, + Err(err) => CONTEXT_STASH.with(|context_stash| { + let mut context_stash = context_stash.borrow_mut(); + if let Some(context_stash) = context_stash.as_mut() { + context_stash.panic_error = Some(err); + winuser::PostQuitMessage(-1); + } + error + }) + } } /// Any window whose callback is configured to this function will have its events propagated @@ -460,6 +520,17 @@ pub unsafe extern "system" fn callback( msg: UINT, wparam: WPARAM, lparam: LPARAM, +) -> LRESULT { + // Unwinding into foreign code is undefined behavior. So we catch any panics that occur in our + // code, and if a panic happens we cancel any future operations. + run_catch_panic(-1, || callback_inner(window, msg, wparam, lparam)) +} + +unsafe fn callback_inner( + window: HWND, + msg: UINT, + wparam: WPARAM, + lparam: LPARAM, ) -> LRESULT { match msg { winuser::WM_CREATE => { @@ -564,7 +635,7 @@ pub unsafe extern "system" fn callback( event: Resized(logical_size), }; - cstash.sender.send(event).ok(); + cstash.sender.send(EventsLoopEvent::WinitEvent(event)).ok(); }); 0 }, @@ -1214,12 +1285,15 @@ pub unsafe extern "system" fn thread_event_target_callback( wparam: WPARAM, lparam: LPARAM, ) -> LRESULT { - match msg { - _ if msg == *EXEC_MSG_ID => { - let mut function: Box> = Box::from_raw(wparam as usize as *mut _); - function(); - 0 - }, - _ => winuser::DefWindowProcW(window, msg, wparam, lparam) - } + // See `callback` comment. + run_catch_panic(-1, || { + match msg { + _ if msg == *EXEC_MSG_ID => { + let mut function: Box> = Box::from_raw(wparam as usize as *mut _); + function(); + 0 + }, + _ => winuser::DefWindowProcW(window, msg, wparam, lparam) + } + }) }