windows: Fix occasional `RefCell already mutably borrowed` panic (#40336)

Lukas Wirth created 3 weeks ago

Release Notes:

- Fixed occasional `RefCell already mutably borrowed` panic in windows
event handling

Change summary

crates/gpui/src/platform/windows/events.rs | 31 +++++++++++++++++------
crates/gpui/src/platform/windows/window.rs |  5 +--
2 files changed, 24 insertions(+), 12 deletions(-)

Detailed changes

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

@@ -530,8 +530,18 @@ impl WindowsWindowInner {
         };
         let scale_factor = lock.scale_factor;
         let wheel_scroll_amount = match modifiers.shift {
-            true => lock.system_settings.mouse_wheel_settings.wheel_scroll_chars,
-            false => lock.system_settings.mouse_wheel_settings.wheel_scroll_lines,
+            true => {
+                self.system_settings
+                    .borrow()
+                    .mouse_wheel_settings
+                    .wheel_scroll_chars
+            }
+            false => {
+                self.system_settings
+                    .borrow()
+                    .mouse_wheel_settings
+                    .wheel_scroll_lines
+            }
         };
         drop(lock);
 
@@ -574,7 +584,11 @@ impl WindowsWindowInner {
             return Some(1);
         };
         let scale_factor = lock.scale_factor;
-        let wheel_scroll_chars = lock.system_settings.mouse_wheel_settings.wheel_scroll_chars;
+        let wheel_scroll_chars = self
+            .system_settings
+            .borrow()
+            .mouse_wheel_settings
+            .wheel_scroll_chars;
         drop(lock);
 
         let wheel_distance =
@@ -707,11 +721,8 @@ 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
-                .state
-                .borrow()
-                .system_settings
-                .auto_hide_taskbar_position
+            && let Some(ref taskbar_position) =
+                self.system_settings.borrow().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
@@ -1101,9 +1112,11 @@ impl WindowsWindowInner {
         if wparam.0 != 0 {
             let mut lock = self.state.borrow_mut();
             let display = lock.display;
-            lock.system_settings.update(display, wparam.0);
             lock.click_state.system_update(wparam.0);
             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);
         } else {
             self.handle_system_theme_changed(handle, lparam)?;
         };

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

@@ -51,7 +51,6 @@ pub struct WindowsWindowState {
     pub renderer: DirectXRenderer,
 
     pub click_state: ClickState,
-    pub system_settings: WindowsSystemSettings,
     pub current_cursor: Option<HCURSOR>,
     pub nc_button_pressed: Option<u32>,
 
@@ -66,6 +65,7 @@ pub(crate) struct WindowsWindowInner {
     pub(super) this: Weak<Self>,
     drop_target_helper: IDropTargetHelper,
     pub(crate) state: RefCell<WindowsWindowState>,
+    pub(crate) system_settings: RefCell<WindowsSystemSettings>,
     pub(crate) handle: AnyWindowHandle,
     pub(crate) hide_title_bar: bool,
     pub(crate) is_movable: bool,
@@ -115,7 +115,6 @@ impl WindowsWindowState {
         let system_key_handled = false;
         let hovered = false;
         let click_state = ClickState::new();
-        let system_settings = WindowsSystemSettings::new(display);
         let nc_button_pressed = None;
         let fullscreen = None;
         let initial_placement = None;
@@ -138,7 +137,6 @@ impl WindowsWindowState {
             hovered,
             renderer,
             click_state,
-            system_settings,
             current_cursor,
             nc_button_pressed,
             display,
@@ -231,6 +229,7 @@ impl WindowsWindowInner {
             validation_number: context.validation_number,
             main_receiver: context.main_receiver.clone(),
             platform_window_handle: context.platform_window_handle,
+            system_settings: RefCell::new(WindowsSystemSettings::new(context.display)),
         }))
     }