From 9bdfd1e98ad27e8e4aba21d0b041c0ab54a5c451 Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Tue, 17 Jun 2025 19:40:59 -0600 Subject: [PATCH] gpui: Fix pending keys dispatch path panic (#32891) For me this is a panic that started occurring today in my use of Zed. The repro is to type `ctrl-x` to start a pending key sequence and then close the collab side panel with the mouse. The issue is that dispatching the action based on pending keystrokes uses the same `DispatchNodeId` as when the 1 second timer was started. `DispatchNodeId` is not stable across frames. This also means that the wrong `DispatchNodeId` can be used in the non-panicing case, potentially causing the action to not occur. The mystery here is why did this only start happening now in my use of Zed, and why isn't it showing up in the panics dashboard / issue reports. Panic looks like ``` { "thread": "main", "payload": "index out of bounds: the len is 467 but the index is 1861", "location_data": { "file": "crates/gpui/src/key_dispatch.rs", "line": 519 }, "backtrace": [ "zed::reliability::init_panic_hook::{{closure}}::he1d8257b19b16eec+155265758", "std::panicking::rust_panic_with_hook::h33b18b24045abff4+128544307", "std::panicking::begin_panic_handler::{{closure}}::hf8313cc2fd0126bc+128543530", "std::sys::backtrace::__rust_end_short_backtrace::h57fe07c8aea5c98a+128537145", "__rustc[95feac21a9532783]::rust_begin_unwind+128542669", "core::panicking::panic_fmt::hd54fb667be51beea+9456688", "core::panicking::panic_bounds_check::h1a9bf3d94de0fc80+9457170", "gpui::key_dispatch::DispatchTree::dispatch_path::hce77d277881569bf+73992023", "gpui::app::App::spawn::{{closure}}::hb1e79bbbdead3012+73687056", "async_task::raw::RawTask::run::hd13f66f99bb24bbd+70694231", "::run::h5a92ddaaf9a06dd1+74465138", "gpui::platform::linux::platform::::run::hd19ac52b2d94268e+74064525", "gpui::app::Application::run::hee83110c717a5af0+151862692", "zed::main::hca7e2265584c4139+153307630", "std::sys::backtrace::__rust_begin_short_backtrace::h2e04f4034c2d82c5+153146899", "std::rt::lang_start::{{closure}}::h91cf1ca0eeae23ae+154454121", "std::rt::lang_start_internal::h418648f91f5be3a1+128467809", "main+153326748", "__libc_start_call_main+25056432783818", "__libc_start_main_impl+25056432784011", "_start+12389486" ], "app_version": "0.190.6", "app_commit_sha": "9a2dcbbe244407fed51d61f38e4a4a59ec1cccc6", "release_channel": "stable", "target": "x86_64-unknown-linux-gnu", "os_name": "Linux X11", "os_version": "ubuntu 24.04", "architecture": "x86_64", "panicked_on": 1750185799233, "system_id": "abae7201-61fb-442b-922b-202071ae81c0", "installation_id": "69a0fb9a-11a2-4065-ad8c-b281e68525ad", "session_id": "bc5b5f2f-e4c3-44a8-948e-c0550a2e2ef2" } ``` Release Notes: - Fixed a rare panic / potential incorrect action dispatch when a pending keysequence is applied after the 1 second timer elapsing. --- crates/gpui/src/key_dispatch.rs | 2 + crates/gpui/src/window.rs | 71 ++++++++++----------------------- 2 files changed, 22 insertions(+), 51 deletions(-) diff --git a/crates/gpui/src/key_dispatch.rs b/crates/gpui/src/key_dispatch.rs index eb6eceeac072e04550d0549711d72b2483016f94..8e2af9422b163afa767e02d48f4b44961ded6bef 100644 --- a/crates/gpui/src/key_dispatch.rs +++ b/crates/gpui/src/key_dispatch.rs @@ -63,6 +63,8 @@ use std::{ rc::Rc, }; +/// ID of a node within `DispatchTree`. Note that these are **not** stable between frames, and so a +/// `DispatchNodeId` should only be used with the `DispatchTree` that provided it. #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] pub(crate) struct DispatchNodeId(usize); diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index c87a65573f88f0ee167b3adf16ed9a670c37005a..f0f4579b2909a805a3297595997965d32ca37ebb 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -1318,21 +1318,13 @@ impl Window { /// Dispatch the given action on the currently focused element. pub fn dispatch_action(&mut self, action: Box, cx: &mut App) { - let focus_handle = self.focused(cx); + let focus_id = self.focused(cx).map(|handle| handle.id); let window = self.handle; cx.defer(move |cx| { window .update(cx, |_, window, cx| { - let node_id = focus_handle - .and_then(|handle| { - window - .rendered_frame - .dispatch_tree - .focusable_node_id(handle.id) - }) - .unwrap_or_else(|| window.rendered_frame.dispatch_tree.root_node_id()); - + let node_id = window.focus_node_id_in_rendered_frame(focus_id); window.dispatch_action_on_node(node_id, action.as_ref(), cx); }) .log_err(); @@ -1709,17 +1701,11 @@ impl Window { /// Determine whether the given action is available along the dispatch path to the currently focused element. pub fn is_action_available(&self, action: &dyn Action, cx: &mut App) -> bool { - let target = self - .focused(cx) - .and_then(|focused_handle| { - self.rendered_frame - .dispatch_tree - .focusable_node_id(focused_handle.id) - }) - .unwrap_or_else(|| self.rendered_frame.dispatch_tree.root_node_id()); + let node_id = + self.focus_node_id_in_rendered_frame(self.focused(cx).map(|handle| handle.id)); self.rendered_frame .dispatch_tree - .is_action_available(action, target) + .is_action_available(action, node_id) } /// The position of the mouse relative to the window. @@ -3484,15 +3470,7 @@ impl Window { self.draw(cx); } - let node_id = self - .focus - .and_then(|focus_id| { - self.rendered_frame - .dispatch_tree - .focusable_node_id(focus_id) - }) - .unwrap_or_else(|| self.rendered_frame.dispatch_tree.root_node_id()); - + let node_id = self.focus_node_id_in_rendered_frame(self.focus); let dispatch_path = self.rendered_frame.dispatch_tree.dispatch_path(node_id); let mut keystroke: Option = None; @@ -3564,6 +3542,7 @@ impl Window { return; }; + let node_id = window.focus_node_id_in_rendered_frame(window.focus); let dispatch_path = window.rendered_frame.dispatch_tree.dispatch_path(node_id); let to_replay = window @@ -3695,15 +3674,7 @@ impl Window { } fn replay_pending_input(&mut self, replays: SmallVec<[Replay; 1]>, cx: &mut App) { - let node_id = self - .focus - .and_then(|focus_id| { - self.rendered_frame - .dispatch_tree - .focusable_node_id(focus_id) - }) - .unwrap_or_else(|| self.rendered_frame.dispatch_tree.root_node_id()); - + let node_id = self.focus_node_id_in_rendered_frame(self.focus); let dispatch_path = self.rendered_frame.dispatch_tree.dispatch_path(node_id); 'replay: for replay in replays { @@ -3739,6 +3710,16 @@ impl Window { } } + fn focus_node_id_in_rendered_frame(&self, focus_id: Option) -> DispatchNodeId { + focus_id + .and_then(|focus_id| { + self.rendered_frame + .dispatch_tree + .focusable_node_id(focus_id) + }) + .unwrap_or_else(|| self.rendered_frame.dispatch_tree.root_node_id()) + } + fn dispatch_action_on_node( &mut self, node_id: DispatchNodeId, @@ -3946,12 +3927,8 @@ impl Window { /// Returns the current context stack. pub fn context_stack(&self) -> Vec { + let node_id = self.focus_node_id_in_rendered_frame(self.focus); let dispatch_tree = &self.rendered_frame.dispatch_tree; - let node_id = self - .focus - .and_then(|focus_id| dispatch_tree.focusable_node_id(focus_id)) - .unwrap_or_else(|| dispatch_tree.root_node_id()); - dispatch_tree .dispatch_path(node_id) .iter() @@ -3961,15 +3938,7 @@ impl Window { /// Returns all available actions for the focused element. pub fn available_actions(&self, cx: &App) -> Vec> { - let node_id = self - .focus - .and_then(|focus_id| { - self.rendered_frame - .dispatch_tree - .focusable_node_id(focus_id) - }) - .unwrap_or_else(|| self.rendered_frame.dispatch_tree.root_node_id()); - + let node_id = self.focus_node_id_in_rendered_frame(self.focus); let mut actions = self.rendered_frame.dispatch_tree.available_actions(node_id); for action_type in cx.global_action_listeners.keys() { if let Err(ix) = actions.binary_search_by_key(action_type, |a| a.as_any().type_id()) {