Skip to content

Commit bcb89d1

Browse files
committed
Auto merge of #7366 - alexcrichton:fix-hang, r=Eh2406
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
2 parents b2d4f20 + 8a3c0fe commit bcb89d1

File tree

1 file changed

+26
-3
lines changed

1 file changed

+26
-3
lines changed

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)