From 542bf6e1067e743cd14f61ea7cd85fa6c6b1bd51 Mon Sep 17 00:00:00 2001
From: Lysastriel <99468824+KiraKiraAyu@users.noreply.github.com>
Date: Tue, 7 Jul 2026 17:02:29 +0800
Subject: [PATCH] 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.
---
 crates/gpui_macos/src/window.rs | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/crates/gpui_macos/src/window.rs b/crates/gpui_macos/src/window.rs
index 938633d474..d3d8760825 100644
--- a/crates/gpui_macos/src/window.rs
+++ b/crates/gpui_macos/src/window.rs
@@ -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);
+    }
+}