Skip to content

Commit de2320e

Browse files
borsbors
committed
Auto merge of #14965 - Veykril:panic-ctx, r=Veykril
Add mandatory panic contexts to all threadpool tasks the diagnostics task is panicking I think, but without this you can't really tell because the stack trace ends in a generic iterator fold call instead of something specific.
2 parents 4fb1df6 + 2d0510e commit de2320e

File tree

5 files changed

+174
-138
lines changed

5 files changed

+174
-138
lines changed

‎crates/rust-analyzer/src/dispatch.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,10 @@ impl<'a> RequestDispatcher<'a> {
104104
None => return self,
105105
};
106106

107-
self.global_state.task_pool.handle.spawn(ThreadIntent::Worker, {
107+
self.global_state.task_pool.handle.spawn(ThreadIntent::Worker, panic_context, {
108108
let world = self.global_state.snapshot();
109109
move || {
110-
let result = panic::catch_unwind(move || {
111-
let _pctx = stdx::panic_context::enter(panic_context);
112-
f(world, params)
113-
});
110+
let result = panic::catch_unwind(move || f(world, params));
114111
match thread_result_to_response::<R>(req.id.clone(), result) {
115112
Ok(response) => Task::Response(response),
116113
Err(_) => Task::Response(lsp_server::Response::new_err(
@@ -178,13 +175,10 @@ impl<'a> RequestDispatcher<'a> {
178175
None => return self,
179176
};
180177

181-
self.global_state.task_pool.handle.spawn(intent, {
178+
self.global_state.task_pool.handle.spawn(intent, panic_context, {
182179
let world = self.global_state.snapshot();
183180
move || {
184-
let result = panic::catch_unwind(move || {
185-
let _pctx = stdx::panic_context::enter(panic_context);
186-
f(world, params)
187-
});
181+
let result = panic::catch_unwind(move || f(world, params));
188182
match thread_result_to_response::<R>(req.id.clone(), result) {
189183
Ok(response) => Task::Response(response),
190184
Err(_) => Task::Retry(req),

‎crates/rust-analyzer/src/handlers/notification.rs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -291,11 +291,15 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
291291
}
292292
Ok(())
293293
};
294-
state.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, move |_| {
295-
if let Err(e) = std::panic::catch_unwind(task) {
296-
tracing::error!("flycheck task panicked: {e:?}")
297-
}
298-
});
294+
state.task_pool.handle.spawn_with_sender(
295+
stdx::thread::ThreadIntent::Worker,
296+
"flycheck",
297+
move |_| {
298+
if let Err(e) = std::panic::catch_unwind(task) {
299+
tracing::error!("flycheck task panicked: {e:?}")
300+
}
301+
},
302+
);
299303
true
300304
} else {
301305
false

‎crates/rust-analyzer/src/main_loop.rs‎

Lines changed: 76 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -397,19 +397,25 @@ impl GlobalState {
397397
tracing::debug!(%cause, "will prime caches");
398398
let num_worker_threads = self.config.prime_caches_num_threads();
399399

400-
self.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, {
401-
let analysis = self.snapshot().analysis;
402-
move |sender| {
403-
sender.send(Task::PrimeCaches(PrimeCachesProgress::Begin)).unwrap();
404-
let res = analysis.parallel_prime_caches(num_worker_threads, |progress| {
405-
let report = PrimeCachesProgress::Report(progress);
406-
sender.send(Task::PrimeCaches(report)).unwrap();
407-
});
408-
sender
409-
.send(Task::PrimeCaches(PrimeCachesProgress::End { cancelled: res.is_err() }))
410-
.unwrap();
411-
}
412-
});
400+
self.task_pool.handle.spawn_with_sender(
401+
stdx::thread::ThreadIntent::Worker,
402+
"prime_caches",
403+
{
404+
let analysis = self.snapshot().analysis;
405+
move |sender| {
406+
sender.send(Task::PrimeCaches(PrimeCachesProgress::Begin)).unwrap();
407+
let res = analysis.parallel_prime_caches(num_worker_threads, |progress| {
408+
let report = PrimeCachesProgress::Report(progress);
409+
sender.send(Task::PrimeCaches(report)).unwrap();
410+
});
411+
sender
412+
.send(Task::PrimeCaches(PrimeCachesProgress::End {
413+
cancelled: res.is_err(),
414+
}))
415+
.unwrap();
416+
}
417+
},
418+
);
413419
}
414420

415421
fn update_status_or_notify(&mut self) {
@@ -796,56 +802,62 @@ impl GlobalState {
796802

797803
// Diagnostics are triggered by the user typing
798804
// so we run them on a latency sensitive thread.
799-
self.task_pool.handle.spawn(stdx::thread::ThreadIntent::LatencySensitive, move || {
800-
let _p = profile::span("publish_diagnostics");
801-
let diagnostics = subscriptions
802-
.into_iter()
803-
.filter_map(|file_id| {
804-
let line_index = snapshot.file_line_index(file_id).ok()?;
805-
Some((
806-
file_id,
807-
line_index,
808-
snapshot
809-
.analysis
810-
.diagnostics(
811-
&snapshot.config.diagnostics(),
812-
ide::AssistResolveStrategy::None,
813-
file_id,
814-
)
815-
.ok()?,
816-
))
817-
})
818-
.map(|(file_id, line_index, it)| {
819-
(
820-
file_id,
821-
it.into_iter()
822-
.map(move |d| lsp_types::Diagnostic {
823-
range: crate::to_proto::range(&line_index, d.range),
824-
severity: Some(crate::to_proto::diagnostic_severity(d.severity)),
825-
code: Some(lsp_types::NumberOrString::String(
826-
d.code.as_str().to_string(),
827-
)),
828-
code_description: Some(lsp_types::CodeDescription {
829-
href: lsp_types::Url::parse(&format!(
830-
"https://rust-analyzer.github.io/manual.html#{}",
831-
d.code.as_str()
832-
))
833-
.unwrap(),
834-
}),
835-
source: Some("rust-analyzer".to_string()),
836-
message: d.message,
837-
related_information: None,
838-
tags: if d.unused {
839-
Some(vec![lsp_types::DiagnosticTag::UNNECESSARY])
840-
} else {
841-
None
842-
},
843-
data: None,
844-
})
845-
.collect::<Vec<_>>(),
846-
)
847-
});
848-
Task::Diagnostics(diagnostics.collect())
849-
});
805+
self.task_pool.handle.spawn(
806+
stdx::thread::ThreadIntent::LatencySensitive,
807+
"publish_diagnostics",
808+
move || {
809+
let _p = profile::span("publish_diagnostics");
810+
let diagnostics = subscriptions
811+
.into_iter()
812+
.filter_map(|file_id| {
813+
let line_index = snapshot.file_line_index(file_id).ok()?;
814+
Some((
815+
file_id,
816+
line_index,
817+
snapshot
818+
.analysis
819+
.diagnostics(
820+
&snapshot.config.diagnostics(),
821+
ide::AssistResolveStrategy::None,
822+
file_id,
823+
)
824+
.ok()?,
825+
))
826+
})
827+
.map(|(file_id, line_index, it)| {
828+
(
829+
file_id,
830+
it.into_iter()
831+
.map(move |d| lsp_types::Diagnostic {
832+
range: crate::to_proto::range(&line_index, d.range),
833+
severity: Some(crate::to_proto::diagnostic_severity(
834+
d.severity,
835+
)),
836+
code: Some(lsp_types::NumberOrString::String(
837+
d.code.as_str().to_string(),
838+
)),
839+
code_description: Some(lsp_types::CodeDescription {
840+
href: lsp_types::Url::parse(&format!(
841+
"https://rust-analyzer.github.io/manual.html#{}",
842+
d.code.as_str()
843+
))
844+
.unwrap(),
845+
}),
846+
source: Some("rust-analyzer".to_string()),
847+
message: d.message,
848+
related_information: None,
849+
tags: if d.unused {
850+
Some(vec![lsp_types::DiagnosticTag::UNNECESSARY])
851+
} else {
852+
None
853+
},
854+
data: None,
855+
})
856+
.collect::<Vec<_>>(),
857+
)
858+
});
859+
Task::Diagnostics(diagnostics.collect())
860+
},
861+
);
850862
}
851863
}

‎crates/rust-analyzer/src/reload.rs‎

Lines changed: 63 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ impl GlobalState {
185185
pub(crate) fn fetch_workspaces(&mut self, cause: Cause) {
186186
tracing::info!(%cause, "will fetch workspaces");
187187

188-
self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, {
188+
self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, "fetch_workspaces", {
189189
let linked_projects = self.config.linked_projects();
190190
let detached_files = self.config.detached_files().to_vec();
191191
let cargo_config = self.config.cargo();
@@ -260,70 +260,80 @@ impl GlobalState {
260260
tracing::info!(%cause, "will fetch build data");
261261
let workspaces = Arc::clone(&self.workspaces);
262262
let config = self.config.cargo();
263-
self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, move |sender| {
264-
sender.send(Task::FetchBuildData(BuildDataProgress::Begin)).unwrap();
263+
self.task_pool.handle.spawn_with_sender(
264+
ThreadIntent::Worker,
265+
"fetch_build_data",
266+
move |sender| {
267+
sender.send(Task::FetchBuildData(BuildDataProgress::Begin)).unwrap();
265268

266-
let progress = {
267-
let sender = sender.clone();
268-
move |msg| {
269-
sender.send(Task::FetchBuildData(BuildDataProgress::Report(msg))).unwrap()
270-
}
271-
};
272-
let res = ProjectWorkspace::run_all_build_scripts(&workspaces, &config, &progress);
269+
let progress = {
270+
let sender = sender.clone();
271+
move |msg| {
272+
sender.send(Task::FetchBuildData(BuildDataProgress::Report(msg))).unwrap()
273+
}
274+
};
275+
let res = ProjectWorkspace::run_all_build_scripts(&workspaces, &config, &progress);
273276

274-
sender.send(Task::FetchBuildData(BuildDataProgress::End((workspaces, res)))).unwrap();
275-
});
277+
sender
278+
.send(Task::FetchBuildData(BuildDataProgress::End((workspaces, res))))
279+
.unwrap();
280+
},
281+
);
276282
}
277283

278284
pub(crate) fn fetch_proc_macros(&mut self, cause: Cause, paths: Vec<ProcMacroPaths>) {
279285
tracing::info!(%cause, "will load proc macros");
280286
let dummy_replacements = self.config.dummy_replacements().clone();
281287
let proc_macro_clients = self.proc_macro_clients.clone();
282288

283-
self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, move |sender| {
284-
sender.send(Task::LoadProcMacros(ProcMacroProgress::Begin)).unwrap();
289+
self.task_pool.handle.spawn_with_sender(
290+
ThreadIntent::Worker,
291+
"fetch_proc_macros",
292+
move |sender| {
293+
sender.send(Task::LoadProcMacros(ProcMacroProgress::Begin)).unwrap();
294+
295+
let dummy_replacements = &dummy_replacements;
296+
let progress = {
297+
let sender = sender.clone();
298+
&move |msg| {
299+
sender.send(Task::LoadProcMacros(ProcMacroProgress::Report(msg))).unwrap()
300+
}
301+
};
285302

286-
let dummy_replacements = &dummy_replacements;
287-
let progress = {
288-
let sender = sender.clone();
289-
&move |msg| {
290-
sender.send(Task::LoadProcMacros(ProcMacroProgress::Report(msg))).unwrap()
303+
let mut res = FxHashMap::default();
304+
let chain = proc_macro_clients
305+
.iter()
306+
.map(|res| res.as_ref().map_err(|e| e.to_string()))
307+
.chain(iter::repeat_with(|| Err("Proc macros servers are not running".into())));
308+
for (client, paths) in chain.zip(paths) {
309+
res.extend(paths.into_iter().map(move |(crate_id, res)| {
310+
(
311+
crate_id,
312+
res.map_or_else(
313+
|_| Err("proc macro crate is missing dylib".to_owned()),
314+
|(crate_name, path)| {
315+
progress(path.display().to_string());
316+
client.as_ref().map_err(Clone::clone).and_then(|client| {
317+
load_proc_macro(
318+
client,
319+
&path,
320+
crate_name
321+
.as_deref()
322+
.and_then(|crate_name| {
323+
dummy_replacements.get(crate_name).map(|v| &**v)
324+
})
325+
.unwrap_or_default(),
326+
)
327+
})
328+
},
329+
),
330+
)
331+
}));
291332
}
292-
};
293-
294-
let mut res = FxHashMap::default();
295-
let chain = proc_macro_clients
296-
.iter()
297-
.map(|res| res.as_ref().map_err(|e| e.to_string()))
298-
.chain(iter::repeat_with(|| Err("Proc macros servers are not running".into())));
299-
for (client, paths) in chain.zip(paths) {
300-
res.extend(paths.into_iter().map(move |(crate_id, res)| {
301-
(
302-
crate_id,
303-
res.map_or_else(
304-
|_| Err("proc macro crate is missing dylib".to_owned()),
305-
|(crate_name, path)| {
306-
progress(path.display().to_string());
307-
client.as_ref().map_err(Clone::clone).and_then(|client| {
308-
load_proc_macro(
309-
client,
310-
&path,
311-
crate_name
312-
.as_deref()
313-
.and_then(|crate_name| {
314-
dummy_replacements.get(crate_name).map(|v| &**v)
315-
})
316-
.unwrap_or_default(),
317-
)
318-
})
319-
},
320-
),
321-
)
322-
}));
323-
}
324333

325-
sender.send(Task::LoadProcMacros(ProcMacroProgress::End(res))).unwrap();
326-
});
334+
sender.send(Task::LoadProcMacros(ProcMacroProgress::End(res))).unwrap();
335+
},
336+
);
327337
}
328338

329339
pub(crate) fn set_proc_macros(&mut self, proc_macros: ProcMacros) {

‎crates/rust-analyzer/src/task_pool.rs‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,41 @@ impl<T> TaskPool<T> {
1414
TaskPool { sender, pool: Pool::new(threads) }
1515
}
1616

17-
pub(crate) fn spawn<F>(&mut self, intent: ThreadIntent, task: F)
18-
where
17+
pub(crate) fn spawn<F>(
18+
&mut self,
19+
intent: ThreadIntent,
20+
panic_context: impl Into<String>,
21+
task: F,
22+
) where
1923
F: FnOnce() -> T + Send + 'static,
2024
T: Send + 'static,
2125
{
26+
let panic_context = panic_context.into();
2227
self.pool.spawn(intent, {
2328
let sender = self.sender.clone();
24-
move || sender.send(task()).unwrap()
29+
move || {
30+
let _pctx = stdx::panic_context::enter(panic_context);
31+
sender.send(task()).unwrap()
32+
}
2533
})
2634
}
2735

28-
pub(crate) fn spawn_with_sender<F>(&mut self, intent: ThreadIntent, task: F)
29-
where
36+
pub(crate) fn spawn_with_sender<F>(
37+
&mut self,
38+
intent: ThreadIntent,
39+
panic_context: impl Into<String>,
40+
task: F,
41+
) where
3042
F: FnOnce(Sender<T>) + Send + 'static,
3143
T: Send + 'static,
3244
{
45+
let panic_context = panic_context.into();
3346
self.pool.spawn(intent, {
3447
let sender = self.sender.clone();
35-
move || task(sender)
48+
move || {
49+
let _pctx = stdx::panic_context::enter(panic_context);
50+
task(sender)
51+
}
3652
})
3753
}
3854

0 commit comments

Comments
 (0)