diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 319d3390..dd56f58f 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -11,8 +11,8 @@ //! //! And the following platform-specific modules: //! -//! - `run_ondemand` (available on `windows`, `android`) -//! - `pump_events` (available on `windows`, `android`) +//! - `run_ondemand` (available on `windows`, `macos`, `android`) +//! - `pump_events` (available on `windows`, `macos`, `android`) //! - `run_return` (available on `windows`, `unix`, `macos`, and `android`) //! //! However only the module corresponding to the platform you're compiling to will be available. @@ -36,10 +36,10 @@ pub mod windows; #[cfg(x11_platform)] pub mod x11; -#[cfg(any(windows_platform, android_platform))] +#[cfg(any(windows_platform, macos_platform, android_platform))] pub mod run_ondemand; -#[cfg(any(windows_platform, android_platform,))] +#[cfg(any(windows_platform, macos_platform, android_platform,))] pub mod pump_events; #[cfg(any( diff --git a/src/platform_impl/macos/app_state.rs b/src/platform_impl/macos/app_state.rs index 1a85b40f..6569a7cf 100644 --- a/src/platform_impl/macos/app_state.rs +++ b/src/platform_impl/macos/app_state.rs @@ -65,14 +65,17 @@ impl EventLoopHandler { RefMut<'_, dyn FnMut(Event<'_, T>, &RootWindowTarget, &mut ControlFlow)>, ), { + // The `NSApp` and our `HANDLER` are global state and so it's possible that + // we could get a delegate callback after the application has exit an + // `EventLoop`. If the loop has been exit then our weak `self.callback` + // will fail to upgrade. + // + // We don't want to panic or output any verbose logging if we fail to + // upgrade the weak reference since it might be valid that the application + // re-starts the `NSApp` after exiting a Winit `EventLoop` if let Some(callback) = self.callback.upgrade() { let callback = callback.borrow_mut(); (f)(self, callback); - } else { - panic!( - "Tried to dispatch an event, but the event loop that \ - owned the event handler callback seems to be destroyed" - ); } } } @@ -90,6 +93,7 @@ impl EventHandler for EventLoopHandler { fn handle_nonuser_event(&mut self, event: Event<'_, Never>, control_flow: &mut ControlFlow) { self.with_callback(|this, mut callback| { if let ControlFlow::ExitWithCode(code) = *control_flow { + // XXX: why isn't event dispatching simply skipped after control_flow = ExitWithCode? let dummy = &mut ControlFlow::ExitWithCode(code); (callback)(event.userify(), &this.window_target, dummy); } else { @@ -102,6 +106,7 @@ impl EventHandler for EventLoopHandler { self.with_callback(|this, mut callback| { for event in this.window_target.p.receiver.try_iter() { if let ControlFlow::ExitWithCode(code) = *control_flow { + // XXX: why isn't event dispatching simply skipped after control_flow = ExitWithCode? let dummy = &mut ControlFlow::ExitWithCode(code); (callback)(Event::UserEvent(event), &this.window_target, dummy); } else { @@ -114,7 +119,11 @@ impl EventHandler for EventLoopHandler { #[derive(Default)] struct Handler { - ready: AtomicBool, + stop_app_on_launch: AtomicBool, + stop_app_before_wait: AtomicBool, + stop_app_on_redraw: AtomicBool, + launched: AtomicBool, + running: AtomicBool, in_callback: AtomicBool, control_flow: Mutex, control_flow_prev: Mutex, @@ -141,12 +150,37 @@ impl Handler { self.waker.lock().unwrap() } - fn is_ready(&self) -> bool { - self.ready.load(Ordering::Acquire) + /// `true` after `ApplicationDelegate::applicationDidFinishLaunching` called + /// + /// NB: This is global / `NSApp` state and since the app will only be launched + /// once but an `EventLoop` may be run more than once then only the first + /// `EventLoop` will observe the `NSApp` before it is launched. + fn is_launched(&self) -> bool { + self.launched.load(Ordering::Acquire) } - fn set_ready(&self) { - self.ready.store(true, Ordering::Release); + /// Set via `ApplicationDelegate::applicationDidFinishLaunching` + fn set_launched(&self) { + self.launched.store(true, Ordering::Release); + } + + /// `true` if an `EventLoop` is currently running + /// + /// NB: This is global / `NSApp` state and may persist beyond the lifetime of + /// a running `EventLoop`. + /// + /// # Caveat + /// This is only intended to be called from the main thread + fn is_running(&self) -> bool { + self.running.load(Ordering::Relaxed) + } + + /// Set when an `EventLoop` starts running, after the `NSApp` is launched + /// + /// # Caveat + /// This is only intended to be called from the main thread + fn set_running(&self) { + self.running.store(true, Ordering::Relaxed); } fn should_exit(&self) -> bool { @@ -156,6 +190,74 @@ impl Handler { ) } + /// Clears the `running` state and resets the `control_flow` state when an `EventLoop` exits + /// + /// Since an `EventLoop` may be run more than once we need make sure to reset the + /// `control_flow` state back to `Poll` each time the loop exits. + /// + /// Note: that if the `NSApp` has been launched then that state is preserved, and we won't + /// need to re-launch the app if subsequent EventLoops are run. + /// + /// # Caveat + /// This is only intended to be called from the main thread + fn exit(&self) { + // Relaxed ordering because we don't actually have multiple threads involved, we just want + // interiour mutability + // + // XXX: As an aside; having each individual bit of state for `Handler` be atomic or wrapped in a + // `Mutex` for the sake of interior mutability seems a bit odd, and also a potential foot + // gun in case the state is unwittingly accessed across threads because the fine-grained locking + // wouldn't ensure that there's interior consistency. + // + // Maybe the whole thing should just be put in a static `Mutex<>` to make it clear + // the we can mutate more than one peice of state while maintaining consistency. (though it + // looks like there have been recuring re-entrancy issues with callback handling that might + // make that awkward) + self.running.store(false, Ordering::Relaxed); + *self.control_flow_prev.lock().unwrap() = ControlFlow::default(); + *self.control_flow.lock().unwrap() = ControlFlow::default(); + self.set_stop_app_on_redraw_requested(false); + self.set_stop_app_before_wait(false); + } + + pub fn request_stop_app_on_launch(&self) { + // Relaxed ordering because we don't actually have multiple threads involved, we just want + // interior mutability + self.stop_app_on_launch.store(true, Ordering::Relaxed); + } + + pub fn should_stop_app_on_launch(&self) -> bool { + // Relaxed ordering because we don't actually have multiple threads involved, we just want + // interior mutability + self.stop_app_on_launch.load(Ordering::Relaxed) + } + + pub fn set_stop_app_before_wait(&self, stop_before_wait: bool) { + // Relaxed ordering because we don't actually have multiple threads involved, we just want + // interior mutability + self.stop_app_before_wait + .store(stop_before_wait, Ordering::Relaxed); + } + + pub fn should_stop_app_before_wait(&self) -> bool { + // Relaxed ordering because we don't actually have multiple threads involved, we just want + // interior mutability + self.stop_app_before_wait.load(Ordering::Relaxed) + } + + pub fn set_stop_app_on_redraw_requested(&self, stop_on_redraw: bool) { + // Relaxed ordering because we don't actually have multiple threads involved, we just want + // interior mutability + self.stop_app_on_redraw + .store(stop_on_redraw, Ordering::Relaxed); + } + + pub fn should_stop_app_on_redraw_requested(&self) -> bool { + // Relaxed ordering because we don't actually have multiple threads involved, we just want + // interior mutability + self.stop_app_on_redraw.load(Ordering::Relaxed) + } + fn get_control_flow_and_update_prev(&self) -> ControlFlow { let control_flow = self.control_flow.lock().unwrap(); *self.control_flow_prev.lock().unwrap() = *control_flow; @@ -192,6 +294,10 @@ impl Handler { self.in_callback.store(in_callback, Ordering::Release); } + fn have_callback(&self) -> bool { + self.callback.lock().unwrap().is_some() + } + fn handle_nonuser_event(&self, wrapper: EventWrapper) { if let Some(ref mut callback) = *self.callback.lock().unwrap() { match wrapper { @@ -253,18 +359,63 @@ impl Handler { pub(crate) enum AppState {} impl AppState { - pub fn set_callback(callback: Weak>, window_target: Rc>) { + /// Associate the application's event callback with the (global static) Handler state + /// + /// # Safety + /// This is ignoring the lifetime of the application callback (which may not be 'static) + /// and can lead to undefined behaviour if the callback is not cleared before the end of + /// its real lifetime. + /// + /// All public APIs that take an event callback (`run`, `run_ondemand`, + /// `pump_events`) _must_ pair a call to `set_callback` with + /// a call to `clear_callback` before returning to avoid undefined behaviour. + pub unsafe fn set_callback( + callback: Weak>, + window_target: Rc>, + ) { *HANDLER.callback.lock().unwrap() = Some(Box::new(EventLoopHandler { callback, window_target, })); } + pub fn clear_callback() { + HANDLER.callback.lock().unwrap().take(); + } + + pub fn is_launched() -> bool { + HANDLER.is_launched() + } + + pub fn is_running() -> bool { + HANDLER.is_running() + } + + // If `pump_events` is called to progress the event loop then we bootstrap the event + // loop via `[NSApp run]` but will use `CFRunLoopRunInMode` for subsequent calls to + // `pump_events` + pub fn request_stop_on_launch() { + HANDLER.request_stop_app_on_launch(); + } + + pub fn set_stop_app_before_wait(stop_before_wait: bool) { + HANDLER.set_stop_app_before_wait(stop_before_wait); + } + + pub fn set_stop_app_on_redraw_requested(stop_on_redraw: bool) { + HANDLER.set_stop_app_on_redraw_requested(stop_on_redraw); + } + + pub fn control_flow() -> ControlFlow { + HANDLER.get_old_and_new_control_flow().1 + } + pub fn exit() -> i32 { HANDLER.set_in_callback(true); HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::LoopDestroyed)); HANDLER.set_in_callback(false); - HANDLER.callback.lock().unwrap().take(); + HANDLER.exit(); + Self::clear_callback(); if let ControlFlow::ExitWithCode(code) = HANDLER.get_old_and_new_control_flow().1 { code } else { @@ -272,6 +423,24 @@ impl AppState { } } + pub fn dispatch_init_events() { + HANDLER.set_in_callback(true); + HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::NewEvents( + StartCause::Init, + ))); + // NB: For consistency all platforms must emit a 'resumed' event even though macOS + // applications don't themselves have a formal suspend/resume lifecycle. + HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::Resumed)); + HANDLER.set_in_callback(false); + } + + pub fn start_running() { + debug_assert!(HANDLER.is_launched()); + + HANDLER.set_running(); + Self::dispatch_init_events() + } + pub fn launched( activation_policy: NSApplicationActivationPolicy, create_default_menu: bool, @@ -286,30 +455,42 @@ impl AppState { window_activation_hack(&app); app.activateIgnoringOtherApps(activate_ignoring_other_apps); - HANDLER.set_ready(); + HANDLER.set_launched(); HANDLER.waker().start(); if create_default_menu { // The menubar initialization should be before the `NewEvents` event, to allow // overriding of the default menu even if it's created menu::initialize(); } - HANDLER.set_in_callback(true); - HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::NewEvents( - StartCause::Init, - ))); - // NB: For consistency all platforms must emit a 'resumed' event even though macOS - // applications don't themselves have a formal suspend/resume lifecycle. - HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::Resumed)); - HANDLER.set_in_callback(false); + + Self::start_running(); + + // If the `NSApp` is being launched via `EventLoop::pump_events()` then we'll + // want to stop the app once it is launched (and return to the external loop) + // + // In this case we still want to consider Winit's `EventLoop` to be "running", + // so we call `start_running()` above. + if HANDLER.should_stop_app_on_launch() { + // Note: the original idea had been to only stop the underlying `RunLoop` + // for the app but that didn't work as expected (`[NSApp run]` effectively + // ignored the attempt to stop the RunLoop and re-started it.). So we + // return from `pump_events` by stopping the `NSApp` + Self::stop(); + } } + // Called by RunLoopObserver after finishing waiting for new events pub fn wakeup(panic_info: Weak) { let panic_info = panic_info .upgrade() .expect("The panic info must exist here. This failure indicates a developer error."); // Return when in callback due to https://github.com/rust-windowing/winit/issues/1779 - if panic_info.is_panicking() || !HANDLER.is_ready() || HANDLER.get_in_callback() { + if panic_info.is_panicking() + || !HANDLER.have_callback() + || !HANDLER.is_running() + || HANDLER.get_in_callback() + { return; } let start = HANDLER.get_start_time().unwrap(); @@ -358,6 +539,12 @@ impl AppState { HANDLER .handle_nonuser_event(EventWrapper::StaticEvent(Event::RedrawRequested(window_id))); HANDLER.set_in_callback(false); + + // `pump_events` will request to stop immediately _after_ dispatching RedrawRequested events + // as a way to ensure that `pump_events` can't block an external loop indefinitely + if HANDLER.should_stop_app_on_redraw_requested() { + AppState::stop(); + } } } @@ -368,13 +555,29 @@ impl AppState { HANDLER.events().push_back(wrapper); } + pub fn stop() { + let app = NSApp(); + autoreleasepool(|_| { + app.stop(None); + // To stop event loop immediately, we need to post some event here. + app.postEvent_atStart(&NSEvent::dummy(), true); + }); + } + + // Called by RunLoopObserver before waiting for new events pub fn cleared(panic_info: Weak) { let panic_info = panic_info .upgrade() .expect("The panic info must exist here. This failure indicates a developer error."); // Return when in callback due to https://github.com/rust-windowing/winit/issues/1779 - if panic_info.is_panicking() || !HANDLER.is_ready() || HANDLER.get_in_callback() { + // XXX: how does it make sense that `get_in_callback()` can ever return `true` here if we're + // about to return to the `CFRunLoop` to poll for new events? + if panic_info.is_panicking() + || !HANDLER.have_callback() + || !HANDLER.is_running() + || HANDLER.get_in_callback() + { return; } @@ -384,6 +587,7 @@ impl AppState { HANDLER.handle_nonuser_event(event); } HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::MainEventsCleared)); + for window_id in HANDLER.should_redraw() { HANDLER .handle_nonuser_event(EventWrapper::StaticEvent(Event::RedrawRequested(window_id))); @@ -392,12 +596,11 @@ impl AppState { HANDLER.set_in_callback(false); if HANDLER.should_exit() { - let app = NSApp(); - autoreleasepool(|_| { - app.stop(None); - // To stop event loop immediately, we need to post some event here. - app.postEvent_atStart(&NSEvent::dummy(), true); - }); + Self::stop(); + } + + if HANDLER.should_stop_app_before_wait() { + Self::stop(); } HANDLER.update_start_time(); match HANDLER.get_old_and_new_control_flow() { diff --git a/src/platform_impl/macos/event_loop.rs b/src/platform_impl/macos/event_loop.rs index 93e893de..00f93c4b 100644 --- a/src/platform_impl/macos/event_loop.rs +++ b/src/platform_impl/macos/event_loop.rs @@ -5,7 +5,7 @@ use std::{ marker::PhantomData, mem, os::raw::c_void, - panic::{catch_unwind, resume_unwind, RefUnwindSafe, UnwindSafe}, + panic::{catch_unwind, resume_unwind, AssertUnwindSafe, RefUnwindSafe, UnwindSafe}, process, ptr, rc::{Rc, Weak}, sync::mpsc, @@ -23,9 +23,10 @@ use raw_window_handle::{AppKitDisplayHandle, RawDisplayHandle}; use super::appkit::{NSApp, NSApplicationActivationPolicy, NSEvent, NSWindow}; use crate::{ + error::RunLoopError, event::Event, event_loop::{ControlFlow, EventLoopClosed, EventLoopWindowTarget as RootWindowTarget}, - platform::macos::ActivationPolicy, + platform::{macos::ActivationPolicy, pump_events::PumpStatus}, platform_impl::platform::{ app::WinitApplication, app_delegate::ApplicationDelegate, @@ -195,7 +196,11 @@ impl EventLoop { where F: 'static + FnMut(Event<'_, T>, &RootWindowTarget, &mut ControlFlow), { - let exit_code = self.run_return(callback); + let exit_code = match self.run_ondemand(callback) { + Err(RunLoopError::ExitFailure(code)) => code, + Err(_err) => 1, + Ok(_) => 0, + }; process::exit(exit_code); } @@ -203,10 +208,34 @@ impl EventLoop { where F: FnMut(Event<'_, T>, &RootWindowTarget, &mut ControlFlow), { - // This transmute is always safe, in case it was reached through `run`, since our - // lifetime will be already 'static. In other cases caller should ensure that all data - // they passed to callback will actually outlive it, some apps just can't move - // everything to event loop, so this is something that they should care about. + match self.run_ondemand(callback) { + Err(RunLoopError::ExitFailure(code)) => code, + Err(_err) => 1, + Ok(_) => 0, + } + } + + // NB: we don't base this on `pump_events` because for `MacOs` we can't support + // `pump_events` elegantly (we just ask to run the loop for a "short" amount of + // time and so a layered implementation would end up using a lot of CPU due to + // redundant wake ups. + pub fn run_ondemand(&mut self, callback: F) -> Result<(), RunLoopError> + where + F: FnMut(Event<'_, T>, &RootWindowTarget, &mut ControlFlow), + { + if AppState::is_running() { + return Err(RunLoopError::AlreadyRunning); + } + + // # Safety + // We are erasing the lifetime of the application callback here so that we + // can (temporarily) store it within 'static global `AppState` that's + // accessible to objc delegate callbacks. + // + // The safety of this depends on on making sure to also clear the callback + // from the global `AppState` before we return from here, ensuring that + // we don't retain a reference beyond the real lifetime of the callback. + let callback = unsafe { mem::transmute::< Rc, &RootWindowTarget, &mut ControlFlow)>>, @@ -224,18 +253,151 @@ impl EventLoop { let weak_cb: Weak<_> = Rc::downgrade(&callback); drop(callback); - AppState::set_callback(weak_cb, Rc::clone(&self.window_target)); - unsafe { app.run() }; - - if let Some(panic) = self.panic_info.take() { - drop(self._callback.take()); - resume_unwind(panic); + // # Safety + // We make sure to call `AppState::clear_callback` before returning + unsafe { + AppState::set_callback(weak_cb, Rc::clone(&self.window_target)); } - AppState::exit() - }); - drop(self._callback.take()); - exit_code + // catch panics to make sure we can't unwind without clearing the set callback + // (which would leave the global `AppState` in an undefined, unsafe state) + let catch_result = catch_unwind(AssertUnwindSafe(|| { + if AppState::is_launched() { + debug_assert!(!AppState::is_running()); + AppState::start_running(); // Set is_running = true + dispatch `NewEvents(Init)` + `Resumed` + } + AppState::set_stop_app_before_wait(false); + unsafe { app.run() }; + + // While the app is running it's possible that we catch a panic + // to avoid unwinding across an objective-c ffi boundary, which + // will lead to us stopping the `NSApp` and saving the + // `PanicInfo` so that we can resume the unwind at a controlled, + // safe point in time. + if let Some(panic) = self.panic_info.take() { + resume_unwind(panic); + } + + AppState::exit() + })); + + // # Safety + // This pairs up with the `unsafe` call to `set_callback` above and ensures that + // we always clear the application callback from the global `AppState` before + // returning + drop(self._callback.take()); + AppState::clear_callback(); + + match catch_result { + Ok(exit_code) => exit_code, + Err(payload) => resume_unwind(payload), + } + }); + + if exit_code == 0 { + Ok(()) + } else { + Err(RunLoopError::ExitFailure(exit_code)) + } + } + + pub fn pump_events(&mut self, callback: F) -> PumpStatus + where + F: FnMut(Event<'_, T>, &RootWindowTarget, &mut ControlFlow), + { + // # Safety + // We are erasing the lifetime of the application callback here so that we + // can (temporarily) store it within 'static global `AppState` that's + // accessible to objc delegate callbacks. + // + // The safety of this depends on on making sure to also clear the callback + // from the global `AppState` before we return from here, ensuring that + // we don't retain a reference beyond the real lifetime of the callback. + + let callback = unsafe { + mem::transmute::< + Rc, &RootWindowTarget, &mut ControlFlow)>>, + Rc, &RootWindowTarget, &mut ControlFlow)>>, + >(Rc::new(RefCell::new(callback))) + }; + + self._callback = Some(Rc::clone(&callback)); + + autoreleasepool(|_| { + let app = NSApp(); + + // A bit of juggling with the callback references to make sure + // that `self.callback` is the only owner of the callback. + let weak_cb: Weak<_> = Rc::downgrade(&callback); + drop(callback); + + // # Safety + // We will make sure to call `AppState::clear_callback` before returning + // to ensure that we don't hold on to the callback beyond its (erased) + // lifetime + unsafe { + AppState::set_callback(weak_cb, Rc::clone(&self.window_target)); + } + + // catch panics to make sure we can't unwind without clearing the set callback + // (which would leave the global `AppState` in an undefined, unsafe state) + let catch_result = catch_unwind(AssertUnwindSafe(|| { + // As a special case, if the `NSApp` hasn't been launched yet then we at least run + // the loop until it has fully launched. + if !AppState::is_launched() { + debug_assert!(!AppState::is_running()); + + AppState::request_stop_on_launch(); + unsafe { + app.run(); + } + + // Note: we dispatch `NewEvents(Init)` + `Resumed` events after the `NSApp` has launched + } else if !AppState::is_running() { + // Even though the NSApp may have been launched, it's possible we aren't running + // if the `EventLoop` was run before and has since exited. This indicates that + // we just starting to re-run the same `EventLoop` again. + AppState::start_running(); // Set is_running = true + dispatch `NewEvents(Init)` + `Resumed` + } else { + // Make sure we can't block any external loop indefinitely by stopping the NSApp + // and returning after dispatching any `RedrawRequested` event or whenever the + // `RunLoop` needs to wait for new events from the OS + AppState::set_stop_app_on_redraw_requested(true); + AppState::set_stop_app_before_wait(true); + unsafe { + app.run(); + } + } + + // While the app is running it's possible that we catch a panic + // to avoid unwinding across an objective-c ffi boundary, which + // will lead to us stopping the `NSApp` and saving the + // `PanicInfo` so that we can resume the unwind at a controlled, + // safe point in time. + if let Some(panic) = self.panic_info.take() { + resume_unwind(panic); + } + + if let ControlFlow::ExitWithCode(code) = AppState::control_flow() { + AppState::exit(); + PumpStatus::Exit(code) + } else { + PumpStatus::Continue + } + })); + + // # Safety + // This pairs up with the `unsafe` call to `set_callback` above and ensures that + // we always clear the application callback from the global `AppState` before + // returning + AppState::clear_callback(); + drop(self._callback.take()); + + match catch_result { + Ok(pump_status) => pump_status, + Err(payload) => resume_unwind(payload), + } + }) } pub fn create_proxy(&self) -> EventLoopProxy {