Skip to content

Commit

Permalink
Prevent panics from tearing down worker threads
Browse files Browse the repository at this point in the history
  • Loading branch information
Veykril committed Feb 4, 2025
1 parent 93b72ce commit 1ce2d7e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
6 changes: 4 additions & 2 deletions src/tools/rust-analyzer/crates/rust-analyzer/src/task_pool.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! A thin wrapper around [`stdx::thread::Pool`] which threads a sender through spawned jobs.
//! It is used in [`crate::global_state::GlobalState`] throughout the main loop.
use std::panic::UnwindSafe;

use crossbeam_channel::Sender;
use stdx::thread::{Pool, ThreadIntent};

Expand All @@ -18,7 +20,7 @@ impl<T> TaskPool<T> {

pub(crate) fn spawn<F>(&mut self, intent: ThreadIntent, task: F)
where
F: FnOnce() -> T + Send + 'static,
F: FnOnce() -> T + Send + UnwindSafe + 'static,
T: Send + 'static,
{
self.pool.spawn(intent, {
Expand All @@ -29,7 +31,7 @@ impl<T> TaskPool<T> {

pub(crate) fn spawn_with_sender<F>(&mut self, intent: ThreadIntent, task: F)
where
F: FnOnce(Sender<T>) + Send + 'static,
F: FnOnce(Sender<T>) + Send + UnwindSafe + 'static,
T: Send + 'static,
{
self.pool.spawn(intent, {
Expand Down
21 changes: 13 additions & 8 deletions src/tools/rust-analyzer/crates/stdx/src/thread/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
//! The thread pool is implemented entirely using
//! the threading utilities in [`crate::thread`].
use std::sync::{
atomic::{AtomicUsize, Ordering},
Arc,
use std::{
panic::{self, UnwindSafe},
sync::{
atomic::{AtomicUsize, Ordering},
Arc,
},
};

use crossbeam_channel::{Receiver, Sender};
Expand All @@ -25,13 +28,13 @@ pub struct Pool {
// so that the channel is actually closed
// before we join the worker threads!
job_sender: Sender<Job>,
_handles: Vec<JoinHandle>,
_handles: Box<[JoinHandle]>,
extant_tasks: Arc<AtomicUsize>,
}

struct Job {
requested_intent: ThreadIntent,
f: Box<dyn FnOnce() + Send + 'static>,
f: Box<dyn FnOnce() + Send + UnwindSafe + 'static>,
}

impl Pool {
Expand All @@ -47,6 +50,7 @@ impl Pool {
let handle = Builder::new(INITIAL_INTENT)
.stack_size(STACK_SIZE)
.name("Worker".into())
.allow_leak(true)
.spawn({
let extant_tasks = Arc::clone(&extant_tasks);
let job_receiver: Receiver<Job> = job_receiver.clone();
Expand All @@ -58,7 +62,8 @@ impl Pool {
current_intent = job.requested_intent;
}
extant_tasks.fetch_add(1, Ordering::SeqCst);
(job.f)();
// discard the panic, we should've logged the backtrace already
_ = panic::catch_unwind(job.f);
extant_tasks.fetch_sub(1, Ordering::SeqCst);
}
}
Expand All @@ -68,12 +73,12 @@ impl Pool {
handles.push(handle);
}

Pool { _handles: handles, extant_tasks, job_sender }
Pool { _handles: handles.into_boxed_slice(), extant_tasks, job_sender }
}

pub fn spawn<F>(&self, intent: ThreadIntent, f: F)
where
F: FnOnce() + Send + 'static,
F: FnOnce() + Send + UnwindSafe + 'static,
{
let f = Box::new(move || {
if cfg!(debug_assertions) {
Expand Down

0 comments on commit 1ce2d7e

Please sign in to comment.