macOS: Run tasks synchronously on main thread instead of asynchronously (#2574)

* Close windows synchronously on main thread

* Set style mask synchronously on main thread

* Set title synchronously on main thread

* Set visibility and focus synchronously on main thread

* Set window level synchronously on main thread

* Set position and size synchronously on main thread

* Set cursor hittest synchronously on main thread

* Add changelog entry

CHANGELOG.md

@@ -8,6 +8,7 @@ And please only add new entries to the top of this list, right below the `# Unre
 
 # Unreleased
 
+- On macOS, run most actions on the main thread, which is strictly more correct, but might make multithreaded applications block slightly more.
 - On macOS, fix panic when getting current monitor without any monitor attached.
 - On Windows and MacOS, add API to enable/disable window buttons (close, minimize, ...etc).
 - On Windows, macOS, X11 and Wayland, add `Window::set_theme`.

src/platform_impl/macos/mod.rs

@@ -57,7 +57,7 @@ pub(crate) struct Window {
 impl Drop for Window {
     fn drop(&mut self) {
         // Ensure the window is closed
-        util::close_async(Id::into_super(self.window.clone()));
+        util::close_sync(&self.window);
     }
 }
 

src/platform_impl/macos/util/async.rs

@@ -1,4 +1,3 @@
-use std::mem;
 use std::ops::Deref;
 
 use dispatch::Queue;
@@ -29,6 +28,14 @@ impl<T> Deref for MainThreadSafe<T> {
     }
 }
 
+fn run_on_main<R: Send>(f: impl FnOnce() -> R + Send) -> R {
+    if is_main_thread() {
+        f()
+    } else {
+        Queue::main().exec_sync(f)
+    }
+}
+
 fn set_style_mask(window: &NSWindow, mask: NSWindowStyleMask) {
     window.setStyleMask(mask);
     // If we don't do this, key handling will break
@@ -40,54 +47,43 @@ fn set_style_mask(window: &NSWindow, mask: NSWindowStyleMask) {
 // `setStyleMask:` isn't thread-safe, so we have to use Grand Central Dispatch.
 // Otherwise, this would vomit out errors about not being on the main thread
 // and fail to do anything.
-pub(crate) fn set_style_mask_async(window: &NSWindow, mask: NSWindowStyleMask) {
-    // TODO(madsmtm): Remove this 'static hack!
-    let window = unsafe { MainThreadSafe(mem::transmute::<&NSWindow, &'static NSWindow>(window)) };
-    Queue::main().exec_async(move || {
-        set_style_mask(&window, mask);
-    });
-}
 pub(crate) fn set_style_mask_sync(window: &NSWindow, mask: NSWindowStyleMask) {
-    if is_main_thread() {
+    let window = MainThreadSafe(window);
-        set_style_mask(window, mask);
+    run_on_main(move || {
-    } else {
+        set_style_mask(&window, mask);
-        let window = MainThreadSafe(window);
+    })
-        Queue::main().exec_sync(move || {
-            set_style_mask(&window, mask);
-        })
-    }
 }
 
 // `setContentSize:` isn't thread-safe either, though it doesn't log any errors
 // and just fails silently. Anyway, GCD to the rescue!
-pub(crate) fn set_content_size_async(window: &NSWindow, size: LogicalSize<f64>) {
+pub(crate) fn set_content_size_sync(window: &NSWindow, size: LogicalSize<f64>) {
-    let window = unsafe { MainThreadSafe(mem::transmute::<&NSWindow, &'static NSWindow>(window)) };
+    let window = MainThreadSafe(window);
-    Queue::main().exec_async(move || {
+    run_on_main(move || {
         window.setContentSize(NSSize::new(size.width as CGFloat, size.height as CGFloat));
     });
 }
 
 // `setFrameTopLeftPoint:` isn't thread-safe, but fortunately has the courtesy
 // to log errors.
-pub(crate) fn set_frame_top_left_point_async(window: &NSWindow, point: NSPoint) {
+pub(crate) fn set_frame_top_left_point_sync(window: &NSWindow, point: NSPoint) {
-    let window = unsafe { MainThreadSafe(mem::transmute::<&NSWindow, &'static NSWindow>(window)) };
+    let window = MainThreadSafe(window);
-    Queue::main().exec_async(move || {
+    run_on_main(move || {
         window.setFrameTopLeftPoint(point);
     });
 }
 
 // `setFrameTopLeftPoint:` isn't thread-safe, and fails silently.
-pub(crate) fn set_level_async(window: &NSWindow, level: NSWindowLevel) {
+pub(crate) fn set_level_sync(window: &NSWindow, level: NSWindowLevel) {
-    let window = unsafe { MainThreadSafe(mem::transmute::<&NSWindow, &'static NSWindow>(window)) };
+    let window = MainThreadSafe(window);
-    Queue::main().exec_async(move || {
+    run_on_main(move || {
         window.setLevel(level);
     });
 }
 
 // `setIgnoresMouseEvents_:` isn't thread-safe, and fails silently.
-pub(crate) fn set_ignore_mouse_events(window: &NSWindow, ignore: bool) {
+pub(crate) fn set_ignore_mouse_events_sync(window: &NSWindow, ignore: bool) {
-    let window = unsafe { MainThreadSafe(mem::transmute::<&NSWindow, &'static NSWindow>(window)) };
+    let window = MainThreadSafe(window);
-    Queue::main().exec_async(move || {
+    run_on_main(move || {
         window.setIgnoresMouseEvents(ignore);
     });
 }
@@ -171,18 +167,18 @@ pub(crate) fn set_maximized_async(
 
 // `orderOut:` isn't thread-safe. Calling it from another thread actually works,
 // but with an odd delay.
-pub(crate) fn order_out_async(window: &NSWindow) {
+pub(crate) fn order_out_sync(window: &NSWindow) {
-    let window = unsafe { MainThreadSafe(mem::transmute::<&NSWindow, &'static NSWindow>(window)) };
+    let window = MainThreadSafe(window);
-    Queue::main().exec_async(move || {
+    run_on_main(move || {
         window.orderOut(None);
     });
 }
 
 // `makeKeyAndOrderFront:` isn't thread-safe. Calling it from another thread
 // actually works, but with an odd delay.
-pub(crate) fn make_key_and_order_front_async(window: &NSWindow) {
+pub(crate) fn make_key_and_order_front_sync(window: &NSWindow) {
-    let window = unsafe { MainThreadSafe(mem::transmute::<&NSWindow, &'static NSWindow>(window)) };
+    let window = MainThreadSafe(window);
-    Queue::main().exec_async(move || {
+    run_on_main(move || {
         window.makeKeyAndOrderFront(None);
     });
 }
@@ -190,21 +186,18 @@ pub(crate) fn make_key_and_order_front_async(window: &NSWindow) {
 // `setTitle:` isn't thread-safe. Calling it from another thread invalidates the
 // window drag regions, which throws an exception when not done in the main
 // thread
-pub(crate) fn set_title_async(window: &NSWindow, title: String) {
+pub(crate) fn set_title_sync(window: &NSWindow, title: &str) {
-    let window = unsafe { MainThreadSafe(mem::transmute::<&NSWindow, &'static NSWindow>(window)) };
+    let window = MainThreadSafe(window);
-    Queue::main().exec_async(move || {
+    run_on_main(move || {
-        window.setTitle(&NSString::from_str(&title));
+        window.setTitle(&NSString::from_str(title));
     });
 }
 
 // `close:` is thread-safe, but we want the event to be triggered from the main
 // thread. Though, it's a good idea to look into that more...
-//
+pub(crate) fn close_sync(window: &NSWindow) {
-// ArturKovacs: It's important that this operation keeps the underlying window alive
-// through the `Id` because otherwise it would dereference free'd memory
-pub(crate) fn close_async(window: Id<NSWindow, Shared>) {
     let window = MainThreadSafe(window);
-    Queue::main().exec_async(move || {
+    run_on_main(move || {
         autoreleasepool(move |_| {
             window.close();
         });

src/platform_impl/macos/window.rs

@@ -459,10 +459,6 @@ impl WinitWindow {
         SharedStateMutexGuard::new(self.shared_state.lock().unwrap(), called_from_fn)
     }
 
-    fn set_style_mask_async(&self, mask: NSWindowStyleMask) {
-        util::set_style_mask_async(self, mask);
-    }
-
     fn set_style_mask_sync(&self, mask: NSWindowStyleMask) {
         util::set_style_mask_sync(self, mask);
     }
@@ -472,13 +468,13 @@ impl WinitWindow {
     }
 
     pub fn set_title(&self, title: &str) {
-        util::set_title_async(self, title.to_string());
+        util::set_title_sync(self, title);
     }
 
     pub fn set_visible(&self, visible: bool) {
         match visible {
-            true => util::make_key_and_order_front_async(self),
+            true => util::make_key_and_order_front_sync(self),
-            false => util::order_out_async(self),
+            false => util::order_out_sync(self),
         }
     }
 
@@ -514,7 +510,7 @@ impl WinitWindow {
     pub fn set_outer_position(&self, position: Position) {
         let scale_factor = self.scale_factor();
         let position = position.to_logical(scale_factor);
-        util::set_frame_top_left_point_async(self, util::window_position(position));
+        util::set_frame_top_left_point_sync(self, util::window_position(position));
     }
 
     #[inline]
@@ -536,7 +532,7 @@ impl WinitWindow {
     #[inline]
     pub fn set_inner_size(&self, size: Size) {
         let scale_factor = self.scale_factor();
-        util::set_content_size_async(self, size.to_logical(scale_factor));
+        util::set_content_size_sync(self, size.to_logical(scale_factor));
     }
 
     pub fn set_min_inner_size(&self, dimensions: Option<Size>) {
@@ -632,8 +628,9 @@ impl WinitWindow {
             } else {
                 mask &= !NSWindowStyleMask::NSResizableWindowMask;
             }
-            self.set_style_mask_async(mask);
+            self.set_style_mask_sync(mask);
-        } // Otherwise, we don't change the mask until we exit fullscreen.
+        }
+        // Otherwise, we don't change the mask until we exit fullscreen.
     }
 
     #[inline]
@@ -754,7 +751,7 @@ impl WinitWindow {
 
     #[inline]
     pub fn set_cursor_hittest(&self, hittest: bool) -> Result<(), ExternalError> {
-        util::set_ignore_mouse_events(self, !hittest);
+        util::set_ignore_mouse_events_sync(self, !hittest);
         Ok(())
     }
 
@@ -774,7 +771,7 @@ impl WinitWindow {
 
         // Roll back temp styles
         if needs_temp_mask {
-            self.set_style_mask_async(curr_mask);
+            self.set_style_mask_sync(curr_mask);
         }
 
         is_zoomed
@@ -805,7 +802,7 @@ impl WinitWindow {
 
         drop(shared_state_lock);
 
-        self.set_style_mask_async(mask);
+        self.set_style_mask_sync(mask);
         self.set_maximized(maximized);
     }
 
@@ -885,7 +882,7 @@ impl WinitWindow {
                 // The coordinate system here has its origin at bottom-left
                 // and Y goes up
                 screen_frame.origin.y += screen_frame.size.height;
-                util::set_frame_top_left_point_async(self, screen_frame.origin);
+                util::set_frame_top_left_point_sync(self, screen_frame.origin);
             }
         }
 
@@ -1054,7 +1051,7 @@ impl WinitWindow {
                 }
                 new_mask
             };
-            self.set_style_mask_async(new_mask);
+            self.set_style_mask_sync(new_mask);
         }
     }
 
@@ -1070,7 +1067,7 @@ impl WinitWindow {
             WindowLevel::AlwaysOnBottom => NSWindowLevel::BELOW_NORMAL,
             WindowLevel::Normal => NSWindowLevel::Normal,
         };
-        util::set_level_async(self, level);
+        util::set_level_sync(self, level);
     }
 
     #[inline]
@@ -1106,7 +1103,7 @@ impl WinitWindow {
 
         if !is_minimized && is_visible {
             NSApp().activateIgnoringOtherApps(true);
-            util::make_key_and_order_front_async(self);
+            util::make_key_and_order_front_sync(self);
         }
     }
 
@@ -1255,7 +1252,7 @@ impl WindowExtMacOS for WinitWindow {
             true
         } else {
             let new_mask = self.saved_style(&mut shared_state_lock);
-            self.set_style_mask_async(new_mask);
+            self.set_style_mask_sync(new_mask);
             shared_state_lock.is_simple_fullscreen = false;
 
             if let Some(presentation_opts) = shared_state_lock.save_presentation_opts {