Fix thread-safety of set_maximized and set_title on macOS (#922)

src/platform_impl/macos/util/async.rs

@@ -1,14 +1,17 @@
-use std::{os::raw::c_void, sync::{Mutex, Weak}};
+use std::{
+    os::raw::c_void,
+    sync::{Mutex, Weak},
+};
 
 use cocoa::{
-    appkit::{CGFloat, NSWindow, NSWindowStyleMask},
+    appkit::{CGFloat, NSScreen, NSWindow, NSWindowStyleMask},
     base::{id, nil},
-    foundation::{NSAutoreleasePool, NSPoint, NSSize},
+    foundation::{NSAutoreleasePool, NSPoint, NSSize, NSString},
 };
 use dispatch::ffi::{dispatch_async_f, dispatch_get_main_queue, dispatch_sync_f};
 
 use crate::dpi::LogicalSize;
-use crate::platform_impl::platform::{ffi, window::SharedState};
+use crate::platform_impl::platform::{ffi, util::IdRef, window::SharedState};
 
 unsafe fn set_style_mask(ns_window: id, ns_view: id, mask: NSWindowStyleMask) {
     ns_window.setStyleMask_(mask);
@@ -237,6 +240,83 @@ pub unsafe fn toggle_full_screen_async(
     );
 }
 
+struct SetMaximizedData {
+    ns_window: id,
+    is_zoomed: bool,
+    maximized: bool,
+    shared_state: Weak<Mutex<SharedState>>,
+}
+impl SetMaximizedData {
+    fn new_ptr(
+        ns_window: id,
+        is_zoomed: bool,
+        maximized: bool,
+        shared_state: Weak<Mutex<SharedState>>,
+    ) -> *mut Self {
+        Box::into_raw(Box::new(SetMaximizedData {
+            ns_window,
+            is_zoomed,
+            maximized,
+            shared_state,
+        }))
+    }
+}
+extern "C" fn set_maximized_callback(context: *mut c_void) {
+    unsafe {
+        let context_ptr = context as *mut SetMaximizedData;
+        {
+            let context = &*context_ptr;
+
+            if let Some(shared_state) = context.shared_state.upgrade() {
+                trace!("Locked shared state in `set_maximized`");
+                let mut shared_state_lock = shared_state.lock().unwrap();
+
+                // Save the standard frame sized if it is not zoomed
+                if !context.is_zoomed {
+                    shared_state_lock.standard_frame = Some(NSWindow::frame(context.ns_window));
+                }
+
+                shared_state_lock.maximized = context.maximized;
+
+                let curr_mask = context.ns_window.styleMask();
+                if shared_state_lock.fullscreen.is_some() {
+                    // Handle it in window_did_exit_fullscreen
+                    return;
+                } else if curr_mask.contains(NSWindowStyleMask::NSResizableWindowMask) {
+                    // Just use the native zoom if resizable
+                    context.ns_window.zoom_(nil);
+                } else {
+                    // if it's not resizable, we set the frame directly
+                    let new_rect = if context.maximized {
+                        let screen = NSScreen::mainScreen(nil);
+                        NSScreen::visibleFrame(screen)
+                    } else {
+                        shared_state_lock.saved_standard_frame()
+                    };
+                    context.ns_window.setFrame_display_(new_rect, 0);
+                }
+
+                trace!("Unlocked shared state in `set_maximized`");
+            }
+        }
+        Box::from_raw(context_ptr);
+    }
+}
+// `setMaximized` is not thread-safe
+pub unsafe fn set_maximized_async(
+    ns_window: id,
+    is_zoomed: bool,
+    maximized: bool,
+    shared_state: Weak<Mutex<SharedState>>,
+) {
+    let context = SetMaximizedData::new_ptr(ns_window, is_zoomed, maximized, shared_state);
+    dispatch_async_f(
+        dispatch_get_main_queue(),
+        context as *mut _,
+        set_maximized_callback,
+    );
+}
+
 struct OrderOutData {
     ns_window: id,
 }
@@ -295,6 +375,38 @@ pub unsafe fn make_key_and_order_front_async(ns_window: id) {
     );
 }
 
+struct SetTitleData {
+    ns_window: id,
+    title: String,
+}
+impl SetTitleData {
+    fn new_ptr(ns_window: id, title: String) -> *mut Self {
+        Box::into_raw(Box::new(SetTitleData { ns_window, title }))
+    }
+}
+extern "C" fn set_title_callback(context: *mut c_void) {
+    unsafe {
+        let context_ptr = context as *mut SetTitleData;
+        {
+            let context = &*context_ptr;
+            let title = IdRef::new(NSString::alloc(nil).init_str(&context.title));
+            context.ns_window.setTitle_(*title);
+        }
+        Box::from_raw(context_ptr);
+    }
+}
+// `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 unsafe fn set_title_async(ns_window: id, title: String) {
+    let context = SetTitleData::new_ptr(ns_window, title);
+    dispatch_async_f(
+        dispatch_get_main_queue(),
+        context as *mut _,
+        set_title_callback,
+    );
+}
+
 struct CloseData {
     ns_window: id,
 }

src/platform_impl/macos/window.rs

@@ -217,12 +217,19 @@ pub struct SharedState {
     pub resizable: bool,
     pub fullscreen: Option<RootMonitorHandle>,
     pub maximized: bool,
-    standard_frame: Option<NSRect>,
+    pub standard_frame: Option<NSRect>,
     is_simple_fullscreen: bool,
     pub saved_style: Option<NSWindowStyleMask>,
     save_presentation_opts: Option<NSApplicationPresentationOptions>,
 }
 
+impl SharedState {
+    pub fn saved_standard_frame(&self) -> NSRect {
+        self.standard_frame
+            .unwrap_or_else(|| NSRect::new(NSPoint::new(50.0, 50.0), NSSize::new(800.0, 600.0)))
+    }
+}
+
 impl From<WindowAttributes> for SharedState {
     fn from(attribs: WindowAttributes) -> Self {
         SharedState {
@@ -378,8 +385,7 @@ impl UnownedWindow {
 
     pub fn set_title(&self, title: &str) {
         unsafe {
-            let title = IdRef::new(NSString::alloc(nil).init_str(title));
-            self.ns_window.setTitle_(*title);
+            util::set_title_async(*self.ns_window, title.to_string());
         }
     }
 
@@ -570,13 +576,6 @@ impl UnownedWindow {
         }
     }
 
-    fn saved_standard_frame(shared_state: &mut SharedState) -> NSRect {
-        shared_state.standard_frame.unwrap_or_else(|| NSRect::new(
-            NSPoint::new(50.0, 50.0),
-            NSSize::new(800.0, 600.0),
-        ))
-    }
-
     pub(crate) fn restore_state_from_fullscreen(&self) {
         let maximized = {
             trace!("Locked shared state in `restore_state_from_fullscreen`");
@@ -596,42 +595,17 @@ impl UnownedWindow {
     #[inline]
     pub fn set_maximized(&self, maximized: bool) {
         let is_zoomed = self.is_zoomed();
-        if is_zoomed == maximized { return };
-
-        trace!("Locked shared state in `set_maximized`");
-        let mut shared_state_lock = self.shared_state.lock().unwrap();
-
-        // Save the standard frame sized if it is not zoomed
-        if !is_zoomed {
-            unsafe {
-                shared_state_lock.standard_frame = Some(NSWindow::frame(*self.ns_window));
-            }
-        }
-
-        shared_state_lock.maximized = maximized;
-
-        let curr_mask = unsafe { self.ns_window.styleMask() };
-        if shared_state_lock.fullscreen.is_some() {
-            // Handle it in window_did_exit_fullscreen
+        if is_zoomed == maximized {
             return;
-        } else if curr_mask.contains(NSWindowStyleMask::NSResizableWindowMask) {
-            // Just use the native zoom if resizable
-            unsafe { self.ns_window.zoom_(nil) };
-        } else {
-            // if it's not resizable, we set the frame directly
-            unsafe {
-                let new_rect = if maximized {
-                    let screen = NSScreen::mainScreen(nil);
-                    NSScreen::visibleFrame(screen)
-                } else {
-                    Self::saved_standard_frame(&mut *shared_state_lock)
-                };
-                // This probably isn't thread-safe!
-                self.ns_window.setFrame_display_(new_rect, 0);
-            }
+        };
+        unsafe {
+            util::set_maximized_async(
+                *self.ns_window,
+                is_zoomed,
+                maximized,
+                Arc::downgrade(&self.shared_state),
+            );
         }
-
-        trace!("Unlocked shared state in `set_maximized`");
     }
 
     #[inline]
@@ -847,7 +821,7 @@ impl WindowExtMacOS for UnownedWindow {
                     app.setPresentationOptions_(presentation_opts);
                 }
 
-                let frame = Self::saved_standard_frame(&mut *shared_state_lock);
+                let frame = shared_state_lock.saved_standard_frame();
                 NSWindow::setFrame_display_(*self.ns_window, frame, YES);
                 NSWindow::setMovable_(*self.ns_window, YES);