Make iOS fully thread safe (#3045)

* macOS & iOS: Refactor EventWrapper

* macOS & iOS: Make EventLoopWindowTarget independent of the user event

* iOS: Use MainThreadMarker instead of marking functions unsafe

* Make iOS thread safe

src/platform_impl/macos/app.rs

@@ -4,7 +4,7 @@ use icrate::Foundation::NSObject;
 use objc2::{declare_class, msg_send, mutability, ClassType};
 
 use super::appkit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType, NSResponder};
-use super::{app_state::AppState, event::EventWrapper, DEVICE_ID};
+use super::{app_state::AppState, DEVICE_ID};
 use crate::event::{DeviceEvent, ElementState, Event};
 
 declare_class!(
@@ -96,5 +96,5 @@ fn queue_device_event(event: DeviceEvent) {
         device_id: DEVICE_ID,
         event,
     };
-    AppState::queue_event(EventWrapper::StaticEvent(event));
+    AppState::queue_event(event);
 }

src/platform_impl/macos/app_state.rs

@@ -6,29 +6,24 @@ use std::{
     rc::{Rc, Weak},
     sync::{
         atomic::{AtomicBool, Ordering},
-        Arc, Mutex, MutexGuard,
+        mpsc, Arc, Mutex, MutexGuard,
     },
     time::Instant,
 };
 
 use core_foundation::runloop::{CFRunLoopGetMain, CFRunLoopWakeUp};
 use icrate::Foundation::{is_main_thread, NSSize};
-use objc2::rc::autoreleasepool;
+use objc2::rc::{autoreleasepool, Id};
 use once_cell::sync::Lazy;
 
 use super::appkit::{NSApp, NSApplication, NSApplicationActivationPolicy, NSEvent};
+use super::{
+    event_loop::PanicInfo, menu, observer::EventLoopWaker, util::Never, window::WinitWindow,
+};
 use crate::{
-    dpi::LogicalSize,
+    dpi::PhysicalSize,
     event::{Event, InnerSizeWriter, StartCause, WindowEvent},
     event_loop::{ControlFlow, EventLoopWindowTarget as RootWindowTarget},
-    platform_impl::platform::{
-        event::{EventProxy, EventWrapper},
-        event_loop::PanicInfo,
-        menu,
-        observer::EventLoopWaker,
-        util::Never,
-        window::WinitWindow,
-    },
     window::WindowId,
 };
 
@@ -54,6 +49,7 @@ pub(crate) type Callback<T> = RefCell<dyn FnMut(Event<T>, &RootWindowTarget<T>,
 struct EventLoopHandler<T: 'static> {
     callback: Weak<Callback<T>>,
     window_target: Rc<RootWindowTarget<T>>,
+    receiver: Rc<mpsc::Receiver<T>>,
 }
 
 impl<T> EventLoopHandler<T> {
@@ -103,7 +99,7 @@ impl<T> EventHandler for EventLoopHandler<T> {
 
     fn handle_user_events(&mut self, control_flow: &mut ControlFlow) {
         self.with_callback(|this, mut callback| {
-            for event in this.window_target.p.receiver.try_iter() {
+            for event in this.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);
@@ -116,6 +112,16 @@ impl<T> EventHandler for EventLoopHandler<T> {
     }
 }
 
+#[derive(Debug)]
+enum EventWrapper {
+    StaticEvent(Event<Never>),
+    ScaleFactorChanged {
+        window: Id<WinitWindow>,
+        suggested_size: PhysicalSize<u32>,
+        scale_factor: f64,
+    },
+}
+
 #[derive(Default)]
 struct Handler {
     stop_app_on_launch: AtomicBool,
@@ -313,14 +319,9 @@ impl Handler {
         self.callback.lock().unwrap().is_some()
     }
 
-    fn handle_nonuser_event(&self, wrapper: EventWrapper) {
+    fn handle_nonuser_event(&self, event: Event<Never>) {
         if let Some(ref mut callback) = *self.callback.lock().unwrap() {
-            match wrapper {
-                EventWrapper::StaticEvent(event) => {
-                    callback.handle_nonuser_event(event, &mut self.control_flow.lock().unwrap())
-                }
-                EventWrapper::EventProxy(proxy) => self.handle_proxy(proxy, callback),
-            }
+            callback.handle_nonuser_event(event, &mut self.control_flow.lock().unwrap())
         }
     }
 
@@ -332,41 +333,27 @@ impl Handler {
 
     fn handle_scale_factor_changed_event(
         &self,
-        callback: &mut Box<dyn EventHandler + 'static>,
         window: &WinitWindow,
-        suggested_size: LogicalSize<f64>,
+        suggested_size: PhysicalSize<u32>,
         scale_factor: f64,
     ) {
-        let new_inner_size = Arc::new(Mutex::new(suggested_size.to_physical(scale_factor)));
-        let event = Event::WindowEvent {
-            window_id: WindowId(window.id()),
-            event: WindowEvent::ScaleFactorChanged {
-                scale_factor,
-                inner_size_writer: InnerSizeWriter::new(Arc::downgrade(&new_inner_size)),
-            },
-        };
+        if let Some(ref mut callback) = *self.callback.lock().unwrap() {
+            let new_inner_size = Arc::new(Mutex::new(suggested_size));
+            let event = Event::WindowEvent {
+                window_id: WindowId(window.id()),
+                event: WindowEvent::ScaleFactorChanged {
+                    scale_factor,
+                    inner_size_writer: InnerSizeWriter::new(Arc::downgrade(&new_inner_size)),
+                },
+            };
 
-        callback.handle_nonuser_event(event, &mut self.control_flow.lock().unwrap());
+            callback.handle_nonuser_event(event, &mut self.control_flow.lock().unwrap());
 
-        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);
-    }
-
-    fn handle_proxy(&self, proxy: EventProxy, callback: &mut Box<dyn EventHandler + 'static>) {
-        match proxy {
-            EventProxy::DpiChangedProxy {
-                window,
-                suggested_size,
-                scale_factor,
-            } => self.handle_scale_factor_changed_event(
-                callback,
-                &window,
-                suggested_size,
-                scale_factor,
-            ),
+            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);
         }
     }
 }
@@ -387,10 +374,12 @@ impl AppState {
     pub unsafe fn set_callback<T>(
         callback: Weak<Callback<T>>,
         window_target: Rc<RootWindowTarget<T>>,
+        receiver: Rc<mpsc::Receiver<T>>,
     ) {
         *HANDLER.callback.lock().unwrap() = Some(Box::new(EventLoopHandler {
             callback,
             window_target,
+            receiver,
         }));
     }
 
@@ -435,7 +424,7 @@ impl AppState {
 
     pub fn exit() -> i32 {
         HANDLER.set_in_callback(true);
-        HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::LoopExiting));
+        HANDLER.handle_nonuser_event(Event::LoopExiting);
         HANDLER.set_in_callback(false);
         HANDLER.exit();
         Self::clear_callback();
@@ -448,12 +437,10 @@ impl AppState {
 
     pub fn dispatch_init_events() {
         HANDLER.set_in_callback(true);
-        HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::NewEvents(
-            StartCause::Init,
-        )));
+        HANDLER.handle_nonuser_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.
-        HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::Resumed));
+        HANDLER.handle_nonuser_event(Event::Resumed);
         HANDLER.set_in_callback(false);
     }
 
@@ -544,7 +531,7 @@ impl AppState {
             ControlFlow::ExitWithCode(_) => StartCause::Poll, //panic!("unexpected `ControlFlow::Exit`"),
         };
         HANDLER.set_in_callback(true);
-        HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::NewEvents(cause)));
+        HANDLER.handle_nonuser_event(Event::NewEvents(cause));
         HANDLER.set_in_callback(false);
     }
 
@@ -564,10 +551,10 @@ impl AppState {
         // Redraw request might come out of order from the OS.
         // -> Don't go back into the callback when our callstack originates from there
         if !HANDLER.in_callback.swap(true, Ordering::AcqRel) {
-            HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::WindowEvent {
+            HANDLER.handle_nonuser_event(Event::WindowEvent {
                 window_id,
                 event: WindowEvent::RedrawRequested,
-            }));
+            });
             HANDLER.set_in_callback(false);
 
             // `pump_events` will request to stop immediately _after_ dispatching RedrawRequested events
@@ -578,11 +565,25 @@ impl AppState {
         }
     }
 
-    pub fn queue_event(wrapper: EventWrapper) {
+    pub fn queue_event(event: Event<Never>) {
         if !is_main_thread() {
-            panic!("Event queued from different thread: {wrapper:#?}");
+            panic!("Event queued from different thread: {event:#?}");
         }
-        HANDLER.events().push_back(wrapper);
+        HANDLER.events().push_back(EventWrapper::StaticEvent(event));
+    }
+
+    pub fn queue_static_scale_factor_changed_event(
+        window: Id<WinitWindow>,
+        suggested_size: PhysicalSize<u32>,
+        scale_factor: f64,
+    ) {
+        HANDLER
+            .events()
+            .push_back(EventWrapper::ScaleFactorChanged {
+                window,
+                suggested_size,
+                scale_factor,
+            });
     }
 
     pub fn stop() {
@@ -614,17 +615,32 @@ impl AppState {
         HANDLER.set_in_callback(true);
         HANDLER.handle_user_events();
         for event in HANDLER.take_events() {
-            HANDLER.handle_nonuser_event(event);
+            match event {
+                EventWrapper::StaticEvent(event) => {
+                    HANDLER.handle_nonuser_event(event);
+                }
+                EventWrapper::ScaleFactorChanged {
+                    window,
+                    suggested_size,
+                    scale_factor,
+                } => {
+                    HANDLER.handle_scale_factor_changed_event(
+                        &window,
+                        suggested_size,
+                        scale_factor,
+                    );
+                }
+            }
         }
 
         for window_id in HANDLER.should_redraw() {
-            HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::WindowEvent {
+            HANDLER.handle_nonuser_event(Event::WindowEvent {
                 window_id,
                 event: WindowEvent::RedrawRequested,
-            }));
+            });
         }
 
-        HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::AboutToWait));
+        HANDLER.handle_nonuser_event(Event::AboutToWait);
         HANDLER.set_in_callback(false);
 
         if HANDLER.should_exit() {

src/platform_impl/macos/event.rs

@@ -5,15 +5,11 @@ use core_foundation::{
     data::{CFDataGetBytePtr, CFDataRef},
 };
 use icrate::Foundation::MainThreadMarker;
-use objc2::rc::Id;
 use smol_str::SmolStr;
 
 use super::appkit::{NSEvent, NSEventModifierFlags};
-use super::util::Never;
-use super::window::WinitWindow;
 use crate::{
-    dpi::LogicalSize,
-    event::{ElementState, Event, KeyEvent, Modifiers},
+    event::{ElementState, KeyEvent, Modifiers},
     keyboard::{
         Key, KeyCode, KeyLocation, ModifiersKeys, ModifiersState, NativeKey, NativeKeyCode,
     },
@@ -21,21 +17,6 @@ use crate::{
     platform_impl::platform::ffi,
 };
 
-#[derive(Debug)]
-pub(crate) enum EventWrapper {
-    StaticEvent(Event<Never>),
-    EventProxy(EventProxy),
-}
-
-#[derive(Debug)]
-pub(crate) enum EventProxy {
-    DpiChangedProxy {
-        window: Id<WinitWindow>,
-        suggested_size: LogicalSize<f64>,
-        scale_factor: f64,
-    },
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct KeyEventExtra {
     pub text_with_all_modifiers: Option<SmolStr>,

src/platform_impl/macos/event_loop.rs

@@ -65,9 +65,10 @@ impl PanicInfo {
     }
 }
 
+#[derive(Debug)]
 pub struct EventLoopWindowTarget<T: 'static> {
-    pub receiver: mpsc::Receiver<T>,
     mtm: MainThreadMarker,
+    p: PhantomData<T>,
 }
 
 impl<T: 'static> EventLoopWindowTarget<T> {
@@ -107,14 +108,18 @@ impl<T> EventLoopWindowTarget<T> {
 }
 
 pub struct EventLoop<T: 'static> {
+    /// Store a reference to the application for convenience.
+    app: Id<WinitApplication>,
     /// The delegate is only weakly referenced by NSApplication, so we keep
     /// it around here as well.
     _delegate: Id<ApplicationDelegate>,
 
+    // Event sender and receiver, used for EventLoopProxy.
     sender: mpsc::Sender<T>,
+    receiver: Rc<mpsc::Receiver<T>>,
+
     window_target: Rc<RootWindowTarget<T>>,
     panic_info: Rc<PanicInfo>,
-    mtm: MainThreadMarker,
 
     /// We make sure that the callback closure is dropped during a panic
     /// by making the event loop own it.
@@ -177,13 +182,17 @@ impl<T> EventLoop<T> {
 
         let (sender, receiver) = mpsc::channel();
         Ok(EventLoop {
+            app,
             _delegate: delegate,
             sender,
+            receiver: Rc::new(receiver),
             window_target: Rc::new(RootWindowTarget {
-                p: EventLoopWindowTarget { receiver, mtm },
+                p: EventLoopWindowTarget {
+                    mtm,
+                    p: PhantomData,
+                },
                 _marker: PhantomData,
             }),
-            mtm,
             panic_info,
             _callback: None,
         })
@@ -231,8 +240,6 @@ impl<T> EventLoop<T> {
         self._callback = Some(Rc::clone(&callback));
 
         let exit_code = autoreleasepool(|_| {
-            let app = NSApplication::shared(self.mtm);
-
             // 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);
@@ -241,7 +248,11 @@ impl<T> EventLoop<T> {
             // # Safety
             // We make sure to call `AppState::clear_callback` before returning
             unsafe {
-                AppState::set_callback(weak_cb, Rc::clone(&self.window_target));
+                AppState::set_callback(
+                    weak_cb,
+                    Rc::clone(&self.window_target),
+                    Rc::clone(&self.receiver),
+                );
             }
 
             // catch panics to make sure we can't unwind without clearing the set callback
@@ -257,7 +268,7 @@ impl<T> EventLoop<T> {
                     debug_assert!(!AppState::is_running());
                     AppState::start_running(); // Set is_running = true + dispatch `NewEvents(Init)` + `Resumed`
                 }
-                unsafe { app.run() };
+                unsafe { 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
@@ -326,7 +337,11 @@ impl<T> EventLoop<T> {
             // 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));
+                AppState::set_callback(
+                    weak_cb,
+                    Rc::clone(&self.window_target),
+                    Rc::clone(&self.receiver),
+                );
             }
 
             // catch panics to make sure we can't unwind without clearing the set callback

src/platform_impl/macos/view.rs

@@ -31,7 +31,7 @@ use crate::{
     platform::scancode::KeyCodeExtScancode,
     platform_impl::platform::{
         app_state::AppState,
-        event::{create_key_event, event_mods, EventWrapper},
+        event::{create_key_event, event_mods},
         util,
         window::WinitWindow,
         DEVICE_ID,
@@ -826,7 +826,7 @@ impl WinitView {
             window_id: self.window_id(),
             event,
         };
-        AppState::queue_event(EventWrapper::StaticEvent(event));
+        AppState::queue_event(event);
     }
 
     fn queue_device_event(&self, event: DeviceEvent) {
@@ -834,7 +834,7 @@ impl WinitView {
             device_id: DEVICE_ID,
             event,
         };
-        AppState::queue_event(EventWrapper::StaticEvent(event));
+        AppState::queue_event(event);
     }
 
     fn scale_factor(&self) -> f64 {

src/platform_impl/macos/window_delegate.rs

@@ -11,16 +11,15 @@ use objc2::{class, declare_class, msg_send, msg_send_id, mutability, sel, ClassT
 use super::appkit::{
     NSApplicationPresentationOptions, NSFilenamesPboardType, NSPasteboard, NSWindowOcclusionState,
 };
+use super::{
+    app_state::AppState,
+    util,
+    window::{get_ns_theme, WinitWindow},
+    Fullscreen,
+};
 use crate::{
     dpi::{LogicalPosition, LogicalSize},
     event::{Event, WindowEvent},
-    platform_impl::platform::{
-        app_state::AppState,
-        event::{EventProxy, EventWrapper},
-        util,
-        window::{get_ns_theme, WinitWindow},
-        Fullscreen,
-    },
     window::WindowId,
 };
 
@@ -447,7 +446,7 @@ impl WinitWindowDelegate {
             window_id: WindowId(self.window.id()),
             event,
         };
-        AppState::queue_event(EventWrapper::StaticEvent(event));
+        AppState::queue_event(event);
     }
 
     fn queue_static_scale_factor_changed_event(&self) {
@@ -457,12 +456,12 @@ impl WinitWindowDelegate {
         };
 
         self.state.previous_scale_factor.set(scale_factor);
-        let wrapper = EventWrapper::EventProxy(EventProxy::DpiChangedProxy {
-            window: self.window.clone(),
-            suggested_size: self.view_size(),
+        let suggested_size = self.view_size();
+        AppState::queue_static_scale_factor_changed_event(
+            self.window.clone(),
+            suggested_size.to_physical(scale_factor),
             scale_factor,
-        });
-        AppState::queue_event(wrapper);
+        );
     }
 
     fn emit_move_event(&self) {