Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Commit

Permalink
Don't abandon jobs on normal termination
Browse files Browse the repository at this point in the history
What is the correct behavior when dropping a non-empty jobs table?
There are two main cases here: normal termination, and unwinding
(`std::thread::panicking()`).

For normal termination, it is reasonable to require that there are no
pending jobs, and that the caller is responsible for explicitly
waiting for all the jobs to finish. In the RLS case, we wait for all
jobs on ShutdownRequest and don't spawn any jobs afterwards.

For panicking, the picture is less clear. The simplest choice is just
to abandon background jobs, but that means that some threads won't be
waited for. This is the current solution.

A more deterministic choice is to cancel the jobs (cancellation is yet
to be implemented) and then wait for them to finish. The problem with
this solution is that not all jobs can be cancelled immediately.
  • Loading branch information
matklad committed Jul 16, 2018
1 parent 8c2fd97 commit af2534f
Showing 1 changed file with 1 addition and 17 deletions.
18 changes: 1 addition & 17 deletions src/concurrency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,6 @@ impl Jobs {
}
}

// For the time being, just detach all currently running
// jobs on `Drop`. Are more deterministic behavior is
// to wait for all jobs to finish.
impl Drop for Jobs {
fn drop(&mut self) {
let jobs = mem::replace(&mut self.jobs, Vec::new());
for job in jobs {
job.abandon()
}
}
}

impl ConcurrentJob {
pub fn new() -> (ConcurrentJob, JobToken) {
let (tx, rx) = bounded(0);
Expand All @@ -88,15 +76,11 @@ impl ConcurrentJob {
fn is_completed(&self) -> bool {
is_closed(&self.chan)
}

fn abandon(mut self) {
self.is_abandoned = true
}
}

impl Drop for ConcurrentJob {
fn drop(&mut self) {
if self.is_abandoned || self.is_completed() || thread::panicking() {
if self.is_completed() || thread::panicking() {
return;
}
panic!("orphaned concurrent job");
Expand Down

0 comments on commit af2534f

Please sign in to comment.