From 854c6873c799c955f4c8030c2de2e26a6682a24e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 11 Nov 2025 22:42:59 +0100 Subject: [PATCH] Revert "gpui: Fix `RefCell already borrowed` in `WindowsPlatform::run`" (#42481) Reverts zed-industries/zed#42440 There are invalid temporaries in here keeping the borrows alive for longer --- 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, 32 insertions(+), 42 deletions(-) diff --git a/crates/gpui/src/platform/windows/events.rs b/crates/gpui/src/platform/windows/events.rs index cc17f19bcfac86a6f8ac31ec1059d76c24e79695..4e6df63106f4c650ad3130e39d410670ddc4687d 100644 --- a/crates/gpui/src/platform/windows/events.rs +++ b/crates/gpui/src/platform/windows/events.rs @@ -487,12 +487,14 @@ impl WindowsWindowInner { let scale_factor = lock.scale_factor; let wheel_scroll_amount = match modifiers.shift { true => { - self.system_settings() + self.system_settings + .borrow() .mouse_wheel_settings .wheel_scroll_chars } false => { - self.system_settings() + self.system_settings + .borrow() .mouse_wheel_settings .wheel_scroll_lines } @@ -539,7 +541,8 @@ impl WindowsWindowInner { }; let scale_factor = lock.scale_factor; let wheel_scroll_chars = self - .system_settings() + .system_settings + .borrow() .mouse_wheel_settings .wheel_scroll_chars; drop(lock); @@ -674,7 +677,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.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 @@ -1068,7 +1072,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_mut().update(display, wparam.0); + self.system_settings.borrow_mut().update(display, wparam.0); } else { self.handle_system_theme_changed(handle, lparam)?; }; diff --git a/crates/gpui/src/platform/windows/platform.rs b/crates/gpui/src/platform/windows/platform.rs index 72f427beb55b18ff5b94a1a90e334e07045b8726..b985cc14b01b1171d4013bf5c41a0c5199565503 100644 --- a/crates/gpui/src/platform/windows/platform.rs +++ b/crates/gpui/src/platform/windows/platform.rs @@ -342,8 +342,9 @@ impl Platform for WindowsPlatform { } } - self.inner - .with_callback(|callbacks| &mut callbacks.quit, |callback| callback()); + if let Some(ref mut callback) = self.inner.state.borrow_mut().callbacks.quit { + callback(); + } } fn quit(&self) { @@ -577,13 +578,14 @@ impl Platform for WindowsPlatform { fn set_cursor_style(&self, style: CursorStyle) { let hcursor = load_cursor(style); - if self.inner.state.borrow_mut().current_cursor.map(|c| c.0) != hcursor.map(|c| c.0) { + let mut lock = self.inner.state.borrow_mut(); + if lock.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)), ); - self.inner.state.borrow_mut().current_cursor = hcursor; + lock.current_cursor = hcursor; } } @@ -722,18 +724,6 @@ 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( - &self, - project: impl Fn(&mut PlatformCallbacks) -> &mut Option, - 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, handle: HWND, @@ -817,36 +807,40 @@ impl WindowsPlatformInner { } fn handle_dock_action_event(&self, action_idx: usize) -> Option { - let Some(action) = self - .state - .borrow_mut() + let mut lock = self.state.borrow_mut(); + let mut callback = lock.callbacks.app_menu_action.take()?; + let Some(action) = lock .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); }; - self.with_callback( - |callbacks| &mut callbacks.app_menu_action, - |callback| callback(&*action), - ); + drop(lock); + callback(&*action); + self.state.borrow_mut().callbacks.app_menu_action = Some(callback); Some(0) } fn handle_keyboard_layout_change(&self) -> Option { - self.with_callback( - |callbacks| &mut callbacks.keyboard_layout_change, - |callback| callback(), - ); + let mut callback = self + .state + .borrow_mut() + .callbacks + .keyboard_layout_change + .take()?; + callback(); + self.state.borrow_mut().callbacks.keyboard_layout_change = Some(callback); Some(0) } fn handle_device_lost(&self, lparam: LPARAM) -> Option { + 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()); diff --git a/crates/gpui/src/platform/windows/window.rs b/crates/gpui/src/platform/windows/window.rs index 4b89fcffb39d9bfbc0734977cec16a00984f5c9a..0050fa4bc0e96b8702314f33637db67998b5941d 100644 --- a/crates/gpui/src/platform/windows/window.rs +++ b/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, - system_settings: RefCell, + pub(crate) system_settings: RefCell, pub(crate) handle: AnyWindowHandle, pub(crate) hide_title_bar: bool, pub(crate) is_movable: bool, @@ -321,14 +321,6 @@ 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)]