gpui_macos: Guard start_display_link against nil screens (#60419)
# Objective Prevent a macOS crash in GPUI when AppKit temporarily reports that a visible window has no associated `NSScreen` during display reconfiguration. The crash can happen on this path: ```text NSScreen::deviceDescription gpui_macos::window::display_id_for_screen gpui_macos::window::MacWindowState::start_display_link gpui_macos::window::window_did_change_screen ``` `start_display_link` checked the window occlusion state before creating a display link, but it still assumed `NSWindow.screen()` was non-null. During display changes, sleep/wake, lid close/open, or monitor reconfiguration, AppKit can transiently return `nil` for `screen`, and passing that into `NSScreen::deviceDescription` aborts with a null pointer dereference. I observed this on macOS 26.5 after the system entered a rare display state: once in that state, running GPUI applications would crash immediately after wake. I have not identified the exact OS/display condition that triggers it, so the full wake/reconfiguration scenario is not deterministic, but the crash report consistently points to `NSWindow.screen()` being `nil` when `start_display_link` tries to create a display link. ## Solution Make `display_id_for_screen` explicitly handle a null `NSScreen` by returning `None`. Callers now handle that case by either: - returning early from `start_display_link`, because there is no valid display id to create a display link for yet - skipping a null screen while iterating `NSScreen::screens` The normal non-null screen path is unchanged. This keeps the nil-screen guard at the FFI boundary where `NSScreen::deviceDescription` is called. ## Testing Tested on macOS 26.5. Commands run: ```bash cargo fmt --check -p gpui_macos cargo test -p gpui_macos display_id_for_screen_returns_none_for_null_screen cargo check -p gpui_macos ``` Added a unit test covering the new boundary behavior: `window::tests::display_id_for_screen_returns_none_for_null_screen` ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed a macOS crash that could occur when display configuration changes while a GPUI window is temporarily not associated with a screen.
This commit is contained in:
1 parent
0514cad371
commit
542bf6e106
1 file changed
+23
-4
@@ -655,7 +655,10 @@ impl MacWindowState {
|
||||
return;
|
||||
}
|
||||
}
|
||||
let display_id = unsafe { display_id_for_screen(self.native_window.screen()) };
|
||||
let Some(display_id) = display_id_for_screen(unsafe { self.native_window.screen() }) else {
|
||||
// AppKit can temporarily report no screen while displays are being reconfigured.
|
||||
return;
|
||||
};
|
||||
if let Some(mut display_link) =
|
||||
DisplayLink::new(display_id, self.native_view.as_ptr() as *mut c_void, step).log_err()
|
||||
{
|
||||
@@ -812,8 +815,10 @@ impl MacWindow {
|
||||
let count: u64 = cocoa::foundation::NSArray::count(screens);
|
||||
for i in 0..count {
|
||||
let screen = cocoa::foundation::NSArray::objectAtIndex(screens, i);
|
||||
let Some(display_id) = display_id_for_screen(screen) else {
|
||||
continue;
|
||||
};
|
||||
let frame = NSScreen::frame(screen);
|
||||
let display_id = display_id_for_screen(screen);
|
||||
if display_id == display.0 {
|
||||
screen_frame = Some(frame);
|
||||
target_screen = screen;
|
||||
@@ -2995,13 +3000,17 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
unsafe fn display_id_for_screen(screen: id) -> CGDirectDisplayID {
|
||||
fn display_id_for_screen(screen: id) -> Option<CGDirectDisplayID> {
|
||||
if screen.is_null() {
|
||||
return None;
|
||||
}
|
||||
|
||||
unsafe {
|
||||
let device_description = NSScreen::deviceDescription(screen);
|
||||
let screen_number_key: id = ns_string("NSScreenNumber");
|
||||
let screen_number = device_description.objectForKey_(screen_number_key);
|
||||
let screen_number: NSUInteger = msg_send![screen_number, unsignedIntegerValue];
|
||||
screen_number as CGDirectDisplayID
|
||||
Some(screen_number as CGDirectDisplayID)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3153,3 +3162,13 @@ extern "C" fn toggle_tab_bar(this: &Object, _sel: Sel, _id: id) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn display_id_for_screen_returns_none_for_null_screen() {
|
||||
assert_eq!(display_id_for_screen(nil), None);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user