From 2d0510e226ea857bcd2f14845740b0b20d3048a2 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 4 Jun 2023 09:09:25 +0200 Subject: [PATCH] Add mandatory panic contexts to all threadpool tasks --- crates/rust-analyzer/src/dispatch.rs | 14 +- .../src/handlers/notification.rs | 14 +- crates/rust-analyzer/src/main_loop.rs | 140 ++++++++++-------- crates/rust-analyzer/src/reload.rs | 116 ++++++++------- crates/rust-analyzer/src/task_pool.rs | 28 +++- 5 files changed, 174 insertions(+), 138 deletions(-) diff --git a/crates/rust-analyzer/src/dispatch.rs b/crates/rust-analyzer/src/dispatch.rs index ebe77b8dfe7..3527d92a4ec 100644 --- a/crates/rust-analyzer/src/dispatch.rs +++ b/crates/rust-analyzer/src/dispatch.rs @@ -104,13 +104,10 @@ pub(crate) fn on_no_retry( None => return self, }; - self.global_state.task_pool.handle.spawn(ThreadIntent::Worker, { + self.global_state.task_pool.handle.spawn(ThreadIntent::Worker, panic_context, { let world = self.global_state.snapshot(); move || { - let result = panic::catch_unwind(move || { - let _pctx = stdx::panic_context::enter(panic_context); - f(world, params) - }); + let result = panic::catch_unwind(move || f(world, params)); match thread_result_to_response::(req.id.clone(), result) { Ok(response) => Task::Response(response), Err(_) => Task::Response(lsp_server::Response::new_err( @@ -178,13 +175,10 @@ fn on_with_thread_intent( None => return self, }; - self.global_state.task_pool.handle.spawn(intent, { + self.global_state.task_pool.handle.spawn(intent, panic_context, { let world = self.global_state.snapshot(); move || { - let result = panic::catch_unwind(move || { - let _pctx = stdx::panic_context::enter(panic_context); - f(world, params) - }); + let result = panic::catch_unwind(move || f(world, params)); match thread_result_to_response::(req.id.clone(), result) { Ok(response) => Task::Response(response), Err(_) => Task::Retry(req), diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index 09de6900c8f..b3623669cc6 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -291,11 +291,15 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool { } Ok(()) }; - state.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, move |_| { - if let Err(e) = std::panic::catch_unwind(task) { - tracing::error!("flycheck task panicked: {e:?}") - } - }); + state.task_pool.handle.spawn_with_sender( + stdx::thread::ThreadIntent::Worker, + "flycheck", + move |_| { + if let Err(e) = std::panic::catch_unwind(task) { + tracing::error!("flycheck task panicked: {e:?}") + } + }, + ); true } else { false diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 19c49a23000..92d44eeee89 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -397,19 +397,25 @@ fn prime_caches(&mut self, cause: String) { tracing::debug!(%cause, "will prime caches"); let num_worker_threads = self.config.prime_caches_num_threads(); - self.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, { - let analysis = self.snapshot().analysis; - move |sender| { - sender.send(Task::PrimeCaches(PrimeCachesProgress::Begin)).unwrap(); - let res = analysis.parallel_prime_caches(num_worker_threads, |progress| { - let report = PrimeCachesProgress::Report(progress); - sender.send(Task::PrimeCaches(report)).unwrap(); - }); - sender - .send(Task::PrimeCaches(PrimeCachesProgress::End { cancelled: res.is_err() })) - .unwrap(); - } - }); + self.task_pool.handle.spawn_with_sender( + stdx::thread::ThreadIntent::Worker, + "prime_caches", + { + let analysis = self.snapshot().analysis; + move |sender| { + sender.send(Task::PrimeCaches(PrimeCachesProgress::Begin)).unwrap(); + let res = analysis.parallel_prime_caches(num_worker_threads, |progress| { + let report = PrimeCachesProgress::Report(progress); + sender.send(Task::PrimeCaches(report)).unwrap(); + }); + sender + .send(Task::PrimeCaches(PrimeCachesProgress::End { + cancelled: res.is_err(), + })) + .unwrap(); + } + }, + ); } fn update_status_or_notify(&mut self) { @@ -796,56 +802,62 @@ fn update_diagnostics(&mut self) { // Diagnostics are triggered by the user typing // so we run them on a latency sensitive thread. - self.task_pool.handle.spawn(stdx::thread::ThreadIntent::LatencySensitive, move || { - let _p = profile::span("publish_diagnostics"); - let diagnostics = subscriptions - .into_iter() - .filter_map(|file_id| { - let line_index = snapshot.file_line_index(file_id).ok()?; - Some(( - file_id, - line_index, - snapshot - .analysis - .diagnostics( - &snapshot.config.diagnostics(), - ide::AssistResolveStrategy::None, - file_id, - ) - .ok()?, - )) - }) - .map(|(file_id, line_index, it)| { - ( - file_id, - it.into_iter() - .map(move |d| lsp_types::Diagnostic { - range: crate::to_proto::range(&line_index, d.range), - severity: Some(crate::to_proto::diagnostic_severity(d.severity)), - code: Some(lsp_types::NumberOrString::String( - d.code.as_str().to_string(), - )), - code_description: Some(lsp_types::CodeDescription { - href: lsp_types::Url::parse(&format!( - "https://rust-analyzer.github.io/manual.html#{}", - d.code.as_str() - )) - .unwrap(), - }), - source: Some("rust-analyzer".to_string()), - message: d.message, - related_information: None, - tags: if d.unused { - Some(vec![lsp_types::DiagnosticTag::UNNECESSARY]) - } else { - None - }, - data: None, - }) - .collect::>(), - ) - }); - Task::Diagnostics(diagnostics.collect()) - }); + self.task_pool.handle.spawn( + stdx::thread::ThreadIntent::LatencySensitive, + "publish_diagnostics", + move || { + let _p = profile::span("publish_diagnostics"); + let diagnostics = subscriptions + .into_iter() + .filter_map(|file_id| { + let line_index = snapshot.file_line_index(file_id).ok()?; + Some(( + file_id, + line_index, + snapshot + .analysis + .diagnostics( + &snapshot.config.diagnostics(), + ide::AssistResolveStrategy::None, + file_id, + ) + .ok()?, + )) + }) + .map(|(file_id, line_index, it)| { + ( + file_id, + it.into_iter() + .map(move |d| lsp_types::Diagnostic { + range: crate::to_proto::range(&line_index, d.range), + severity: Some(crate::to_proto::diagnostic_severity( + d.severity, + )), + code: Some(lsp_types::NumberOrString::String( + d.code.as_str().to_string(), + )), + code_description: Some(lsp_types::CodeDescription { + href: lsp_types::Url::parse(&format!( + "https://rust-analyzer.github.io/manual.html#{}", + d.code.as_str() + )) + .unwrap(), + }), + source: Some("rust-analyzer".to_string()), + message: d.message, + related_information: None, + tags: if d.unused { + Some(vec![lsp_types::DiagnosticTag::UNNECESSARY]) + } else { + None + }, + data: None, + }) + .collect::>(), + ) + }); + Task::Diagnostics(diagnostics.collect()) + }, + ); } } diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 6e8c8ea91a1..5911e24d999 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -185,7 +185,7 @@ pub(crate) fn current_status(&self) -> lsp_ext::ServerStatusParams { pub(crate) fn fetch_workspaces(&mut self, cause: Cause) { tracing::info!(%cause, "will fetch workspaces"); - self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, { + self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, "fetch_workspaces", { let linked_projects = self.config.linked_projects(); let detached_files = self.config.detached_files().to_vec(); let cargo_config = self.config.cargo(); @@ -260,19 +260,25 @@ pub(crate) fn fetch_build_data(&mut self, cause: Cause) { tracing::info!(%cause, "will fetch build data"); let workspaces = Arc::clone(&self.workspaces); let config = self.config.cargo(); - self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, move |sender| { - sender.send(Task::FetchBuildData(BuildDataProgress::Begin)).unwrap(); + self.task_pool.handle.spawn_with_sender( + ThreadIntent::Worker, + "fetch_build_data", + move |sender| { + sender.send(Task::FetchBuildData(BuildDataProgress::Begin)).unwrap(); - let progress = { - let sender = sender.clone(); - move |msg| { - sender.send(Task::FetchBuildData(BuildDataProgress::Report(msg))).unwrap() - } - }; - let res = ProjectWorkspace::run_all_build_scripts(&workspaces, &config, &progress); + let progress = { + let sender = sender.clone(); + move |msg| { + sender.send(Task::FetchBuildData(BuildDataProgress::Report(msg))).unwrap() + } + }; + let res = ProjectWorkspace::run_all_build_scripts(&workspaces, &config, &progress); - sender.send(Task::FetchBuildData(BuildDataProgress::End((workspaces, res)))).unwrap(); - }); + sender + .send(Task::FetchBuildData(BuildDataProgress::End((workspaces, res)))) + .unwrap(); + }, + ); } pub(crate) fn fetch_proc_macros(&mut self, cause: Cause, paths: Vec) { @@ -280,50 +286,54 @@ pub(crate) fn fetch_proc_macros(&mut self, cause: Cause, paths: Vec, threads: usize) -> TaskPool TaskPool { sender, pool: Pool::new(threads) } } - pub(crate) fn spawn(&mut self, intent: ThreadIntent, task: F) - where + pub(crate) fn spawn( + &mut self, + intent: ThreadIntent, + panic_context: impl Into, + task: F, + ) where F: FnOnce() -> T + Send + 'static, T: Send + 'static, { + let panic_context = panic_context.into(); self.pool.spawn(intent, { let sender = self.sender.clone(); - move || sender.send(task()).unwrap() + move || { + let _pctx = stdx::panic_context::enter(panic_context); + sender.send(task()).unwrap() + } }) } - pub(crate) fn spawn_with_sender(&mut self, intent: ThreadIntent, task: F) - where + pub(crate) fn spawn_with_sender( + &mut self, + intent: ThreadIntent, + panic_context: impl Into, + task: F, + ) where F: FnOnce(Sender) + Send + 'static, T: Send + 'static, { + let panic_context = panic_context.into(); self.pool.spawn(intent, { let sender = self.sender.clone(); - move || task(sender) + move || { + let _pctx = stdx::panic_context::enter(panic_context); + task(sender) + } }) }