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
This commit is contained in:
Oleksiy Syvokon 2026-05-21 06:59:50 +03:00 committed by GitHub
parent ee5c7b6d45
commit ddb847d03a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 52 additions and 8 deletions

View file

@ -709,26 +709,36 @@ impl PickerDelegate for OpenPathDelegate {
) -> Option<Self::ListItem> {
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!({

View file

@ -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);