diff options
author | Aleksey Kladov <[email protected]> | 2021-01-10 15:02:02 +0000 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2021-01-10 15:02:02 +0000 |
commit | 2ed258ba423788e95885d43724bb9fc1761cc776 (patch) | |
tree | b4f099cf99710feb2fc6684002813fb8682726c2 /crates | |
parent | 77362c71735a8b5ab4b5cd9f396fa657fbffe2cb (diff) |
Fix progress token is already registered crash
After we started reporting progress when running cargo check during
loading, it is possible to crash the client with two identical progress
tokens.
This points to a deeper issue: we might be running several cargo checks
concurrently, which doesn't make sense.
This commit linearizes all workspace fetches, making sure no updates are
lost.
As an additional touch, it also normalizes progress & result reporting,
to make sure they stand in sync.
Diffstat (limited to 'crates')
-rw-r--r-- | crates/rust-analyzer/src/global_state.rs | 3 | ||||
-rw-r--r-- | crates/rust-analyzer/src/lib.rs | 1 | ||||
-rw-r--r-- | crates/rust-analyzer/src/main_loop.rs | 16 | ||||
-rw-r--r-- | crates/rust-analyzer/src/op_queue.rs | 25 | ||||
-rw-r--r-- | crates/rust-analyzer/src/reload.rs | 24 |
5 files changed, 57 insertions, 12 deletions
diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 1f6bf1c8c..442fbd14c 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs | |||
@@ -22,6 +22,7 @@ use crate::{ | |||
22 | from_proto, | 22 | from_proto, |
23 | line_endings::LineEndings, | 23 | line_endings::LineEndings, |
24 | main_loop::Task, | 24 | main_loop::Task, |
25 | op_queue::OpQueue, | ||
25 | reload::SourceRootConfig, | 26 | reload::SourceRootConfig, |
26 | request_metrics::{LatestRequests, RequestMetrics}, | 27 | request_metrics::{LatestRequests, RequestMetrics}, |
27 | thread_pool::TaskPool, | 28 | thread_pool::TaskPool, |
@@ -78,6 +79,7 @@ pub(crate) struct GlobalState { | |||
78 | pub(crate) source_root_config: SourceRootConfig, | 79 | pub(crate) source_root_config: SourceRootConfig, |
79 | pub(crate) proc_macro_client: Option<ProcMacroClient>, | 80 | pub(crate) proc_macro_client: Option<ProcMacroClient>, |
80 | pub(crate) workspaces: Arc<Vec<ProjectWorkspace>>, | 81 | pub(crate) workspaces: Arc<Vec<ProjectWorkspace>>, |
82 | pub(crate) fetch_workspaces_queue: OpQueue, | ||
81 | latest_requests: Arc<RwLock<LatestRequests>>, | 83 | latest_requests: Arc<RwLock<LatestRequests>>, |
82 | } | 84 | } |
83 | 85 | ||
@@ -130,6 +132,7 @@ impl GlobalState { | |||
130 | source_root_config: SourceRootConfig::default(), | 132 | source_root_config: SourceRootConfig::default(), |
131 | proc_macro_client: None, | 133 | proc_macro_client: None, |
132 | workspaces: Arc::new(Vec::new()), | 134 | workspaces: Arc::new(Vec::new()), |
135 | fetch_workspaces_queue: OpQueue::default(), | ||
133 | latest_requests: Default::default(), | 136 | latest_requests: Default::default(), |
134 | } | 137 | } |
135 | } | 138 | } |
diff --git a/crates/rust-analyzer/src/lib.rs b/crates/rust-analyzer/src/lib.rs index c9494e300..2207b9a87 100644 --- a/crates/rust-analyzer/src/lib.rs +++ b/crates/rust-analyzer/src/lib.rs | |||
@@ -35,6 +35,7 @@ mod lsp_utils; | |||
35 | mod thread_pool; | 35 | mod thread_pool; |
36 | mod document; | 36 | mod document; |
37 | mod diff; | 37 | mod diff; |
38 | mod op_queue; | ||
38 | pub mod lsp_ext; | 39 | pub mod lsp_ext; |
39 | pub mod config; | 40 | pub mod config; |
40 | 41 | ||
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 22ee96775..51fb2eb74 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs | |||
@@ -11,7 +11,6 @@ use ide::{Canceled, FileId}; | |||
11 | use ide_db::base_db::VfsPath; | 11 | use ide_db::base_db::VfsPath; |
12 | use lsp_server::{Connection, Notification, Request, Response}; | 12 | use lsp_server::{Connection, Notification, Request, Response}; |
13 | use lsp_types::notification::Notification as _; | 13 | use lsp_types::notification::Notification as _; |
14 | use project_model::ProjectWorkspace; | ||
15 | use vfs::ChangeKind; | 14 | use vfs::ChangeKind; |
16 | 15 | ||
17 | use crate::{ | 16 | use crate::{ |
@@ -62,7 +61,6 @@ enum Event { | |||
62 | pub(crate) enum Task { | 61 | pub(crate) enum Task { |
63 | Response(Response), | 62 | Response(Response), |
64 | Diagnostics(Vec<(FileId, Vec<lsp_types::Diagnostic>)>), | 63 | Diagnostics(Vec<(FileId, Vec<lsp_types::Diagnostic>)>), |
65 | Workspaces(Vec<anyhow::Result<ProjectWorkspace>>), | ||
66 | PrimeCaches(PrimeCachesProgress), | 64 | PrimeCaches(PrimeCachesProgress), |
67 | FetchWorkspace(ProjectWorkspaceProgress), | 65 | FetchWorkspace(ProjectWorkspaceProgress), |
68 | } | 66 | } |
@@ -143,7 +141,8 @@ impl GlobalState { | |||
143 | |_, _| (), | 141 | |_, _| (), |
144 | ); | 142 | ); |
145 | 143 | ||
146 | self.fetch_workspaces(); | 144 | self.fetch_workspaces_request(); |
145 | self.fetch_workspaces_if_needed(); | ||
147 | 146 | ||
148 | while let Some(event) = self.next_event(&inbox) { | 147 | while let Some(event) = self.next_event(&inbox) { |
149 | if let Event::Lsp(lsp_server::Message::Notification(not)) = &event { | 148 | if let Event::Lsp(lsp_server::Message::Notification(not)) = &event { |
@@ -204,7 +203,6 @@ impl GlobalState { | |||
204 | self.diagnostics.set_native_diagnostics(file_id, diagnostics) | 203 | self.diagnostics.set_native_diagnostics(file_id, diagnostics) |
205 | } | 204 | } |
206 | } | 205 | } |
207 | Task::Workspaces(workspaces) => self.switch_workspaces(workspaces), | ||
208 | Task::PrimeCaches(progress) => match progress { | 206 | Task::PrimeCaches(progress) => match progress { |
209 | PrimeCachesProgress::Started => prime_caches_progress.push(progress), | 207 | PrimeCachesProgress::Started => prime_caches_progress.push(progress), |
210 | PrimeCachesProgress::StartedOnCrate { .. } => { | 208 | PrimeCachesProgress::StartedOnCrate { .. } => { |
@@ -224,7 +222,11 @@ impl GlobalState { | |||
224 | ProjectWorkspaceProgress::Report(msg) => { | 222 | ProjectWorkspaceProgress::Report(msg) => { |
225 | (Progress::Report, Some(msg)) | 223 | (Progress::Report, Some(msg)) |
226 | } | 224 | } |
227 | ProjectWorkspaceProgress::End => (Progress::End, None), | 225 | ProjectWorkspaceProgress::End(workspaces) => { |
226 | self.fetch_workspaces_completed(); | ||
227 | self.switch_workspaces(workspaces); | ||
228 | (Progress::End, None) | ||
229 | } | ||
228 | }; | 230 | }; |
229 | self.report_progress("fetching", state, msg, None); | 231 | self.report_progress("fetching", state, msg, None); |
230 | } | 232 | } |
@@ -403,6 +405,8 @@ impl GlobalState { | |||
403 | } | 405 | } |
404 | } | 406 | } |
405 | 407 | ||
408 | self.fetch_workspaces_if_needed(); | ||
409 | |||
406 | let loop_duration = loop_start.elapsed(); | 410 | let loop_duration = loop_start.elapsed(); |
407 | if loop_duration > Duration::from_millis(100) { | 411 | if loop_duration > Duration::from_millis(100) { |
408 | log::warn!("overly long loop turn: {:?}", loop_duration); | 412 | log::warn!("overly long loop turn: {:?}", loop_duration); |
@@ -440,7 +444,7 @@ impl GlobalState { | |||
440 | } | 444 | } |
441 | 445 | ||
442 | RequestDispatcher { req: Some(req), global_state: self } | 446 | RequestDispatcher { req: Some(req), global_state: self } |
443 | .on_sync::<lsp_ext::ReloadWorkspace>(|s, ()| Ok(s.fetch_workspaces()))? | 447 | .on_sync::<lsp_ext::ReloadWorkspace>(|s, ()| Ok(s.fetch_workspaces_request()))? |
444 | .on_sync::<lsp_ext::JoinLines>(|s, p| handlers::handle_join_lines(s.snapshot(), p))? | 448 | .on_sync::<lsp_ext::JoinLines>(|s, p| handlers::handle_join_lines(s.snapshot(), p))? |
445 | .on_sync::<lsp_ext::OnEnter>(|s, p| handlers::handle_on_enter(s.snapshot(), p))? | 449 | .on_sync::<lsp_ext::OnEnter>(|s, p| handlers::handle_on_enter(s.snapshot(), p))? |
446 | .on_sync::<lsp_types::request::Shutdown>(|s, ()| { | 450 | .on_sync::<lsp_types::request::Shutdown>(|s, ()| { |
diff --git a/crates/rust-analyzer/src/op_queue.rs b/crates/rust-analyzer/src/op_queue.rs new file mode 100644 index 000000000..51d66f4b3 --- /dev/null +++ b/crates/rust-analyzer/src/op_queue.rs | |||
@@ -0,0 +1,25 @@ | |||
1 | //! Bookkeeping to make sure only one long-running operation is executed. | ||
2 | |||
3 | #[derive(Default)] | ||
4 | pub(crate) struct OpQueue { | ||
5 | op_scheduled: bool, | ||
6 | op_in_progress: bool, | ||
7 | } | ||
8 | |||
9 | impl OpQueue { | ||
10 | pub(crate) fn request_op(&mut self) { | ||
11 | self.op_scheduled = true; | ||
12 | } | ||
13 | pub(crate) fn should_start_op(&mut self) -> bool { | ||
14 | if !self.op_in_progress && self.op_scheduled { | ||
15 | self.op_in_progress = true; | ||
16 | self.op_scheduled = false; | ||
17 | return true; | ||
18 | } | ||
19 | false | ||
20 | } | ||
21 | pub(crate) fn op_completed(&mut self) { | ||
22 | assert!(self.op_in_progress); | ||
23 | self.op_in_progress = false; | ||
24 | } | ||
25 | } | ||
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index f4e084741..accf2ef8c 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs | |||
@@ -19,7 +19,7 @@ use lsp_ext::StatusParams; | |||
19 | pub(crate) enum ProjectWorkspaceProgress { | 19 | pub(crate) enum ProjectWorkspaceProgress { |
20 | Begin, | 20 | Begin, |
21 | Report(String), | 21 | Report(String), |
22 | End, | 22 | End(Vec<anyhow::Result<ProjectWorkspace>>), |
23 | } | 23 | } |
24 | 24 | ||
25 | impl GlobalState { | 25 | impl GlobalState { |
@@ -30,7 +30,7 @@ impl GlobalState { | |||
30 | self.analysis_host.update_lru_capacity(self.config.lru_capacity()); | 30 | self.analysis_host.update_lru_capacity(self.config.lru_capacity()); |
31 | } | 31 | } |
32 | if self.config.linked_projects() != old_config.linked_projects() { | 32 | if self.config.linked_projects() != old_config.linked_projects() { |
33 | self.fetch_workspaces() | 33 | self.fetch_workspaces_request() |
34 | } else if self.config.flycheck() != old_config.flycheck() { | 34 | } else if self.config.flycheck() != old_config.flycheck() { |
35 | self.reload_flycheck(); | 35 | self.reload_flycheck(); |
36 | } | 36 | } |
@@ -44,7 +44,7 @@ impl GlobalState { | |||
44 | Status::Ready | Status::Invalid => (), | 44 | Status::Ready | Status::Invalid => (), |
45 | } | 45 | } |
46 | if self.config.cargo_autoreload() { | 46 | if self.config.cargo_autoreload() { |
47 | self.fetch_workspaces(); | 47 | self.fetch_workspaces_request(); |
48 | } else { | 48 | } else { |
49 | self.transition(Status::NeedsReload); | 49 | self.transition(Status::NeedsReload); |
50 | } | 50 | } |
@@ -98,8 +98,15 @@ impl GlobalState { | |||
98 | }); | 98 | }); |
99 | } | 99 | } |
100 | } | 100 | } |
101 | pub(crate) fn fetch_workspaces(&mut self) { | 101 | |
102 | pub(crate) fn fetch_workspaces_request(&mut self) { | ||
103 | self.fetch_workspaces_queue.request_op() | ||
104 | } | ||
105 | pub(crate) fn fetch_workspaces_if_needed(&mut self) { | ||
102 | log::info!("will fetch workspaces"); | 106 | log::info!("will fetch workspaces"); |
107 | if !self.fetch_workspaces_queue.should_start_op() { | ||
108 | return; | ||
109 | } | ||
103 | 110 | ||
104 | self.task_pool.handle.spawn_with_sender({ | 111 | self.task_pool.handle.spawn_with_sender({ |
105 | let linked_projects = self.config.linked_projects(); | 112 | let linked_projects = self.config.linked_projects(); |
@@ -133,12 +140,17 @@ impl GlobalState { | |||
133 | }) | 140 | }) |
134 | .collect::<Vec<_>>(); | 141 | .collect::<Vec<_>>(); |
135 | 142 | ||
136 | sender.send(Task::FetchWorkspace(ProjectWorkspaceProgress::End)).unwrap(); | ||
137 | log::info!("did fetch workspaces {:?}", workspaces); | 143 | log::info!("did fetch workspaces {:?}", workspaces); |
138 | sender.send(Task::Workspaces(workspaces)).unwrap() | 144 | sender |
145 | .send(Task::FetchWorkspace(ProjectWorkspaceProgress::End(workspaces))) | ||
146 | .unwrap(); | ||
139 | } | 147 | } |
140 | }); | 148 | }); |
141 | } | 149 | } |
150 | pub(crate) fn fetch_workspaces_completed(&mut self) { | ||
151 | self.fetch_workspaces_queue.op_completed() | ||
152 | } | ||
153 | |||
142 | pub(crate) fn switch_workspaces(&mut self, workspaces: Vec<anyhow::Result<ProjectWorkspace>>) { | 154 | pub(crate) fn switch_workspaces(&mut self, workspaces: Vec<anyhow::Result<ProjectWorkspace>>) { |
143 | let _p = profile::span("GlobalState::switch_workspaces"); | 155 | let _p = profile::span("GlobalState::switch_workspaces"); |
144 | log::info!("will switch workspaces: {:?}", workspaces); | 156 | log::info!("will switch workspaces: {:?}", workspaces); |