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.
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
panicintoResultby 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:
thread::spawnandthread::JoinHandlerwhich 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 hereWorkerHandlestruct (which exists solely to join the thread) and instead add ourJoinHandler(what's a good name here?ScopedThread,ThreadGuard?) to theWorkerWorker's shutdown method to return a pair of(ScopedThread, Receiver)thead_workerto the new API, paying close attention to possible deadlocks. Specifically, channel senders should be dropped beforeScopedThread, even in case of a panic.