We're currently working on separating poison from mutexes, such that the default mutexes won't have poisoning (no more `.lock().unwrap()`), and if you want poisoning you can use something like `Mutex<Poison<T>>`.
While we're at it, another thing that'd be nice to get rid of is `AssertUnwindSafe`, which I find even more pointless.
I do not have a good grasp on panics in Rust, but panics in Rust being able to either unwind or abort dependent on configuration, seems complex, and that design happened for historical reasons, from what I have read elsewhere.
The hard use case to handle without unwinding is an HTTP server that wants to allow for panics in a request handler without panicking the entire process. Unwinding is a janky way to handle that, and creates issues in code that doesn't expect unwinding (e.g. half-modified states), and poisoning in particular seems likely to cascade and bring down other parts of the process anyway if some needed resource gets poisoned. But we need a reasonable alternative to propose for that use case, in order to seriously evaluate eliminating unwinding.
If a request handler for example has at some point in time 7 nested calls, in call no. 2 and call no. 6 have resources and partial computation that needs clean-up somehow and somewhere, and call no. 7 panics, I wonder what the code would look like in the different calls, and what would happen and when, and what the compiler would require, and what other relevant code would look like.
Today, people sometimes do that by using `panic = "unwind"`, and writing a `catch_unwind` around their program, and using that to essentially implement a "finally" block. Or, they do it by having an RAII type that cleans up on `Drop`, and then they count on unwinding to ensure their `Drop` gets called even on panic. (That leaves aside the issue that something called from `Drop` is not allowed to fail or panic itself.) The question is, how would you do that without unwinding?
We have a panic hook mechanism, where on panic the standard library will call a user-supplied function. However, there is only one panic hook; if you set it, it replaces the old hook. If you have only one cleanup to do, that works fine. For more than one, you can follow the semantic of having your panic hook call the previous hook, but that does not allow unregistering hooks out of order; it only really works if you register a panic hook once for the whole program and never unregister it (e.g. "here's the hook for cleaning up tracing", "here's the hook for cleaning up the terminal state").
Suppose, instead, we had a mechanism that allowed registering arbitrary panic hooks, and unregistering them when no longer needed, in any order. Then, we could do RAII-style resource handling: you could have a `CursesTerminal` type, which is responsible for cleaning up the terminal, and it cleans up the terminal on `Drop` and on panic. To do the latter, it would register a panic hook, and deregister that hook on `Drop`.
With such a mechanism, panic hooks could replace anything that uses `catch_unwind` to do cleanup before going on to exit the program. That wouldn't fully solve the problem of doing cleanup and then swallowing the panic and continuing, but it'd be a useful component for that.
This doesn't get rid of unwinding at all- it's an inefficient reimplementation of it. There's a reason language implementations have switched away from having the main execution path register and unregister destructors and finally blocks, to storing them in a side table and recovering them at the time of the throw.
And I would expect to turn it into an efficient solution, in part by doing the "store in a side table" approach for hook registration.
* You have to know when an API can unwind, and you have to make it an error to unwind when the caller isn't expecting it. If this is done statically, you are getting into effect annotation territory. If this is done dynamically, are essentially just injecting drop bombs into code that doesn't expect unwinding. Either way, you are multiplying complexity for generic code. (Not to mention you have to invent a whole new set of idioms for panic-free code.)
* You still have to be able to clean up the resources held by a caller that does expect unwinding. So all your vocabulary/glue/library code (the stuff that can't just assume panic=abort) still needs these "scoped panic hooks" in all the same places it has any level of panic awareness in Drop today.
So for anyone to actually benefit from this, they have to be writing panic-free code with whatever new static or dynamic tools come with this, and they have to be narrowly scoped and purpose-specific enough that they could essentially already today afford panic=abort. Who is this even for?
In my case it's usually waiting for the GPU to finish some asynchronous work that's been spun up by CPU threads that may have panicked while holding the lock. This is necessary to avoid freeing resources that the GPU may still be using.
I usually prefix this with `if !std::thread::panicking() {}`, so I don't end up waiting (possibly forever) if I'm already cleaning up after a panic.
I have a container of resources, e.g. textures. When the GPU wants to use them, CPU will lease them until a point of time in the future denoted by a value (u64) of a GPU timeline semaphore. The handle and value of the semaphore is added to a list guarded by a mutex. Then GPU work is kicked off and the GPU will increment semaphore to that value when done.
In the Drop implementation of the container, we need to wait until all semaphores reach their respective value before freeing resources, and do so even if some thread panicked while holding the lock guarding the list. This is where I use .unwrap_or_else to get the list from the poison value.
It's not infeasible to try to catch any errors and propagate them when the lock is grabbed. But this is mostly for OOM and asserts that are not expected to fire. The ergonomics would be worse if the "lease" function would be fallible.
This said, I would not object to poisoning being made optional.
Panic propagation is typically handled much better at thread `join()` boundaries.
Heck, you can have Drop impls observing the state while unwinding.
A true panic-safe data structure requires serious thought, and mutex poisoning does nothing here - it is neither necessary nor sufficient.
This seems analogous to arguing that because seat belts don't save the lives of all people involved in car crashes, and they're kind of annoying, then they shouldn't be factory-standard.
> Heck, you can have Drop impls observing the state while unwinding.
Yeah, this is really painful and regularly forgotten. And one reason it'd be nice to not have unwinding.
There's something to be said here about what I've sometimes called the cancellation blast radius. The issues with cancellation happen when the data corruption/invariant violation is externally visible (if the corrupt data is torn down, who cares.) Mutexes make data corruption externally visible very often.
Lots of data structures need to care about panic-safety. Inserting a node in a tree must leave the tree in a valid state if allocating memory for the new node fails, for example. All of that is completely orthogonal to whether or not the data structure is also observable from multiple threads behind a mutex, and I would argue especially in the case of mutex, whose purpose it is to make an object usable from multiple threads as-if they had ownership.
Edit: isn’t that an argument not to change the default? If people were recovering from poison a lot and that was painful, that’s one thing. But if people aren’t doing that, why is this a problem?
I'm suggesting that the balance of pain to benefit is not working out enough to inflict it on everyone by default. I'm not suggesting it has no value, just not enough to be worth it.
I take it as uncontroversial that an important function of Mutexes is to ensure that invariants about data are maintained when the data is modified and that very bad things can happen when a program's data invariants are violated at runtime and the program doesn't notice. Maybe folks disagree about whether a program should always panic when invariants are violated at runtime (though there's certainly plenty of precedent in Rust itself for doing this, like with array bounds checking). Probably the bigger question mark is that panicking with a Mutex held doesn't necessarily mean an invariant is violated. But it does mean that the mechanism for ensuring the invariant has itself failed. I can see different choices about what to do here. For myself, the event itself is so rare and the impact of getting an invariant wrong so high that I absolutely do want to panic -- the false positive rate is just too small to matter.
Correctness trumps ergonomics, and the default should definitely be poisoning/panicking unless handled. There could definitely be an optional poison-eating mutex, but I argue the current Mutex does the right thing.