Skip to content

Commit 8a3c0fe

Browse files
committed
Don't hang when Cargo's worker threads panic
This shouldn't ever happen during normal development, but happens from time to time while developing Cargo itself. Closes #6433
1 parent be01054 commit 8a3c0fe

File tree

1 file changed

+26
-3
lines changed

1 file changed

+26
-3
lines changed

Diff for: src/cargo/core/compiler/job_queue.rs

+26-3
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ use std::cell::Cell;
22
use std::collections::{HashMap, HashSet};
33
use std::io;
44
use std::marker;
5+
use std::mem;
56
use std::sync::mpsc::{channel, Receiver, Sender};
67
use std::sync::Arc;
78

89
use crossbeam_utils::thread::Scope;
10+
use failure::format_err;
911
use jobserver::{Acquired, HelperThread};
1012
use log::{debug, info, trace};
1113

@@ -490,7 +492,13 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
490492
rmeta_required: Cell::new(rmeta_required),
491493
_marker: marker::PhantomData,
492494
};
493-
let res = job.run(&state);
495+
496+
let mut sender = FinishOnDrop {
497+
tx: &my_tx,
498+
id,
499+
result: Err(format_err!("worker panicked")),
500+
};
501+
sender.result = job.run(&state);
494502

495503
// If the `rmeta_required` wasn't consumed but it was set
496504
// previously, then we either have:
@@ -504,13 +512,28 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
504512
// we'll just naturally abort the compilation operation but for 1
505513
// we need to make sure that the metadata is flagged as produced so
506514
// send a synthetic message here.
507-
if state.rmeta_required.get() && res.is_ok() {
515+
if state.rmeta_required.get() && sender.result.is_ok() {
508516
my_tx
509517
.send(Message::Finish(id, Artifact::Metadata, Ok(())))
510518
.unwrap();
511519
}
512520

513-
my_tx.send(Message::Finish(id, Artifact::All, res)).unwrap();
521+
// Use a helper struct with a `Drop` implementation to guarantee
522+
// that a `Finish` message is sent even if our job panics. We
523+
// shouldn't panic unless there's a bug in Cargo, so we just need
524+
// to make sure nothing hangs by accident.
525+
struct FinishOnDrop<'a> {
526+
tx: &'a Sender<Message>,
527+
id: u32,
528+
result: CargoResult<()>,
529+
}
530+
531+
impl Drop for FinishOnDrop<'_> {
532+
fn drop(&mut self) {
533+
let msg = mem::replace(&mut self.result, Ok(()));
534+
drop(self.tx.send(Message::Finish(self.id, Artifact::All, msg)));
535+
}
536+
}
514537
};
515538

516539
if !cx.bcx.build_config.build_plan {

0 commit comments

Comments
 (0)