Skip to content

Commit 48f8799

Browse files
borsbors
committed
Auto merge of #14967 - rust-lang:revert-14965-panic-ctx, r=Veykril
Revert "Add mandatory panic contexts to all threadpool tasks" Reverts rust-lang/rust-analyzer#14965 This won't quite work actually given the use of `catch_unwind` in some of these
2 parents de2320e + a1af9eb commit 48f8799

File tree

5 files changed

+138
-174
lines changed

5 files changed

+138
-174
lines changed

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

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

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

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

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -291,15 +291,11 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
291291
}
292292
Ok(())
293293
};
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-
);
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+
});
303299
true
304300
} else {
305301
false

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

Lines changed: 64 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -397,25 +397,19 @@ 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(
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-
);
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+
});
419413
}
420414

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

803797
// Diagnostics are triggered by the user typing
804798
// so we run them on a latency sensitive thread.
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-
);
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+
});
862850
}
863851
}

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

Lines changed: 53 additions & 63 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, "fetch_workspaces", {
188+
self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, {
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,80 +260,70 @@ 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(
264-
ThreadIntent::Worker,
265-
"fetch_build_data",
266-
move |sender| {
267-
sender.send(Task::FetchBuildData(BuildDataProgress::Begin)).unwrap();
263+
self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, move |sender| {
264+
sender.send(Task::FetchBuildData(BuildDataProgress::Begin)).unwrap();
268265

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

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

284278
pub(crate) fn fetch_proc_macros(&mut self, cause: Cause, paths: Vec<ProcMacroPaths>) {
285279
tracing::info!(%cause, "will load proc macros");
286280
let dummy_replacements = self.config.dummy_replacements().clone();
287281
let proc_macro_clients = self.proc_macro_clients.clone();
288282

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-
};
283+
self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, move |sender| {
284+
sender.send(Task::LoadProcMacros(ProcMacroProgress::Begin)).unwrap();
302285

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-
}));
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()
332291
}
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+
}
333324

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

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

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

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

17-
pub(crate) fn spawn<F>(
18-
&mut self,
19-
intent: ThreadIntent,
20-
panic_context: impl Into<String>,
21-
task: F,
22-
) where
17+
pub(crate) fn spawn<F>(&mut self, intent: ThreadIntent, task: F)
18+
where
2319
F: FnOnce() -> T + Send + 'static,
2420
T: Send + 'static,
2521
{
26-
let panic_context = panic_context.into();
2722
self.pool.spawn(intent, {
2823
let sender = self.sender.clone();
29-
move || {
30-
let _pctx = stdx::panic_context::enter(panic_context);
31-
sender.send(task()).unwrap()
32-
}
24+
move || sender.send(task()).unwrap()
3325
})
3426
}
3527

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
28+
pub(crate) fn spawn_with_sender<F>(&mut self, intent: ThreadIntent, task: F)
29+
where
4230
F: FnOnce(Sender<T>) + Send + 'static,
4331
T: Send + 'static,
4432
{
45-
let panic_context = panic_context.into();
4633
self.pool.spawn(intent, {
4734
let sender = self.sender.clone();
48-
move || {
49-
let _pctx = stdx::panic_context::enter(panic_context);
50-
task(sender)
51-
}
35+
move || task(sender)
5236
})
5337
}
5438

0 commit comments

Comments
 (0)