text_finder: Fix crash when dismissing the finder (#60437)
Some checks are pending
deploy_nightly_docs / deploy_docs (push) Waiting to run
run_tests / orchestrate (push) Waiting to run
run_tests / check_style (push) Waiting to run
run_tests / clippy_windows (push) Blocked by required conditions
run_tests / clippy_linux (push) Blocked by required conditions
run_tests / clippy_mac (push) Blocked by required conditions
run_tests / clippy_mac_x86_64 (push) Blocked by required conditions
run_tests / run_tests_windows (push) Blocked by required conditions
run_tests / run_tests_linux (push) Blocked by required conditions
run_tests / run_tests_mac (push) Blocked by required conditions
run_tests / miri_scheduler (push) Blocked by required conditions
run_tests / doctests (push) Blocked by required conditions
run_tests / check_workspace_binaries (push) Blocked by required conditions
run_tests / build_visual_tests_binary (push) Blocked by required conditions
run_tests / check_wasm (push) Blocked by required conditions
run_tests / check_dependencies (push) Blocked by required conditions
run_tests / check_docs (push) Blocked by required conditions
run_tests / check_licenses (push) Blocked by required conditions
run_tests / check_scripts (push) Blocked by required conditions
run_tests / check_postgres_and_protobuf_migrations (push) Blocked by required conditions
run_tests / extension_tests (push) Blocked by required conditions
run_tests / tests_pass (push) Blocked by required conditions
Congratsbot / check-author (push) Waiting to run
Congratsbot / congrats (push) Blocked by required conditions

# Objective

Fixes #60436

Pressing Escape (or triggering any workspace-level action that closes
modals, such as `pane::DeploySearch` / `search::NewSearch`) while the
Text Finder was open crashes the app with:

```
cannot read workspace::Workspace while it is already being updated
crates/gpui/src/app/entity_map.rs:164
```

`TextFinder::on_before_dismiss` read the `Workspace` entity
(`workspace.read(cx).database_id()`) to persist the last search query.
Dismissal can be initiated from inside a `workspace.register_action`
handler — e.g. `buffer_search`'s `SearchActionsRegistrar` calls
`workspace.hide_modal(...)` while the `Workspace` entity is leased — so
the modal layer invokes `on_before_dismiss` synchronously under that
lease, and the read trips GPUI's re-entrancy guard. Since the finder
seeds the last query on open, the query is non-empty immediately, making
the crash reproducible on the very first Escape.

Dump file analysis confirms the diagnosis.

## Solution

Stop reading the `Workspace` entity in the dismiss path. Both places
that create the modal (`TextFinder::open` and
`TextFinder::open_from_project_search`) already run inside
`workspace.update_in`, where `workspace.database_id()` is a plain field
access on `&mut Workspace`. Capture the `Option<WorkspaceId>` there,
store it on `TextFinder`, and have `on_before_dismiss` use the stored
id. The dismiss path now touches no entity that can be mid-update, so it
is safe regardless of which code path initiates `hide_modal`. The
remaining reads in `on_before_dismiss` (`picker` and its query editor)
are never leased in any dismissal chain.

The `id` is captured once at creation; a workspace that gains a database
id during the modal's lifetime would persist under `None` (i.e. skip
persistence), which matches the previous behavior for unpersisted
workspaces.

## Testing

- Verified the fix by code review and re-tested in a rebuilt bundle
(local build).
- To reproduce/verify: open the Text Finder (`text_finder::Toggle`),
type a query (or rely on a seeded one), then press Escape with a binding
that resolves to `editor::Cancel`, or press `cmd-shift-f`
(`DeploySearch`) while the finder is open. Before this change the app
crashes. After it, the modal dismisses and the query is persisted.
- Tested on macOS 26.5.1 (arm64); the affected code is
platform-independent.

## Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks — no unsafe blocks
- [x] The content adheres to Zed's UI standards — no UI changes
- [x] Tests cover the new/changed behavior — added
- [x] Performance impact has been considered and is acceptable — one
`Option<WorkspaceId>` copy at modal creation

---

Release Notes:

- Fixed a crash when closing the Text Finder while a workspace action
(e.g. Escape via `editor::Cancel`, or deploying search) triggered the
dismissal
This commit is contained in:
Yevhen Nakonechnyi 2026-07-06 06:47:21 -07:00 committed by GitHub
parent b1f4563909
commit 050e6f3407
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -34,6 +34,7 @@ actions!(text_finder, [ToProjectSearch, Fold, Unfold, ToggleFoldAll]);
pub struct TextFinder {
picker: Entity<Picker<Delegate>>,
init_modifiers: Option<Modifiers>,
workspace_id: Option<WorkspaceId>,
_subscription: Subscription,
}
@ -158,8 +159,9 @@ impl TextFinder {
workspace
.update_in(cx, |workspace, window, cx| {
remove_project_search_tab(project_search_item_id, workspace, window, cx);
let workspace_id = workspace.database_id();
workspace.toggle_modal(window, cx, |window, cx| {
Self::new(delegate, None, window, cx)
Self::new(delegate, None, workspace_id, window, cx)
});
})
.ok();
@ -380,8 +382,9 @@ impl TextFinder {
let delegate = delegate_task.await;
workspace
.update_in(cx, |workspace, window, cx| {
let workspace_id = workspace.database_id();
workspace.toggle_modal(window, cx, |window, cx| {
Self::new(delegate, seed_query, window, cx)
Self::new(delegate, seed_query, workspace_id, window, cx)
});
})
.ok();
@ -391,6 +394,7 @@ impl TextFinder {
fn new(
delegate: Delegate,
seed_query: Option<SearchSeed>,
workspace_id: Option<WorkspaceId>,
window: &mut Window,
cx: &mut Context<Self>,
) -> Self {
@ -418,6 +422,7 @@ impl TextFinder {
Self {
picker,
init_modifiers: window.modifiers().modified().then_some(window.modifiers()),
workspace_id,
_subscription: subscription,
}
}
@ -450,11 +455,7 @@ impl ModalView for TextFinder {
let query = picker.query(cx);
if !query.is_empty() {
let options = picker.delegate.search_options;
let workspace_id = self
.weak_workspace(cx)
.upgrade()
.and_then(|workspace| workspace.read(cx).database_id());
store_last_search(workspace_id, query, options, cx);
store_last_search(self.workspace_id, query, options, cx);
}
DismissDecision::Dismiss(true)
}
@ -478,3 +479,74 @@ pub struct SearchMatch {
pub line_text: String,
pub line_number: u32,
}
#[cfg(test)]
mod tests {
use gpui::{TestAppContext, VisualTestContext};
use project::{FakeFs, Project};
use serde_json::json;
use settings::SettingsStore;
use util::path;
use workspace::MultiWorkspace;
use super::*;
fn init_test(cx: &mut TestAppContext) {
cx.update(|cx| {
let settings = SettingsStore::test(cx);
cx.set_global(settings);
theme_settings::init(theme::LoadThemes::JustBase, cx);
editor::init(cx);
crate::init(cx);
});
}
/// Dismissal can be initiated from inside a workspace update: workspace-level
/// action handlers (e.g. buffer search's `SearchActionsRegistrar`) call
/// `Workspace::hide_modal` while the workspace entity is leased, which runs
/// `on_before_dismiss` synchronously under that lease. Reading the workspace
/// entity there panics with "cannot read workspace::Workspace while it is
/// already being updated", so this test dismisses the finder the same way.
#[gpui::test]
async fn test_dismiss_from_within_workspace_update(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.background_executor.clone());
fs.insert_tree(path!("/dir"), json!({"one.rs": "const ONE: usize = 1;"}))
.await;
let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await;
let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx));
let workspace = window
.read_with(cx, |multi_workspace, _| multi_workspace.workspace().clone())
.unwrap();
let cx = &mut VisualTestContext::from_window(window.into(), cx);
// Seed a query: the last-search persistence in `on_before_dismiss` (the
// code path that read the workspace entity) only runs when the query is
// non-empty, which is the common case in practice since the finder seeds
// the previous query on open.
let seed_query = SearchSeed {
query: "ONE".to_string(),
options: None,
};
workspace
.update_in(cx, |_, window, cx| {
TextFinder::open(Some(seed_query), window, cx)
})
.await;
workspace.update(cx, |workspace, cx| {
assert!(workspace.active_modal::<TextFinder>(cx).is_some());
});
workspace.update_in(cx, |workspace, window, cx| {
assert!(workspace.hide_modal(window, cx));
});
workspace.update(cx, |workspace, cx| {
assert!(workspace.active_modal::<TextFinder>(cx).is_none());
});
}
}