zed-industries/zed
1
1
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2024-09-19 18:41:56 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix race when language server registers for workspace/didChangeWatchedFiles (#9177)

Browse Source 
This fixes #8896 by storing the `watched_paths` in a separate HashMap,
allowing us to handle the request even before we mark the language
server as running.

Downside is that we have yet another data structure for language
servers, but it also makes the `Running` enum case a bit smaller.

And it fixes the race condition.

Release Notes:

- Fixed language servers not being notified of file changes if language
server registers for file-notification right after starting up.
([#8896](https://github.com/zed-industries/zed/issues/8896)).

Co-authored-by: Bennet <bennetbo@gmx.de>
Co-authored-by: Remco <djsmits12@gmail.com>
This commit is contained in:
Thorsten Ball 2024-03-11 16:30:30 +01:00 committed by GitHub
parent 0be20d0817
commit f066dd268f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 66 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

139
crates/project/src/project.rs
View File

@ -131,6 +131,7 @@ pub struct Project {
language_server_ids: HashMap<(WorktreeId, LanguageServerName), LanguageServerId>, language_server_ids: HashMap<(WorktreeId, LanguageServerName), LanguageServerId>,
language_server_statuses: BTreeMap<LanguageServerId, LanguageServerStatus>, language_server_statuses: BTreeMap<LanguageServerId, LanguageServerStatus>,
last_workspace_edits_by_language_server: HashMap<LanguageServerId, ProjectTransaction>, last_workspace_edits_by_language_server: HashMap<LanguageServerId, ProjectTransaction>,
language_server_watched_paths: HashMap<LanguageServerId, HashMap<WorktreeId, GlobSet>>,
client: Arc<client::Client>, client: Arc<client::Client>,
next_entry_id: Arc<AtomicUsize>, next_entry_id: Arc<AtomicUsize>,
join_project_response_message_id: u32, join_project_response_message_id: u32,
@ -305,7 +306,6 @@ pub enum LanguageServerState {
language: Arc<Language>, language: Arc<Language>,
adapter: Arc<CachedLspAdapter>, adapter: Arc<CachedLspAdapter>,
server: Arc<LanguageServer>, server: Arc<LanguageServer>,
watched_paths: HashMap<WorktreeId, GlobSet>,
simulate_disk_based_diagnostics_completion: Option<Task<()>>, simulate_disk_based_diagnostics_completion: Option<Task<()>>,
}, },
} }
@ -601,6 +601,7 @@ impl Project {
language_server_ids: HashMap::default(), language_server_ids: HashMap::default(),
language_server_statuses: Default::default(), language_server_statuses: Default::default(),
last_workspace_edits_by_language_server: Default::default(), last_workspace_edits_by_language_server: Default::default(),
language_server_watched_paths: HashMap::default(),
buffers_being_formatted: Default::default(), buffers_being_formatted: Default::default(),
buffers_needing_diff: Default::default(), buffers_needing_diff: Default::default(),
git_diff_debouncer: DebouncedDelay::new(), git_diff_debouncer: DebouncedDelay::new(),
@ -732,6 +733,7 @@ impl Project {
}) })
.collect(), .collect(),
last_workspace_edits_by_language_server: Default::default(), last_workspace_edits_by_language_server: Default::default(),
language_server_watched_paths: HashMap::default(),
opened_buffers: Default::default(), opened_buffers: Default::default(),
buffers_being_formatted: Default::default(), buffers_being_formatted: Default::default(),
buffers_needing_diff: Default::default(), buffers_needing_diff: Default::default(),
@ -3317,7 +3319,6 @@ impl Project {
LanguageServerState::Running { LanguageServerState::Running {
adapter: adapter.clone(), adapter: adapter.clone(),
language: language.clone(), language: language.clone(),
watched_paths: Default::default(),
server: language_server.clone(), server: language_server.clone(),
simulate_disk_based_diagnostics_completion: None, simulate_disk_based_diagnostics_completion: None,
}, },
@ -3462,13 +3463,14 @@ impl Project {
} }
} }
self.language_server_watched_paths.remove(&server_id);
self.language_server_statuses.remove(&server_id); self.language_server_statuses.remove(&server_id);
cx.notify(); cx.notify();
let server_state = self.language_servers.remove(&server_id); let server_state = self.language_servers.remove(&server_id);
cx.emit(Event::LanguageServerRemoved(server_id)); cx.emit(Event::LanguageServerRemoved(server_id));
cx.spawn(move |this, cx| async move { cx.spawn(move |_, cx| async move {
Self::shutdown_language_server(this, server_state, name, server_id, cx).await; Self::shutdown_language_server(server_state, name, cx).await;
orphaned_worktrees orphaned_worktrees
}) })
} else { } else {
@ -3477,11 +3479,9 @@ impl Project {
} }
async fn shutdown_language_server( async fn shutdown_language_server(
this: WeakModel<Project>,
server_state: Option<LanguageServerState>, server_state: Option<LanguageServerState>,
name: Arc<str>, name: Arc<str>,
server_id: LanguageServerId, cx: AsyncAppContext,
mut cx: AsyncAppContext,
) { ) {
let server = match server_state { let server = match server_state {
Some(LanguageServerState::Starting(task)) => { Some(LanguageServerState::Starting(task)) => {
@ -3512,14 +3512,6 @@ impl Project {
shutdown.await; shutdown.await;
} }
} }
if let Some(this) = this.upgrade() {
this.update(&mut cx, |this, cx| {
this.language_server_statuses.remove(&server_id);
cx.notify();
})
.ok();
}
} }
pub fn restart_language_servers_for_buffers( pub fn restart_language_servers_for_buffers(
@ -3890,64 +3882,63 @@ impl Project {
params: DidChangeWatchedFilesRegistrationOptions, params: DidChangeWatchedFilesRegistrationOptions,
cx: &mut ModelContext<Self>, cx: &mut ModelContext<Self>,
) { ) {
if let Some(LanguageServerState::Running { watched_paths, .. }) = let watched_paths = self
self.language_servers.get_mut(&language_server_id) .language_server_watched_paths
{ .entry(language_server_id)
let mut builders = HashMap::default(); .or_default();
for watcher in params.watchers {
for worktree in &self.worktrees { let mut builders = HashMap::default();
if let Some(worktree) = worktree.upgrade() { for watcher in params.watchers {
let glob_is_inside_worktree = worktree.update(cx, |tree, _| { for worktree in &self.worktrees {
if let Some(abs_path) = tree.abs_path().to_str() { if let Some(worktree) = worktree.upgrade() {
let relative_glob_pattern = match &watcher.glob_pattern { let glob_is_inside_worktree = worktree.update(cx, |tree, _| {
lsp::GlobPattern::String(s) => s if let Some(abs_path) = tree.abs_path().to_str() {
.strip_prefix(abs_path) let relative_glob_pattern = match &watcher.glob_pattern {
.and_then(|s| s.strip_prefix(std::path::MAIN_SEPARATOR)), lsp::GlobPattern::String(s) => s
lsp::GlobPattern::Relative(rp) => { .strip_prefix(abs_path)
let base_uri = match &rp.base_uri { .and_then(|s| s.strip_prefix(std::path::MAIN_SEPARATOR)),
lsp::OneOf::Left(workspace_folder) => { lsp::GlobPattern::Relative(rp) => {
&workspace_folder.uri let base_uri = match &rp.base_uri {
} lsp::OneOf::Left(workspace_folder) => &workspace_folder.uri,
lsp::OneOf::Right(base_uri) => base_uri, lsp::OneOf::Right(base_uri) => base_uri,
}; };
base_uri.to_file_path().ok().and_then(|file_path| { base_uri.to_file_path().ok().and_then(|file_path| {
(file_path.to_str() == Some(abs_path)) (file_path.to_str() == Some(abs_path))
.then_some(rp.pattern.as_str()) .then_some(rp.pattern.as_str())
}) })
}
};
if let Some(relative_glob_pattern) = relative_glob_pattern {
let literal_prefix = glob_literal_prefix(relative_glob_pattern);
tree.as_local_mut()
.unwrap()
.add_path_prefix_to_scan(Path::new(literal_prefix).into());
if let Some(glob) = Glob::new(relative_glob_pattern).log_err() {
builders
.entry(tree.id())
.or_insert_with(|| GlobSetBuilder::new())
.add(glob);
}
return true;
} }
};
if let Some(relative_glob_pattern) = relative_glob_pattern {
let literal_prefix = glob_literal_prefix(relative_glob_pattern);
tree.as_local_mut()
.unwrap()
.add_path_prefix_to_scan(Path::new(literal_prefix).into());
if let Some(glob) = Glob::new(relative_glob_pattern).log_err() {
builders
.entry(tree.id())
.or_insert_with(|| GlobSetBuilder::new())
.add(glob);
}
return true;
} }
false
});
if glob_is_inside_worktree {
break;
} }
false
});
if glob_is_inside_worktree {
break;
} }
} }
} }
watched_paths.clear();
for (worktree_id, builder) in builders {
if let Ok(globset) = builder.build() {
watched_paths.insert(worktree_id, globset);
}
}
cx.notify();
} }
watched_paths.clear();
for (worktree_id, builder) in builders {
if let Ok(globset) = builder.build() {
watched_paths.insert(worktree_id, globset);
}
}
cx.notify();
} }
async fn on_lsp_workspace_edit( async fn on_lsp_workspace_edit(
@ -6731,6 +6722,8 @@ impl Project {
self.language_server_ids self.language_server_ids
.remove(&(id_to_remove, server_name)); .remove(&(id_to_remove, server_name));
self.language_server_statuses.remove(&server_id_to_remove); self.language_server_statuses.remove(&server_id_to_remove);
self.language_server_watched_paths
.remove(&server_id_to_remove);
self.last_workspace_edits_by_language_server self.last_workspace_edits_by_language_server
.remove(&server_id_to_remove); .remove(&server_id_to_remove);
self.language_servers.remove(&server_id_to_remove); self.language_servers.remove(&server_id_to_remove);
@ -6985,13 +6978,14 @@ impl Project {
let abs_path = worktree_handle.read(cx).abs_path(); let abs_path = worktree_handle.read(cx).abs_path();
for server_id in &language_server_ids { for server_id in &language_server_ids {
if let Some(LanguageServerState::Running { if let Some(LanguageServerState::Running { server, .. }) =
server, self.language_servers.get(server_id)
watched_paths,
..
}) = self.language_servers.get(server_id)
{ {
if let Some(watched_paths) = watched_paths.get(&worktree_id) { if let Some(watched_paths) = self
.language_server_watched_paths
.get(&server_id)
.and_then(|paths| paths.get(&worktree_id))
{
let params = lsp::DidChangeWatchedFilesParams { let params = lsp::DidChangeWatchedFilesParams {
changes: changes changes: changes
.iter() .iter()
@ -7013,7 +7007,6 @@ impl Project {
}) })
.collect(), .collect(),
}; };
if !params.changes.is_empty() { if !params.changes.is_empty() {
server server
.notify::<lsp::notification::DidChangeWatchedFiles>(params) .notify::<lsp::notification::DidChangeWatchedFiles>(params)