From 1d8bd151b712a055aaf25c4bfe3b5109581dfe9c Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 8 Jan 2025 18:05:34 -0500 Subject: [PATCH] Fix double read panic in nav history (#22754) This one seems to be triggered when the assistant's `View` is leased during the call into `NavHistory::for_each_entry`, which then tries to read it again through the `ItemHandle` interface. Fix it by skipping entries that can't be read in the history iteration. Release Notes: - N/A --- crates/assistant/src/assistant_panel.rs | 4 ++++ crates/workspace/src/item.rs | 9 +++++++++ crates/workspace/src/pane.rs | 10 ++++++++-- crates/workspace/src/workspace.rs | 6 ++++-- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index 712aaa845ab6a2c51ee759c0e9984864f4b14ac8..0fa0557e9c5f196862d87a9ff1c61cfd0cd19d7d 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -4272,6 +4272,10 @@ impl Item for ContextEditor { None } } + + fn include_in_nav_history() -> bool { + false + } } impl SearchableItem for ContextEditor { diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index 3a3a61b319a101937657cdba1887a33faa455335..5796d64f87780687905124d4714fca48fb8b706b 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -320,6 +320,10 @@ pub trait Item: FocusableView + EventEmitter { fn preserve_preview(&self, _cx: &AppContext) -> bool { false } + + fn include_in_nav_history() -> bool { + true + } } pub trait SerializableItem: Item { @@ -464,6 +468,7 @@ pub trait ItemHandle: 'static + Send { fn downgrade_item(&self) -> Box; fn workspace_settings<'a>(&self, cx: &'a AppContext) -> &'a WorkspaceSettings; fn preserve_preview(&self, cx: &AppContext) -> bool; + fn include_in_nav_history(&self) -> bool; } pub trait WeakItemHandle: Send + Sync { @@ -877,6 +882,10 @@ impl ItemHandle for View { fn preserve_preview(&self, cx: &AppContext) -> bool { self.read(cx).preserve_preview(cx) } + + fn include_in_nav_history(&self) -> bool { + T::include_in_nav_history() + } } impl From> for AnyView { diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 33808ffb3cfe3fada03f6627d1df0c19700f3e81..607d5b7bcc39064292b1d92ab101ad95940be053 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -3095,8 +3095,14 @@ impl Render for Pane { impl ItemNavHistory { pub fn push(&mut self, data: Option, cx: &mut WindowContext) { - self.history - .push(data, self.item.clone(), self.is_preview, cx); + if self + .item + .upgrade() + .is_some_and(|item| item.include_in_nav_history()) + { + self.history + .push(data, self.item.clone(), self.is_preview, cx); + } } pub fn pop_backward(&mut self, cx: &mut WindowContext) -> Option { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 25fa708c7be4891a527e6685cff00683d427c3d1..3a4f52cd02112e6924047b37d0ff54db34d66fe4 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2975,13 +2975,15 @@ impl Workspace { match target { Some(ActivateInDirectionTarget::Pane(pane)) => cx.focus_view(&pane), Some(ActivateInDirectionTarget::Dock(dock)) => { - dock.update(cx, |dock, cx| { + // Defer this to avoid a panic when the dock's active panel is already on the stack. + cx.defer(move |cx| { + let dock = dock.read(cx); if let Some(panel) = dock.active_panel() { panel.focus_handle(cx).focus(cx); } else { log::error!("Could not find a focus target when in switching focus in {direction} direction for a {:?} dock", dock.position()); } - }); + }) } None => {} }