From 466995cb8125d10d6859beffb835b79b6e68ca6d Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 23 Mar 2026 17:29:32 +0100 Subject: [PATCH] gpui_macos: Guard deferred AppKit calls against closed windows (#51992) 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 ... --- crates/gpui_macos/src/window.rs | 135 ++++++++++++++++++++------------ 1 file changed, 84 insertions(+), 51 deletions(-) diff --git a/crates/gpui_macos/src/window.rs b/crates/gpui_macos/src/window.rs index 002c4719f768d698b5c8b599f039579cfe78640c..0bf7c0a2e7f3c28b9e2e9b7014202bb4581b7278 100644 --- a/crates/gpui_macos/src/window.rs +++ b/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>, toggle_tab_bar_callback: Option>, activated_least_once: bool, + closed: Arc, // The parent window if this window is a sheet (Dialog kind) sheet_parent: Option, } @@ -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, f: impl FnOnce()) { + if !closed.load(Ordering::Acquire) { + f(); + } +} + impl PlatformWindow for MacWindow { fn bounds(&self) -> Bounds { self.0.as_ref().lock().bounds() @@ -1040,14 +1056,15 @@ impl PlatformWindow for MacWindow { fn resize(&mut self, size: Size) { 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() };