Closed
Description
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
andthread::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 ourJoinHandler
(what's a good name here?ScopedThread
,ThreadGuard
?) to theWorker
- 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 beforeScopedThread
, even in case of a panic.