gpui_macos: Guard deferred AppKit calls against closed windows (#51992) (cherry-pick to preview) (#52226)

zed-zippy[bot] and Lukas Wirth created

Cherry-pick of #51992 to preview

----
MacWindow methods like activate(), zoom(), toggle_fullscreen(),
resize(), prompt(), and titlebar_double_click() capture the raw
native_window pointer and spawn detached async tasks on the foreground
executor. If the window is closed between the spawn and execution (e.g.
the user clicks the close button), the task sends AppKit messages to a
closed NSWindow, which throws an ObjC exception in
_changeKeyAndMainLimitedOK: and aborts the process.

Add an isVisible check inside each deferred task before sending messages
to the native window. After [NSWindow close], isVisible returns NO, so
the guard prevents the crash. Both the check and the subsequent AppKit
call execute synchronously within the same main-thread task, so there is
no TOCTOU race.

For prompt(), the else branch releases the alert object to avoid leaking
it; the oneshot sender inside the completion block is dropped, which
cancels the channel and propagates as an error to the caller.

Closes ZED-5TN


Release Notes:

- N/A or Added/Fixed/Improved ...

Co-authored-by: Lukas Wirth <lukas@zed.dev>

Change summary

crates/gpui_macos/src/window.rs | 135 +++++++++++++++++++++-------------
1 file changed, 84 insertions(+), 51 deletions(-)

Detailed changes

crates/gpui_macos/src/window.rs 🔗

@@ -55,7 +55,10 @@ use std::{
     path::PathBuf,
     ptr::{self, NonNull},
     rc::Rc,
-    sync::{Arc, Weak},
+    sync::{
+        Arc, Weak,
+        atomic::{AtomicBool, Ordering},
+    },
     time::Duration,
 };
 use util::ResultExt;
@@ -440,6 +443,7 @@ struct MacWindowState {
     select_previous_tab_callback: Option<Box<dyn FnMut()>>,
     toggle_tab_bar_callback: Option<Box<dyn FnMut()>>,
     activated_least_once: bool,
+    closed: Arc<AtomicBool>,
     // The parent window if this window is a sheet (Dialog kind)
     sheet_parent: Option<id>,
 }
@@ -764,6 +768,7 @@ impl MacWindow {
                 select_previous_tab_callback: None,
                 toggle_tab_bar_callback: None,
                 activated_least_once: false,
+                closed: Arc::new(AtomicBool::new(false)),
                 sheet_parent: None,
             })));
 
@@ -1020,6 +1025,17 @@ impl Drop for MacWindow {
     }
 }
 
+/// Calls `f` if the window is not closed.
+///
+/// This should be used when spawning foreground tasks interacting with the
+/// window, as some messages will end hard faulting if dispatched to no longer
+/// valid window handles.
+fn if_window_not_closed(closed: Arc<AtomicBool>, f: impl FnOnce()) {
+    if !closed.load(Ordering::Acquire) {
+        f();
+    }
+}
+
 impl PlatformWindow for MacWindow {
     fn bounds(&self) -> Bounds<Pixels> {
         self.0.as_ref().lock().bounds()
@@ -1040,14 +1056,15 @@ impl PlatformWindow for MacWindow {
     fn resize(&mut self, size: Size<Pixels>) {
         let this = self.0.lock();
         let window = this.native_window;
+        let closed = this.closed.clone();
         this.foreground_executor
             .spawn(async move {
-                unsafe {
+                if_window_not_closed(closed, || unsafe {
                     window.setContentSize_(NSSize {
                         width: size.width.as_f32() as f64,
                         height: size.height.as_f32() as f64,
                     });
-                }
+                })
             })
             .detach();
     }
@@ -1260,15 +1277,21 @@ impl PlatformWindow for MacWindow {
                 }
             });
             let block = block.copy();
-            let native_window = self.0.lock().native_window;
-            let executor = self.0.lock().foreground_executor.clone();
+            let lock = self.0.lock();
+            let native_window = lock.native_window;
+            let closed = lock.closed.clone();
+            let executor = lock.foreground_executor.clone();
             executor
                 .spawn(async move {
-                    let _: () = msg_send![
-                        alert,
-                        beginSheetModalForWindow: native_window
-                        completionHandler: block
-                    ];
+                    if !closed.load(Ordering::Acquire) {
+                        let _: () = msg_send![
+                            alert,
+                            beginSheetModalForWindow: native_window
+                            completionHandler: block
+                        ];
+                    } else {
+                        let _: () = msg_send![alert, release];
+                    }
                 })
                 .detach();
 
@@ -1277,12 +1300,16 @@ impl PlatformWindow for MacWindow {
     }
 
     fn activate(&self) {
-        let window = self.0.lock().native_window;
-        let executor = self.0.lock().foreground_executor.clone();
+        let lock = self.0.lock();
+        let window = lock.native_window;
+        let closed = lock.closed.clone();
+        let executor = lock.foreground_executor.clone();
         executor
             .spawn(async move {
-                unsafe {
-                    let _: () = msg_send![window, makeKeyAndOrderFront: nil];
+                if !closed.load(Ordering::Acquire) {
+                    unsafe {
+                        let _: () = msg_send![window, makeKeyAndOrderFront: nil];
+                    }
                 }
             })
             .detach();
@@ -1420,11 +1447,12 @@ impl PlatformWindow for MacWindow {
     fn zoom(&self) {
         let this = self.0.lock();
         let window = this.native_window;
+        let closed = this.closed.clone();
         this.foreground_executor
             .spawn(async move {
-                unsafe {
+                if_window_not_closed(closed, || unsafe {
                     window.zoom_(nil);
-                }
+                })
             })
             .detach();
     }
@@ -1432,11 +1460,12 @@ impl PlatformWindow for MacWindow {
     fn toggle_fullscreen(&self) {
         let this = self.0.lock();
         let window = this.native_window;
+        let closed = this.closed.clone();
         this.foreground_executor
             .spawn(async move {
-                unsafe {
+                if_window_not_closed(closed, || unsafe {
                     window.toggleFullScreen_(nil);
-                }
+                })
             })
             .detach();
     }
@@ -1577,45 +1606,48 @@ impl PlatformWindow for MacWindow {
     fn titlebar_double_click(&self) {
         let this = self.0.lock();
         let window = this.native_window;
+        let closed = this.closed.clone();
         this.foreground_executor
             .spawn(async move {
-                unsafe {
-                    let defaults: id = NSUserDefaults::standardUserDefaults();
-                    let domain = ns_string("NSGlobalDomain");
-                    let key = ns_string("AppleActionOnDoubleClick");
-
-                    let dict: id = msg_send![defaults, persistentDomainForName: domain];
-                    let action: id = if !dict.is_null() {
-                        msg_send![dict, objectForKey: key]
-                    } else {
-                        nil
-                    };
+                if_window_not_closed(closed, || {
+                    unsafe {
+                        let defaults: id = NSUserDefaults::standardUserDefaults();
+                        let domain = ns_string("NSGlobalDomain");
+                        let key = ns_string("AppleActionOnDoubleClick");
+
+                        let dict: id = msg_send![defaults, persistentDomainForName: domain];
+                        let action: id = if !dict.is_null() {
+                            msg_send![dict, objectForKey: key]
+                        } else {
+                            nil
+                        };
 
-                    let action_str = if !action.is_null() {
-                        CStr::from_ptr(NSString::UTF8String(action)).to_string_lossy()
-                    } else {
-                        "".into()
-                    };
+                        let action_str = if !action.is_null() {
+                            CStr::from_ptr(NSString::UTF8String(action)).to_string_lossy()
+                        } else {
+                            "".into()
+                        };
 
-                    match action_str.as_ref() {
-                        "None" => {
-                            // "Do Nothing" selected, so do no action
-                        }
-                        "Minimize" => {
-                            window.miniaturize_(nil);
-                        }
-                        "Maximize" => {
-                            window.zoom_(nil);
-                        }
-                        "Fill" => {
-                            // There is no documented API for "Fill" action, so we'll just zoom the window
-                            window.zoom_(nil);
-                        }
-                        _ => {
-                            window.zoom_(nil);
+                        match action_str.as_ref() {
+                            "None" => {
+                                // "Do Nothing" selected, so do no action
+                            }
+                            "Minimize" => {
+                                window.miniaturize_(nil);
+                            }
+                            "Maximize" => {
+                                window.zoom_(nil);
+                            }
+                            "Fill" => {
+                                // There is no documented API for "Fill" action, so we'll just zoom the window
+                                window.zoom_(nil);
+                            }
+                            _ => {
+                                window.zoom_(nil);
+                            }
                         }
                     }
-                }
+                })
             })
             .detach();
     }
@@ -2185,6 +2217,7 @@ extern "C" fn close_window(this: &Object, _: Sel) {
         let close_callback = {
             let window_state = get_window_state(this);
             let mut lock = window_state.as_ref().lock();
+            lock.closed.store(true, Ordering::Release);
             lock.close_callback.take()
         };