leyoda/winit
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

macOS: Remove panic wrapper (#4147)

Browse source 
This is unnecessary nowadays, unwinding in CF observer callbacks is safe
(and is safe in Rust after the introduction of `extern "C-unwind"`).

Panicking elsewhere (such as in NSNotificationCenter callbacks or
delegate methods) _may_ still lead to an abort, if AppKit tries to catch
it with libc++, since Rust panics are not compatible with those.
That's "just" a quality-of-implementation detail of current Rust though,
not an inherent limitation, and should really be solved in rustc.
This commit is contained in:
Mads Marquart 2025-05-23 15:53:12 +02:00 • committed by GitHub
parent f51a470872
commit 3e50911adb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 23 additions and 156 deletions
Download patch file Download diff file Expand all files Collapse all files

20
src/platform_impl/apple/appkit/app_state.rs
View file

@ -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<Self>, panic_info: Weak<PanicInfo>) {
let panic_info = panic_info
.upgrade()
.expect("The panic info must exist here. This failure indicates a developer error.");
pub fn wakeup(self: &Rc<Self>) {
// 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<Self>, panic_info: Weak<PanicInfo>) {
let panic_info = panic_info
.upgrade()
.expect("The panic info must exist here. This failure indicates a developer error.");
pub fn cleared(self: &Rc<Self>) {
// 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;
}

91
src/platform_impl/apple/appkit/event_loop.rs
View file

@ -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<Option<Box<dyn Any + Send + 'static>>>,
}
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<dyn Any + Send + 'static>) {
if !self.is_panicking() {
self.inner.set(Some(p));
}
}
pub fn take(&self) -> Option<Box<dyn Any + Send + 'static>> {
self.inner.take()
}
}
#[derive(Debug)]
pub struct ActiveEventLoop {
pub(super) app_state: Rc<AppState>,
@ -183,7 +143,6 @@ pub struct EventLoop {
app_state: Rc<AppState>,
window_target: ActiveEventLoop,
panic_info: Rc<PanicInfo>,
// 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<PanicInfo> = 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<F: FnOnce() -> R + UnwindSafe, R>(
mtm: MainThreadMarker,
panic_info: Weak<PanicInfo>,
f: F,
) -> Option<R> {
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
},
}
}

68
src/platform_impl/apple/appkit/observer.rs
View file

@ -4,9 +4,7 @@
//! <https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/RunLoopManagement/RunLoopManagement.html>
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<F>(panic_info: *mut c_void, f: F)
where
F: FnOnce(Weak<PanicInfo>) + 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<T>` 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<PanicInfo>) {
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,