Skip to content

Bad handling of thread join deadlock in standard library #34971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eefriedman opened this issue Jul 22, 2016 · 1 comment
Closed

Bad handling of thread join deadlock in standard library #34971

eefriedman opened this issue Jul 22, 2016 · 1 comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@eefriedman
Copy link
Contributor

eefriedman commented Jul 22, 2016

use std::sync::mpsc::Sender;
use std::sync::mpsc::Receiver;
use std::thread::JoinHandle;
fn main() {
    let (c1s, c1r) = std::sync::mpsc::channel::<JoinHandle<_>>();
    let (c2s, c2r) = std::sync::mpsc::channel::<JoinHandle<_>>();
    let (c3s, c3r) = std::sync::mpsc::channel::<()>();
    let t1 = std::thread::spawn(move || {
        let () = c1r.recv().expect("D").join().expect("A");
    });
    let t2 = std::thread::spawn(move || {
        let () = c2r.recv().expect("E").join().expect("B");
        c3s.send(());
    });
    c1s.send(t2);
    c2s.send(t1);
    c3r.recv().expect("C");
}

Gives (on playground):

thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', ../src/libcore/option.rs:325
note: Run with `RUST_BACKTRACE=1` for a backtrace.
thread '<unnamed>' panicked at 'B: Any', ../src/libcore/result.rs:785
thread '<main>' panicked at 'C: RecvError', ../src/libcore/result.rs:785

There are two problems here:

  1. The error message is absolutely terrible.
  2. For the sake of safety, we might need to do something other than panic if this happens. This could affect crates which require that join() never returns early, like crossbeam.
@steveklabnik steveklabnik added A-libs A-diagnostics Area: Messages for errors, warnings, and lints labels Jul 25, 2016
@steveklabnik steveklabnik removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum removed the A-diagnostics Area: Messages for errors, warnings, and lints label Jun 16, 2017
@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Jul 26, 2017
alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 27, 2017
Also add to the documentation that the `join` method can panic.

cc rust-lang#34971
cc rust-lang#43539
bors added a commit that referenced this issue Aug 27, 2017
std: Handle OS errors when joining threads

Also add to the documentation that the `join` method can panic.

cc #34971
cc #43539
@Mark-Simulacrum
Copy link
Member

Possibly in #44112 or otherwise, but this seems to have since been fixed -- this now deadlocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants