diff --git a/src/platform_impl/apple/appkit/app_state.rs b/src/platform_impl/apple/appkit/app_state.rs index b20181b2..6ebcbf1f 100644 --- a/src/platform_impl/apple/appkit/app_state.rs +++ b/src/platform_impl/apple/appkit/app_state.rs @@ -1,6 +1,6 @@ use std::cell::{Cell, OnceCell, RefCell}; use std::mem; -use std::rc::{Rc, Weak}; +use std::rc::Rc; use std::sync::Arc; use std::time::Instant; @@ -15,7 +15,7 @@ use winit_core::window::WindowId; use super::super::event_handler::EventHandler; use super::super::event_loop_proxy::EventLoopProxy; -use super::event_loop::{notify_windows_of_exit, stop_app_immediately, ActiveEventLoop, PanicInfo}; +use super::event_loop::{notify_windows_of_exit, stop_app_immediately, ActiveEventLoop}; use super::menu; use super::observer::{EventLoopWaker, RunLoop}; @@ -309,13 +309,9 @@ impl AppState { } // Called by RunLoopObserver after finishing waiting for new events - pub fn wakeup(self: &Rc, panic_info: Weak) { - let panic_info = panic_info - .upgrade() - .expect("The panic info must exist here. This failure indicates a developer error."); - + pub fn wakeup(self: &Rc) { // Return when in event handler due to https://github.com/rust-windowing/winit/issues/1779 - if panic_info.is_panicking() || !self.event_handler.ready() || !self.is_running() { + if !self.event_handler.ready() || !self.is_running() { return; } @@ -341,15 +337,11 @@ impl AppState { } // Called by RunLoopObserver before waiting for new events - pub fn cleared(self: &Rc, panic_info: Weak) { - let panic_info = panic_info - .upgrade() - .expect("The panic info must exist here. This failure indicates a developer error."); - + pub fn cleared(self: &Rc) { // Return when in event handler due to https://github.com/rust-windowing/winit/issues/1779 // XXX: how does it make sense that `event_handler.ready()` can ever return `false` here if // we're about to return to the `CFRunLoop` to poll for new events? - if panic_info.is_panicking() || !self.event_handler.ready() || !self.is_running() { + if !self.event_handler.ready() || !self.is_running() { return; } diff --git a/src/platform_impl/apple/appkit/event_loop.rs b/src/platform_impl/apple/appkit/event_loop.rs index 063ed98a..e360da64 100644 --- a/src/platform_impl/apple/appkit/event_loop.rs +++ b/src/platform_impl/apple/appkit/event_loop.rs @@ -1,8 +1,4 @@ -use std::any::Any; -use std::cell::Cell; -use std::fmt; -use std::panic::{catch_unwind, resume_unwind, RefUnwindSafe, UnwindSafe}; -use std::rc::{Rc, Weak}; +use std::rc::Rc; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -36,42 +32,6 @@ use super::observer::setup_control_flow_observers; use crate::platform::macos::ActivationPolicy; use crate::platform_impl::Window; -#[derive(Default)] -pub struct PanicInfo { - inner: Cell>>, -} - -impl fmt::Debug for PanicInfo { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("PanicInfo").finish_non_exhaustive() - } -} - -// WARNING: -// As long as this struct is used through its `impl`, it is UnwindSafe. -// (If `get_mut` is called on `inner`, unwind safety may get broken.) -impl UnwindSafe for PanicInfo {} -impl RefUnwindSafe for PanicInfo {} -impl PanicInfo { - pub fn is_panicking(&self) -> bool { - let inner = self.inner.take(); - let result = inner.is_some(); - self.inner.set(inner); - result - } - - /// Overwrites the current state if the current state is not panicking - pub fn set_panic(&self, p: Box) { - if !self.is_panicking() { - self.inner.set(Some(p)); - } - } - - pub fn take(&self) -> Option> { - self.inner.take() - } -} - #[derive(Debug)] pub struct ActiveEventLoop { pub(super) app_state: Rc, @@ -183,7 +143,6 @@ pub struct EventLoop { app_state: Rc, window_target: ActiveEventLoop, - panic_info: Rc, // Since macOS 10.11, we no longer need to remove the observers before they are deallocated; // the system instead cleans it up next time it would have posted a notification to it. @@ -259,14 +218,12 @@ impl EventLoop { }, ); - let panic_info: Rc = Default::default(); - setup_control_flow_observers(mtm, Rc::downgrade(&panic_info)); + setup_control_flow_observers(mtm); Ok(EventLoop { app, app_state: app_state.clone(), window_target: ActiveEventLoop { app_state, mtm }, - panic_info, _did_finish_launching_observer, _will_terminate_observer, }) @@ -306,15 +263,6 @@ impl EventLoop { // NOTE: Make sure to not run the application re-entrantly, as that'd be confusing. self.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 `NSApplication` 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); - } - self.app_state.internal_exit() }) }); @@ -370,15 +318,6 @@ impl EventLoop { self.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 application 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 self.app_state.exiting() { self.app_state.internal_exit(); PumpStatus::Exit(0) @@ -423,29 +362,3 @@ pub(super) fn notify_windows_of_exit(app: &NSApplication) { window.close(); } } - -/// Catches panics that happen inside `f` and when a panic -/// happens, stops the `sharedApplication` -#[inline] -pub fn stop_app_on_panic R + UnwindSafe, R>( - mtm: MainThreadMarker, - panic_info: Weak, - f: F, -) -> Option { - match catch_unwind(f) { - Ok(r) => Some(r), - Err(e) => { - // It's important that we set the panic before requesting a `stop` - // because some callback are still called during the `stop` message - // and we need to know in those callbacks if the application is currently - // panicking - { - let panic_info = panic_info.upgrade().unwrap(); - panic_info.set_panic(e); - } - let app = NSApplication::sharedApplication(mtm); - stop_app_immediately(&app); - None - }, - } -} diff --git a/src/platform_impl/apple/appkit/observer.rs b/src/platform_impl/apple/appkit/observer.rs index ae836208..3629187c 100644 --- a/src/platform_impl/apple/appkit/observer.rs +++ b/src/platform_impl/apple/appkit/observer.rs @@ -4,9 +4,7 @@ //! use std::cell::Cell; use std::ffi::c_void; -use std::panic::{AssertUnwindSafe, UnwindSafe}; use std::ptr; -use std::rc::Weak; use std::time::Instant; use objc2::MainThreadMarker; @@ -18,47 +16,18 @@ use objc2_core_foundation::{ use tracing::error; use super::app_state::AppState; -use super::event_loop::{stop_app_on_panic, PanicInfo}; - -unsafe fn control_flow_handler(panic_info: *mut c_void, f: F) -where - F: FnOnce(Weak) + UnwindSafe, -{ - let info_from_raw = unsafe { Weak::from_raw(panic_info as *mut PanicInfo) }; - // Asserting unwind safety on this type should be fine because `PanicInfo` is - // `RefUnwindSafe` and `Rc` is `UnwindSafe` if `T` is `RefUnwindSafe`. - let panic_info = AssertUnwindSafe(Weak::clone(&info_from_raw)); - // `from_raw` takes ownership of the data behind the pointer. - // But if this scope takes ownership of the weak pointer, then - // the weak pointer will get free'd at the end of the scope. - // However we want to keep that weak reference around after the function. - std::mem::forget(info_from_raw); - - let mtm = MainThreadMarker::new().unwrap(); - stop_app_on_panic(mtm, Weak::clone(&panic_info), move || { - let _ = &panic_info; - f(panic_info.0) - }); -} // begin is queued with the highest priority to ensure it is processed before other observers extern "C-unwind" fn control_flow_begin_handler( _: *mut CFRunLoopObserver, activity: CFRunLoopActivity, - panic_info: *mut c_void, + _info: *mut c_void, ) { - unsafe { - control_flow_handler(panic_info, |panic_info| { - #[allow(non_upper_case_globals)] - match activity { - CFRunLoopActivity::AfterWaiting => { - // trace!("Triggered `CFRunLoopAfterWaiting`"); - AppState::get(MainThreadMarker::new().unwrap()).wakeup(panic_info); - // trace!("Completed `CFRunLoopAfterWaiting`"); - }, - _ => unreachable!(), - } - }); + match activity { + CFRunLoopActivity::AfterWaiting => { + AppState::get(MainThreadMarker::new().unwrap()).wakeup(); + }, + _ => unreachable!(), } } @@ -67,21 +36,14 @@ extern "C-unwind" fn control_flow_begin_handler( extern "C-unwind" fn control_flow_end_handler( _: *mut CFRunLoopObserver, activity: CFRunLoopActivity, - panic_info: *mut c_void, + _info: *mut c_void, ) { - unsafe { - control_flow_handler(panic_info, |panic_info| { - #[allow(non_upper_case_globals)] - match activity { - CFRunLoopActivity::BeforeWaiting => { - // trace!("Triggered `CFRunLoopBeforeWaiting`"); - AppState::get(MainThreadMarker::new().unwrap()).cleared(panic_info); - // trace!("Completed `CFRunLoopBeforeWaiting`"); - }, - CFRunLoopActivity::Exit => (), /* unimplemented!(), // not expected to ever happen */ - _ => unreachable!(), - } - }); + match activity { + CFRunLoopActivity::BeforeWaiting => { + AppState::get(MainThreadMarker::new().unwrap()).cleared(); + }, + CFRunLoopActivity::Exit => (), // unimplemented!(), // not expected to ever happen + _ => unreachable!(), } } @@ -180,11 +142,11 @@ impl RunLoop { } } -pub fn setup_control_flow_observers(mtm: MainThreadMarker, panic_info: Weak) { +pub fn setup_control_flow_observers(mtm: MainThreadMarker) { let run_loop = RunLoop::main(mtm); unsafe { let mut context = CFRunLoopObserverContext { - info: Weak::into_raw(panic_info) as *mut _, + info: ptr::null_mut(), version: 0, retain: None, release: None,