From 41e7572147665b8a26dd9702914c1ccb94730e91 Mon Sep 17 00:00:00 2001 From: mitchmindtree Date: Sun, 19 Mar 2017 19:09:20 +1100 Subject: [PATCH] [macos] Avoid panic when callback is `None`. This can happen when window is destroyed/created during a call to user callback as this causes WindowDelegate method to be called. Instead if the user callback is `None` store the event in `pending_events`. --- src/platform/macos/events_loop.rs | 54 +++++++++++++++++++++++-------- src/platform/macos/window.rs | 2 +- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/platform/macos/events_loop.rs b/src/platform/macos/events_loop.rs index f283a763..e4666a30 100644 --- a/src/platform/macos/events_loop.rs +++ b/src/platform/macos/events_loop.rs @@ -19,7 +19,7 @@ pub struct EventsLoop { // // This is *only* `Some` for the duration of a call to either of these methods and will be // `None` otherwise. - pub user_callback: UserCallback, + user_callback: UserCallback, } struct Modifiers { @@ -61,15 +61,19 @@ impl UserCallback { // Emits the given event via the user-given callback. // - // This is *only* called within the `poll_events` and `run_forever` methods so we know that it - // is safe to `unwrap` the callback without causing a panic as there must be at least one - // callback stored. - // // This is unsafe as it requires dereferencing the pointer to the user-given callback. We // guarantee this is safe by ensuring the `UserCallback` never lives longer than the user-given // callback. - pub unsafe fn call_with_event(&self, event: Event) { - let callback: *mut FnMut(Event) = self.mutex.lock().unwrap().take().unwrap(); + // + // Note that the callback may not always be `Some`. This is because some `NSWindowDelegate` + // callbacks can be triggered by means other than `NSApp().sendEvent`. For example, if a window + // is destroyed or created during a call to the user's callback, the `WindowDelegate` methods + // may be called with `windowShouldClose` or `windowDidResignKey`. + unsafe fn call_with_event(&self, event: Event) { + let callback = match self.mutex.lock().unwrap().take() { + Some(callback) => callback, + None => return, + }; (*callback)(event); *self.mutex.lock().unwrap() = Some(callback); } @@ -117,9 +121,7 @@ impl EventsLoop { loop { unsafe { // First, yield all pending events. - while let Some(event) = self.pending_events.lock().unwrap().pop_front() { - self.user_callback.call_with_event(event); - } + self.call_user_callback_with_pending_events(); let pool = foundation::NSAutoreleasePool::new(cocoa::base::nil); @@ -161,9 +163,7 @@ impl EventsLoop { loop { unsafe { // First, yield all pending events. - while let Some(event) = self.pending_events.lock().unwrap().pop_front() { - self.user_callback.call_with_event(event); - } + self.call_user_callback_with_pending_events(); let pool = foundation::NSAutoreleasePool::new(cocoa::base::nil); @@ -217,6 +217,34 @@ impl EventsLoop { } } + fn call_user_callback_with_pending_events(&self) { + loop { + let event = match self.pending_events.lock().unwrap().pop_front() { + Some(event) => event, + None => return, + }; + unsafe { + self.user_callback.call_with_event(event); + } + } + } + + // Calls the user callback if one exists. + // + // Otherwise, stores the event in the `pending_events` queue. + // + // This is necessary for the case when `WindowDelegate` callbacks are triggered during a call + // to the user's callback. + pub fn call_user_callback_with_event_or_store_in_pending(&self, event: Event) { + if self.user_callback.mutex.lock().unwrap().is_some() { + unsafe { + self.user_callback.call_with_event(event); + } + } else { + self.pending_events.lock().unwrap().push_back(event); + } + } + // Convert some given `NSEvent` into a winit `Event`. unsafe fn ns_event_to_event(&self, ns_event: cocoa::base::id) -> Option { if ns_event == cocoa::base::nil { diff --git a/src/platform/macos/window.rs b/src/platform/macos/window.rs index 04e17f9c..9ff25043 100644 --- a/src/platform/macos/window.rs +++ b/src/platform/macos/window.rs @@ -57,7 +57,7 @@ impl WindowDelegate { }; if let Some(events_loop) = state.events_loop.upgrade() { - events_loop.user_callback.call_with_event(event); + events_loop.call_user_callback_with_event_or_store_in_pending(event); } }