From 8dc3e890b22004cbd19e62affd31a408fd43df8d Mon Sep 17 00:00:00 2001 From: "zed-zippy[bot]" <234243425+zed-zippy[bot]@users.noreply.github.com> Date: Mon, 23 Mar 2026 16:44:05 +0000 Subject: [PATCH] gpui_macos: Guard deferred AppKit calls against closed windows (#51992) (cherry-pick to stable) (#52227) Cherry-pick of #51992 to stable ---- 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 --- 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 456ee31ac3b03780e68267621d66435b1ceab4a9..7d30a93d4a0970b51540668bd3e9f7d4051529de 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; @@ -436,6 +439,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, } @@ -760,6 +764,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, }))); @@ -1016,6 +1021,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() @@ -1036,14 +1052,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(); } @@ -1256,15 +1273,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(); @@ -1273,12 +1296,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(); @@ -1416,11 +1443,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(); } @@ -1428,11 +1456,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(); } @@ -1573,45 +1602,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(); } @@ -2174,6 +2206,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() };