gpui: Fix `RefCell already borrowed` in `WindowsPlatform::run` (#42440)

Lukas Wirth created

Fixes ZED-1VX

Release Notes:

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

Change summary

crates/gpui/src/platform/windows/events.rs   | 14 ++---
crates/gpui/src/platform/windows/platform.rs | 50 ++++++++++++---------
crates/gpui/src/platform/windows/window.rs   | 10 +++
3 files changed, 42 insertions(+), 32 deletions(-)

Detailed changes

crates/gpui/src/platform/windows/events.rs 🔗

@@ -487,14 +487,12 @@ impl WindowsWindowInner {
         let scale_factor = lock.scale_factor;
         let wheel_scroll_amount = match modifiers.shift {
             true => {
-                self.system_settings
-                    .borrow()
+                self.system_settings()
                     .mouse_wheel_settings
                     .wheel_scroll_chars
             }
             false => {
-                self.system_settings
-                    .borrow()
+                self.system_settings()
                     .mouse_wheel_settings
                     .wheel_scroll_lines
             }
@@ -541,8 +539,7 @@ impl WindowsWindowInner {
         };
         let scale_factor = lock.scale_factor;
         let wheel_scroll_chars = self
-            .system_settings
-            .borrow()
+            .system_settings()
             .mouse_wheel_settings
             .wheel_scroll_chars;
         drop(lock);
@@ -677,8 +674,7 @@ impl WindowsWindowInner {
         // used by Chrome. However, it may result in one row of pixels being obscured
         // in our client area. But as Chrome says, "there seems to be no better solution."
         if is_maximized
-            && let Some(ref taskbar_position) =
-                self.system_settings.borrow().auto_hide_taskbar_position
+            && let Some(ref taskbar_position) = self.system_settings().auto_hide_taskbar_position
         {
             // For the auto-hide taskbar, adjust in by 1 pixel on taskbar edge,
             // so the window isn't treated as a "fullscreen app", which would cause
@@ -1072,7 +1068,7 @@ impl WindowsWindowInner {
             lock.border_offset.update(handle).log_err();
             // system settings may emit a window message which wants to take the refcell lock, so drop it
             drop(lock);
-            self.system_settings.borrow_mut().update(display, wparam.0);
+            self.system_settings_mut().update(display, wparam.0);
         } else {
             self.handle_system_theme_changed(handle, lparam)?;
         };

crates/gpui/src/platform/windows/platform.rs 🔗

@@ -342,9 +342,8 @@ impl Platform for WindowsPlatform {
             }
         }
 
-        if let Some(ref mut callback) = self.inner.state.borrow_mut().callbacks.quit {
-            callback();
-        }
+        self.inner
+            .with_callback(|callbacks| &mut callbacks.quit, |callback| callback());
     }
 
     fn quit(&self) {
@@ -578,14 +577,13 @@ impl Platform for WindowsPlatform {
 
     fn set_cursor_style(&self, style: CursorStyle) {
         let hcursor = load_cursor(style);
-        let mut lock = self.inner.state.borrow_mut();
-        if lock.current_cursor.map(|c| c.0) != hcursor.map(|c| c.0) {
+        if self.inner.state.borrow_mut().current_cursor.map(|c| c.0) != hcursor.map(|c| c.0) {
             self.post_message(
                 WM_GPUI_CURSOR_STYLE_CHANGED,
                 WPARAM(0),
                 LPARAM(hcursor.map_or(0, |c| c.0 as isize)),
             );
-            lock.current_cursor = hcursor;
+            self.inner.state.borrow_mut().current_cursor = hcursor;
         }
     }
 
@@ -724,6 +722,18 @@ impl WindowsPlatformInner {
         }))
     }
 
+    /// Calls `project` to project to the corresponding callback field, removes it from callbacks, calls `f` with the callback and then puts the callback back.
+    fn with_callback<T>(
+        &self,
+        project: impl Fn(&mut PlatformCallbacks) -> &mut Option<T>,
+        f: impl FnOnce(&mut T),
+    ) {
+        if let Some(mut callback) = project(&mut self.state.borrow_mut().callbacks).take() {
+            f(&mut callback);
+            *project(&mut self.state.borrow_mut().callbacks) = Some(callback)
+        }
+    }
+
     fn handle_msg(
         self: &Rc<Self>,
         handle: HWND,
@@ -807,40 +817,36 @@ impl WindowsPlatformInner {
     }
 
     fn handle_dock_action_event(&self, action_idx: usize) -> Option<isize> {
-        let mut lock = self.state.borrow_mut();
-        let mut callback = lock.callbacks.app_menu_action.take()?;
-        let Some(action) = lock
+        let Some(action) = self
+            .state
+            .borrow_mut()
             .jump_list
             .dock_menus
             .get(action_idx)
             .map(|dock_menu| dock_menu.action.boxed_clone())
         else {
-            lock.callbacks.app_menu_action = Some(callback);
             log::error!("Dock menu for index {action_idx} not found");
             return Some(1);
         };
-        drop(lock);
-        callback(&*action);
-        self.state.borrow_mut().callbacks.app_menu_action = Some(callback);
+        self.with_callback(
+            |callbacks| &mut callbacks.app_menu_action,
+            |callback| callback(&*action),
+        );
         Some(0)
     }
 
     fn handle_keyboard_layout_change(&self) -> Option<isize> {
-        let mut callback = self
-            .state
-            .borrow_mut()
-            .callbacks
-            .keyboard_layout_change
-            .take()?;
-        callback();
-        self.state.borrow_mut().callbacks.keyboard_layout_change = Some(callback);
+        self.with_callback(
+            |callbacks| &mut callbacks.keyboard_layout_change,
+            |callback| callback(),
+        );
         Some(0)
     }
 
     fn handle_device_lost(&self, lparam: LPARAM) -> Option<isize> {
-        let mut lock = self.state.borrow_mut();
         let directx_devices = lparam.0 as *const DirectXDevices;
         let directx_devices = unsafe { &*directx_devices };
+        let mut lock = self.state.borrow_mut();
         lock.directx_devices.take();
         lock.directx_devices = Some(directx_devices.clone());
 

crates/gpui/src/platform/windows/window.rs 🔗

@@ -63,7 +63,7 @@ pub(crate) struct WindowsWindowInner {
     hwnd: HWND,
     drop_target_helper: IDropTargetHelper,
     pub(crate) state: RefCell<WindowsWindowState>,
-    pub(crate) system_settings: RefCell<WindowsSystemSettings>,
+    system_settings: RefCell<WindowsSystemSettings>,
     pub(crate) handle: AnyWindowHandle,
     pub(crate) hide_title_bar: bool,
     pub(crate) is_movable: bool,
@@ -321,6 +321,14 @@ impl WindowsWindowInner {
         }
         Ok(())
     }
+
+    pub(crate) fn system_settings(&self) -> std::cell::Ref<'_, WindowsSystemSettings> {
+        self.system_settings.borrow()
+    }
+
+    pub(crate) fn system_settings_mut(&self) -> std::cell::RefMut<'_, WindowsSystemSettings> {
+        self.system_settings.borrow_mut()
+    }
 }
 
 #[derive(Default)]