From ddb847d03af4a45d987dc0405f7930922656fdfd Mon Sep 17 00:00:00 2001 From: Oleksiy Syvokon Date: Thu, 21 May 2026 06:59:50 +0300 Subject: [PATCH] Fix race condition in OpenPathDelegate (#57290) Directory entries and string matches can briefly go out of sync during async updates. When that happens, we used to highlight the wrong substring (best case), or panic if the offset happens to be mid-Unicode or out of range. After this change, we fall back to rendering a row without highlights when matches are out of sync. The highlight will be shown on the next frame. This is a rare condition and the easiest fix, so should be acceptable. Closes FR-11 Release Notes: - Fixed rare panic in the open path dialog --- .../open_path_prompt/src/open_path_prompt.rs | 24 +++++++++---- .../src/open_path_prompt_tests.rs | 36 ++++++++++++++++++- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/crates/open_path_prompt/src/open_path_prompt.rs b/crates/open_path_prompt/src/open_path_prompt.rs index 607dfb13e4..8cfcb115cd 100644 --- a/crates/open_path_prompt/src/open_path_prompt.rs +++ b/crates/open_path_prompt/src/open_path_prompt.rs @@ -709,26 +709,36 @@ impl PickerDelegate for OpenPathDelegate { ) -> Option { let settings = FileFinderSettings::get_global(cx); let candidate = self.get_entry(ix)?; - let mut match_positions = match &self.directory_state { - DirectoryState::List { .. } => self.string_matches.get(ix)?.positions.clone(), + let string_match = match &self.directory_state { + DirectoryState::List { .. } => self.string_matches.get(ix), DirectoryState::Create { user_input, .. } => { if let Some(user_input) = user_input { if !user_input.exists || !user_input.is_dir { if ix == 0 { - Vec::new() + None } else { - self.string_matches.get(ix - 1)?.positions.clone() + self.string_matches.get(ix - 1) } } else { - self.string_matches.get(ix)?.positions.clone() + self.string_matches.get(ix) } } else { - self.string_matches.get(ix)?.positions.clone() + self.string_matches.get(ix) } } - DirectoryState::None { .. } => Vec::new(), + DirectoryState::None { .. } => None, }; + // Directory entries and string matches can briefly go out of sync during + // async updates. When that happens, render the row without highlights. + let mut match_positions = string_match + .filter(|string_match| { + string_match.candidate_id == candidate.path.id + && string_match.string == candidate.path.string + }) + .map(|string_match| string_match.positions.clone()) + .unwrap_or_default(); + let is_current_dir_candidate = candidate.path.string == self.current_dir(); let file_icon = maybe!({ diff --git a/crates/open_path_prompt/src/open_path_prompt_tests.rs b/crates/open_path_prompt/src/open_path_prompt_tests.rs index 7d359dbf17..d89b657860 100644 --- a/crates/open_path_prompt/src/open_path_prompt_tests.rs +++ b/crates/open_path_prompt/src/open_path_prompt_tests.rs @@ -1,5 +1,6 @@ use std::sync::Arc; +use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{AppContext, Entity, TestAppContext, VisualTestContext}; use picker::{Picker, PickerDelegate}; use project::Project; @@ -8,7 +9,7 @@ use ui::rems; use util::path; use workspace::{AppState, MultiWorkspace}; -use crate::OpenPathDelegate; +use crate::{CandidateInfo, DirectoryState, OpenPathDelegate}; #[gpui::test] async fn test_open_path_prompt(cx: &mut TestAppContext) { @@ -372,6 +373,39 @@ async fn test_new_path_prompt(cx: &mut TestAppContext) { assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1"]); } +#[gpui::test] +async fn test_open_path_prompt_panics_with_stale_highlight_positions(cx: &mut TestAppContext) { + let app_state = init_test(cx); + app_state + .fs + .as_fake() + .insert_tree(path!("/root"), json!({})) + .await; + + let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await; + let (picker, cx) = build_open_path_prompt(project, false, false, cx); + + picker.update_in(cx, |picker, window, cx| { + picker.delegate.prompt_root = "/".to_string(); + picker.delegate.directory_state = DirectoryState::List { + parent_path: picker.delegate.prompt_root.clone(), + entries: vec![CandidateInfo { + path: StringMatchCandidate::new(0, "éclair"), + is_dir: false, + }], + error: None, + }; + picker.delegate.string_matches = vec![StringMatch { + candidate_id: 0, + score: 0.0, + positions: vec![1], + string: "ab".to_string(), + }]; + + picker.delegate.render_match(0, false, window, cx); + }); +} + #[gpui::test] async fn test_open_path_prompt_with_show_hidden(cx: &mut TestAppContext) { let app_state = init_test(cx);