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

macOS: Refactor event handler storage (#3532)

Browse source 
This makes our use of `unsafe` to make the event handler temporarily 'static be local to a module, in a way that's (hopefully) much easier to reason about.
This commit is contained in:
Mads Marquart 2024-02-28 04:33:47 +01:00 • committed by GitHub
parent a4480a0652
commit a5dbd3ee52
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 213 additions and 234 deletions
Download patch file Download diff file Expand all files Collapse all files

158
src/platform_impl/macos/app_delegate.rs
View file

@ -1,7 +1,7 @@
use std::cell::{Cell, RefCell};
use std::collections::VecDeque;
use std::mem;
use std::rc::{Rc, Weak};
use std::rc::Weak;
use std::sync::{Arc, Mutex};
use std::time::Instant;
@ -11,13 +11,14 @@ use objc2::rc::Id;
use objc2::runtime::AnyObject;
use objc2::{declare_class, msg_send_id, mutability, ClassType, DeclaredClass};
use super::event_loop::{stop_app_immediately, PanicInfo};
use super::event_handler::EventHandler;
use super::event_loop::{stop_app_immediately, ActiveEventLoop, PanicInfo};
use super::observer::{EventLoopWaker, RunLoop};
use super::window::WinitWindow;
use super::{menu, WindowId, DEVICE_ID};
use crate::dpi::PhysicalSize;
use crate::event::{DeviceEvent, Event, InnerSizeWriter, StartCause, WindowEvent};
use crate::event_loop::{ActiveEventLoop as RootWindowTarget, ControlFlow};
use crate::event_loop::{ActiveEventLoop as RootActiveEventLoop, ControlFlow};
use crate::window::WindowId as RootWindowId;
#[derive(Debug, Default)]
@ -25,10 +26,7 @@ pub(super) struct State {
activation_policy: NSApplicationActivationPolicy,
default_menu: bool,
activate_ignoring_other_apps: bool,
/// Whether the application is currently executing a callback.
in_callback: Cell<bool>,
/// The lifetime-erased callback.
callback: RefCell<Option<EventLoopHandler>>,
event_handler: EventHandler,
stop_on_launch: Cell<bool>,
stop_before_wait: Cell<bool>,
stop_after_wait: Cell<bool>,
@ -146,34 +144,14 @@ impl ApplicationDelegate {
}
}
/// Associate the application's event callback with the application delegate.
///
/// # 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_on_demand`,
/// `pump_events`) _must_ pair a call to `set_callback` with
/// a call to `clear_callback` before returning to avoid undefined behaviour.
#[allow(clippy::type_complexity)]
pub unsafe fn set_callback(
/// Place the event handler in the application delegate for the duration
/// of the given closure.
pub fn set_event_handler<R>(
&self,
callback: Weak<RefCell<dyn FnMut(Event<HandlePendingUserEvents>, &RootWindowTarget)>>,
window_target: Rc<RootWindowTarget>,
) {
*self.ivars().callback.borrow_mut() = Some(EventLoopHandler {
callback,
window_target,
});
}
pub fn clear_callback(&self) {
self.ivars().callback.borrow_mut().take();
}
fn have_callback(&self) -> bool {
self.ivars().callback.borrow().is_some()
handler: impl FnMut(Event<HandlePendingUserEvents>, &RootActiveEventLoop),
closure: impl FnOnce() -> R,
) -> R {
self.ivars().event_handler.set(handler, closure)
}
/// If `pump_events` is called to progress the event loop then we
@ -204,16 +182,13 @@ impl ApplicationDelegate {
/// Note: that if the `NSApplication` has been launched then that state is preserved,
/// and we won't need to re-launch the app if subsequent EventLoops are run.
pub fn internal_exit(&self) {
self.set_in_callback(true);
self.handle_event(Event::LoopExiting);
self.set_in_callback(false);
self.set_is_running(false);
self.set_stop_on_redraw(false);
self.set_stop_before_wait(false);
self.set_stop_after_wait(false);
self.set_wait_timeout(None);
self.clear_callback();
}
pub fn is_launched(&self) -> bool {
@ -240,10 +215,6 @@ impl ApplicationDelegate {
self.ivars().exit.get()
}
fn set_in_callback(&self, value: bool) {
self.ivars().in_callback.set(value)
}
pub fn set_control_flow(&self, value: ControlFlow) {
self.ivars().control_flow.set(value)
}
@ -285,13 +256,12 @@ impl ApplicationDelegate {
pub fn handle_redraw(&self, window_id: WindowId) {
let mtm = MainThreadMarker::from(self);
// Redraw request might come out of order from the OS.
// -> Don't go back into the callback when our callstack originates from there
if !self.ivars().in_callback.get() {
// -> Don't go back into the event handler when our callstack originates from there
if !self.ivars().event_handler.in_use() {
self.handle_event(Event::WindowEvent {
window_id: RootWindowId(window_id),
event: WindowEvent::RedrawRequested,
});
self.ivars().in_callback.set(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
@ -311,19 +281,17 @@ impl ApplicationDelegate {
}
fn handle_event(&self, event: Event<HandlePendingUserEvents>) {
if let Some(ref mut callback) = *self.ivars().callback.borrow_mut() {
callback.handle_event(event)
}
self.ivars()
.event_handler
.handle_event(event, &ActiveEventLoop::new_root(self.retain()))
}
/// dispatch `NewEvents(Init)` + `Resumed`
pub fn dispatch_init_events(&self) {
self.set_in_callback(true);
self.handle_event(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.
self.handle_event(Event::Resumed);
self.set_in_callback(false);
}
// Called by RunLoopObserver after finishing waiting for new events
@ -333,12 +301,8 @@ impl ApplicationDelegate {
.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()
|| self.ivars().in_callback.get()
|| !self.have_callback()
|| !self.is_running()
{
// Return when in event handler due to https://github.com/rust-windowing/winit/issues/1779
if panic_info.is_panicking() || !self.ivars().event_handler.ready() || !self.is_running() {
return;
}
@ -369,9 +333,7 @@ impl ApplicationDelegate {
}
};
self.set_in_callback(true);
self.handle_event(Event::NewEvents(cause));
self.set_in_callback(false);
}
// Called by RunLoopObserver before waiting for new events
@ -381,18 +343,13 @@ impl ApplicationDelegate {
.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
// XXX: how does it make sense that `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()
|| self.ivars().in_callback.get()
|| !self.have_callback()
|| !self.is_running()
{
// 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.ivars().event_handler.ready() || !self.is_running() {
return;
}
self.set_in_callback(true);
self.handle_event(Event::UserEvent(HandlePendingUserEvents));
let events = mem::take(&mut *self.ivars().pending_events.borrow_mut());
@ -415,32 +372,30 @@ impl ApplicationDelegate {
suggested_size,
scale_factor,
} => {
if let Some(ref mut callback) = *self.ivars().callback.borrow_mut() {
let new_inner_size = Arc::new(Mutex::new(suggested_size));
let scale_factor_changed_event = Event::WindowEvent {
window_id: RootWindowId(window.id()),
event: WindowEvent::ScaleFactorChanged {
scale_factor,
inner_size_writer: InnerSizeWriter::new(Arc::downgrade(
&new_inner_size,
)),
},
};
let new_inner_size = Arc::new(Mutex::new(suggested_size));
let scale_factor_changed_event = Event::WindowEvent {
window_id: RootWindowId(window.id()),
event: WindowEvent::ScaleFactorChanged {
scale_factor,
inner_size_writer: InnerSizeWriter::new(Arc::downgrade(
&new_inner_size,
)),
},
};
callback.handle_event(scale_factor_changed_event);
self.handle_event(scale_factor_changed_event);
let physical_size = *new_inner_size.lock().unwrap();
drop(new_inner_size);
let logical_size = physical_size.to_logical(scale_factor);
let size = NSSize::new(logical_size.width, logical_size.height);
window.setContentSize(size);
let physical_size = *new_inner_size.lock().unwrap();
drop(new_inner_size);
let logical_size = physical_size.to_logical(scale_factor);
let size = NSSize::new(logical_size.width, logical_size.height);
window.setContentSize(size);
let resized_event = Event::WindowEvent {
window_id: RootWindowId(window.id()),
event: WindowEvent::Resized(physical_size),
};
callback.handle_event(resized_event);
}
let resized_event = Event::WindowEvent {
window_id: RootWindowId(window.id()),
event: WindowEvent::Resized(physical_size),
};
self.handle_event(resized_event);
}
}
}
@ -454,7 +409,6 @@ impl ApplicationDelegate {
}
self.handle_event(Event::AboutToWait);
self.set_in_callback(false);
if self.exiting() {
let app = NSApplication::sharedApplication(mtm);
@ -493,30 +447,6 @@ pub(crate) enum QueuedEvent {
#[derive(Debug)]
pub(crate) struct HandlePendingUserEvents;
#[derive(Debug)]
struct EventLoopHandler {
#[allow(clippy::type_complexity)]
callback: Weak<RefCell<dyn FnMut(Event<HandlePendingUserEvents>, &RootWindowTarget)>>,
window_target: Rc<RootWindowTarget>,
}
impl EventLoopHandler {
fn handle_event(&mut self, event: Event<HandlePendingUserEvents>) {
// `NSApplication` and our app delegate 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 `NSApplication` after exiting a Winit `EventLoop`
if let Some(callback) = self.callback.upgrade() {
let mut callback = callback.borrow_mut();
(callback)(event, &self.window_target);
}
}
}
/// Returns the minimum `Option<Instant>`, taking into account that `None`
/// equates to an infinite timeout, not a zero timeout (so can't just use
/// `Option::min`)

136
src/platform_impl/macos/event_handler.rs Normal file
View file

@ -0,0 +1,136 @@
use std::cell::RefCell;
use std::fmt;
use std::mem;
use super::app_delegate::HandlePendingUserEvents;
use crate::event::Event;
use crate::event_loop::ActiveEventLoop as RootActiveEventLoop;
struct EventHandlerData {
#[allow(clippy::type_complexity)]
handler: Box<dyn FnMut(Event<HandlePendingUserEvents>, &RootActiveEventLoop) + 'static>,
}
impl fmt::Debug for EventHandlerData {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("EventHandlerData").finish_non_exhaustive()
}
}
#[derive(Debug, Default)]
pub(crate) struct EventHandler {
/// This can be in the following states:
/// - Not registered by the event loop (None).
/// - Present (Some(handler)).
/// - Currently executing the handler / in use (RefCell borrowed).
inner: RefCell<Option<EventHandlerData>>,
}
impl EventHandler {
/// Set the event loop handler for the duration of the given closure.
///
/// This is similar to using the `scoped-tls` or `scoped-tls-hkt` crates
/// to store the handler in a thread local, such that it can be accessed
/// from within the closure.
pub(crate) fn set<'handler, R>(
&self,
handler: impl FnMut(Event<HandlePendingUserEvents>, &RootActiveEventLoop) + 'handler,
closure: impl FnOnce() -> R,
) -> R {
// SAFETY: We extend the lifetime of the handler here so that we can
// store it in `EventHandler`'s `RefCell`.
//
// This is sound, since we make sure to unset the handler again at the
// end of this function, and as such the lifetime isn't actually
// extended beyond `'handler`.
let handler = unsafe {
mem::transmute::<
Box<dyn FnMut(Event<HandlePendingUserEvents>, &RootActiveEventLoop) + 'handler>,
Box<dyn FnMut(Event<HandlePendingUserEvents>, &RootActiveEventLoop) + 'static>,
>(Box::new(handler))
};
match self.inner.try_borrow_mut().as_deref_mut() {
Ok(Some(_)) => {
unreachable!("tried to set handler while another was already set");
}
Ok(data @ None) => {
*data = Some(EventHandlerData { handler });
}
Err(_) => {
unreachable!("tried to set handler that is currently in use");
}
}
struct ClearOnDrop<'a>(&'a EventHandler);
impl Drop for ClearOnDrop<'_> {
fn drop(&mut self) {
match self.0.inner.try_borrow_mut().as_deref_mut() {
Ok(data @ Some(_)) => {
*data = None;
}
Ok(None) => {
log::error!("tried to clear handler, but no handler was set");
}
Err(_) => {
// Note: This is not expected to ever happen, this
// module generally controls the `RefCell`, and
// prevents it from ever being borrowed outside of it.
//
// But if it _does_ happen, it is a serious error, and
// we must abort the process, it'd be unsound if we
// weren't able to unset the handler.
eprintln!("tried to clear handler that is currently in use");
std::process::abort();
}
}
}
}
let _clear_on_drop = ClearOnDrop(self);
// Note: The RefCell should not be borrowed while executing the
// closure, that'd defeat the whole point.
closure()
// `_clear_on_drop` will be dropped here, or when unwinding, ensuring
// soundness.
}
pub(crate) fn in_use(&self) -> bool {
self.inner.try_borrow().is_err()
}
pub(crate) fn ready(&self) -> bool {
matches!(self.inner.try_borrow().as_deref(), Ok(Some(_)))
}
pub(crate) fn handle_event(
&self,
event: Event<HandlePendingUserEvents>,
event_loop: &RootActiveEventLoop,
) {
match self.inner.try_borrow_mut().as_deref_mut() {
Ok(Some(EventHandlerData { handler })) => {
// It is important that we keep the reference borrowed here,
// so that `in_use` can properly detect that the handler is
// still in use.
//
// If the handler unwinds, the `RefMut` will ensure that the
// handler is no longer borrowed.
(handler)(event, event_loop);
}
Ok(None) => {
// `NSApplication`, our app delegate and this handler are all
// global state and so it's not impossible that we could get
// an event after the application has exited the `EventLoop`.
log::error!("tried to run event handler, but no handler was set");
}
Err(_) => {
// Prevent re-entrancy.
panic!("tried to handle event while another event is currently being handled");
}
}
}
}

152
src/platform_impl/macos/event_loop.rs
View file

@ -1,11 +1,10 @@
use std::{
any::Any,
cell::{Cell, RefCell},
cell::Cell,
collections::VecDeque,
marker::PhantomData,
mem,
os::raw::c_void,
panic::{catch_unwind, resume_unwind, AssertUnwindSafe, RefUnwindSafe, UnwindSafe},
panic::{catch_unwind, resume_unwind, RefUnwindSafe, UnwindSafe},
ptr,
rc::{Rc, Weak},
sync::mpsc,
@ -79,6 +78,15 @@ pub struct ActiveEventLoop {
}
impl ActiveEventLoop {
pub(super) fn new_root(delegate: Id<ApplicationDelegate>) -> RootWindowTarget {
let mtm = MainThreadMarker::from(&*delegate);
let p = Self { delegate, mtm };
RootWindowTarget {
p,
_marker: PhantomData,
}
}
pub fn create_custom_cursor(&self, source: CustomCursorSource) -> RootCustomCursor {
RootCustomCursor {
inner: CustomCursor::new(source.inner),
@ -188,17 +196,8 @@ pub struct EventLoop<T: 'static> {
sender: mpsc::Sender<T>,
receiver: Rc<mpsc::Receiver<T>>,
window_target: Rc<RootWindowTarget>,
window_target: RootWindowTarget,
panic_info: Rc<PanicInfo>,
/// We make sure that the callback closure is dropped during a panic
/// by making the event loop own it.
///
/// Every other reference should be a Weak reference which is only upgraded
/// into a strong reference in order to call the callback but then the
/// strong reference should be dropped as soon as possible.
#[allow(clippy::type_complexity)]
_callback: Option<Rc<RefCell<dyn FnMut(Event<HandlePendingUserEvents>, &RootWindowTarget)>>>,
}
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
@ -257,12 +256,11 @@ impl<T> EventLoop<T> {
delegate: delegate.clone(),
sender,
receiver: Rc::new(receiver),
window_target: Rc::new(RootWindowTarget {
window_target: RootWindowTarget {
p: ActiveEventLoop { delegate, mtm },
_marker: PhantomData,
}),
},
panic_info,
_callback: None,
})
}
@ -270,56 +268,25 @@ impl<T> EventLoop<T> {
&self.window_target
}
pub fn run<F>(mut self, callback: F) -> Result<(), EventLoopError>
pub fn run<F>(mut self, handler: F) -> Result<(), EventLoopError>
where
F: FnMut(Event<T>, &RootWindowTarget),
{
self.run_on_demand(callback)
self.run_on_demand(handler)
}
// 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_on_demand<F>(&mut self, callback: F) -> Result<(), EventLoopError>
pub fn run_on_demand<F>(&mut self, handler: F) -> Result<(), EventLoopError>
where
F: FnMut(Event<T>, &RootWindowTarget),
{
let callback = map_user_event(callback, self.receiver.clone());
let handler = map_user_event(handler, self.receiver.clone());
// # Safety
// We are erasing the lifetime of the application callback here so that we
// can (temporarily) store it within 'static app delegate that's
// accessible to objc delegate callbacks.
//
// The safety of this depends on on making sure to also clear the callback
// from the app delegate 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<RefCell<dyn FnMut(Event<HandlePendingUserEvents>, &RootWindowTarget)>>,
Rc<RefCell<dyn FnMut(Event<HandlePendingUserEvents>, &RootWindowTarget)>>,
>(Rc::new(RefCell::new(callback)))
};
self._callback = Some(Rc::clone(&callback));
autoreleasepool(|_| {
// 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 make sure to call `delegate.clear_callback` before returning
unsafe {
self.delegate
.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 app delegate in an undefined, unsafe state)
let catch_result = catch_unwind(AssertUnwindSafe(|| {
self.delegate.set_event_handler(handler, || {
autoreleasepool(|_| {
// clear / normalize pump_events state
self.delegate.set_wait_timeout(None);
self.delegate.set_stop_before_wait(false);
@ -331,6 +298,8 @@ impl<T> EventLoop<T> {
self.delegate.set_is_running(true);
self.delegate.dispatch_init_events();
}
// SAFETY: We do not run the application re-entrantly
unsafe { self.app.run() };
// While the app is running it's possible that we catch a panic
@ -343,73 +312,28 @@ impl<T> EventLoop<T> {
}
self.delegate.internal_exit()
}));
// # Safety
// This pairs up with the `unsafe` call to `set_callback` above and ensures that
// we always clear the application callback from the app delegate before returning.
drop(self._callback.take());
self.delegate.clear_callback();
if let Err(payload) = catch_result {
resume_unwind(payload)
}
})
});
Ok(())
}
pub fn pump_events<F>(&mut self, timeout: Option<Duration>, callback: F) -> PumpStatus
pub fn pump_events<F>(&mut self, timeout: Option<Duration>, handler: F) -> PumpStatus
where
F: FnMut(Event<T>, &RootWindowTarget),
{
let callback = map_user_event(callback, self.receiver.clone());
let handler = map_user_event(handler, self.receiver.clone());
// # Safety
// We are erasing the lifetime of the application callback here so that we
// can (temporarily) store it within 'static global app delegate that's
// accessible to objc delegate callbacks.
//
// The safety of this depends on on making sure to also clear the callback
// from the app delegate 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<RefCell<dyn FnMut(Event<HandlePendingUserEvents>, &RootWindowTarget)>>,
Rc<RefCell<dyn FnMut(Event<HandlePendingUserEvents>, &RootWindowTarget)>>,
>(Rc::new(RefCell::new(callback)))
};
self._callback = Some(Rc::clone(&callback));
autoreleasepool(|_| {
// 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 `delegate.clear_callback` before returning
// to ensure that we don't hold on to the callback beyond its (erased)
// lifetime
unsafe {
self.delegate
.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 app delegate in an undefined, unsafe state)
let catch_result = catch_unwind(AssertUnwindSafe(|| {
self.delegate.set_event_handler(handler, || {
autoreleasepool(|_| {
// As a special case, if the application hasn't been launched yet then we at least run
// the loop until it has fully launched.
if !self.delegate.is_launched() {
debug_assert!(!self.delegate.is_running());
self.delegate.set_stop_on_launch();
unsafe {
self.app.run();
}
// SAFETY: We do not run the application re-entrantly
unsafe { self.app.run() };
// Note: we dispatch `NewEvents(Init)` + `Resumed` events after the application has launched
} else if !self.delegate.is_running() {
@ -438,9 +362,8 @@ impl<T> EventLoop<T> {
}
}
self.delegate.set_stop_on_redraw(true);
unsafe {
self.app.run();
}
// SAFETY: We do not run the application re-entrantly
unsafe { self.app.run() };
}
// While the app is running it's possible that we catch a panic
@ -458,18 +381,7 @@ impl<T> EventLoop<T> {
} 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 app delegate before returning
self.delegate.clear_callback();
drop(self._callback.take());
match catch_result {
Ok(pump_status) => pump_status,
Err(payload) => resume_unwind(payload),
}
})
})
}

1
src/platform_impl/macos/mod.rs
View file

@ -5,6 +5,7 @@ mod app;
mod app_delegate;
mod cursor;
mod event;
mod event_handler;
mod event_loop;
mod ffi;
mod menu;