Change thread_worker library to propagate panics by default

[matklad]

We have a small thread_worker for spawning background threads. It's main purpose is to force clean thread shutdown: if you don't join the thread, it might be forcefully terminated when the process exists, skipping destructors. Leaving threads unjoined is also bad for reliable unit-testing, and in general bad for overall architecture of the system.

A slight problem with this library is that it turns a thread's panic into Result by forcing you to use the shutdown method. I now think this is a bad idea: it's usually best to escalate the panic instead of catching it, even on the thread boundary. We need to switch "panic, if child thread panicked" behavior.

To do this, we need the following changes:

• introduce a wrapper around thread::spawn and thread::JoinHandler which joins the thread automatically in Drop, propagating the panic from the child thread if the parent thread is not panicking itself. We should use this wrapper in cases where we spawn bare threads, like here
• Remove WorkerHandle struct (which exists solely to join the thread) and instead add our JoinHandler (what's a good name here? ScopedThread, ThreadGuard?) to the Worker
• Change Worker's shutdown method to return a pair of (ScopedThread, Receiver)
• update usages of thead_worker to the new API, paying close attention to possible deadlocks. Specifically, channel senders should be dropped before ScopedThread, even in case of a panic.

[matklad]

cc @stjepang, this is very related to rust-lang/rust#48830 and shows an interesting interaction between channels and threads (drop sender prior to joining).

[matklad]

Slightly tangential, but, if you are interested in threads and clean cancellation, there's an interesting discussion over here: https://trio.discourse.group/t/structured-concurrency-in-rust/73

[matklad]

drop sender prior to joining

Hm, this seems fun indeed, looks like the pattern we want is

let (sender, receiver) = channel();
let thread = spawn(move || {
    for event in receiver {
        ...
    }
})

// subtle, force sender to be dropped *before* thread, 
// avoiding deadlock if *this* thread panics.
let sender = sender;


// Alternative way to spell this

let thread;
let (sender, receiver) = channel();
thread = spawn(move || {
    for event in receiver {
        ...
    }
})

That's a super uncomfortable level of subtlety, can we do better?

[DJMcNab]

We could just use std::mem::drop manually.

[matklad]

That works for the happy case, but not for the panics, a panic will cause a deadlock