Could `Mutex<T>` have been `Mutex<Poison<T>>`?

It's too late for libstd to change it, but I wonder if poisoning could have been made optional by separating it into another type.

Currently, Mutex is a bundle of two features in one, which forces users to use poisoning even where they don't need to. Conversely, users who need poisoning can't get it without using a Mutex either, which lead to the whole heuristic of UnwindSafe.

So it occurred to me it would have been much more explicit if there was a Poison<T> wrapper type that poisons itself on unwinding, and a separate Mutex<T> type that concerns itself only with locking.

Maybe one day when there's specialization, Mutex<T> could become a type alias for some PlainMutex<Poison<T>>…

4 Likes
2 Likes

Such a world wouldn't be trivially Mutex<Poison<T>> without losing some capabilities we have today, at least, because Drop behavior is needed to implement both, and Poison<T> wouldn't be able to provide that without itself having mutex-like guards, and if you do that, you end up with a situation where you need to call two .lock() like methods and borrow two things, which (since Rust has no general self-reference mechanism) narrows down the number of ways you can wrap a Mutex<Poison<T>>.

I think a better solution to this overall problem would be to have std incorporate something like lock_api, allowing users of std to use its operating system knowledge while wrapping it with specific mutex design choices like poisoning or non-poisoning (and ArcMutexGuard which is an example of eliminating self-referential borrowing problems).

8 Likes

Out of curiosity, does anyone happen to know what motivated the choice to make the default form of .lock functionality be to return a result based on poisoning rather than just panicking by default and having some alternate way of checking for poisoning in the IME very rare case where you actually want to do that? (I'll admit I haven't read a ton of library code, but I've never actually seen code which does anything other than just unwrap locks, which is the natural thing to do since poisoning indicates a data-dependency on a thread which panicked, and propagating panic over data dependencies seems like a reasonable default.)

3 Likes

There is already .is_poisoned(). But if you use that to check for poisoning, then you have a race condition, as the documentation already notes:

If another thread is active, the mutex can still become poisoned at any time. You should not trust a false value for program correctness without additional synchronization.

Concretely, this is a possible sequence of events:

  1. Thread A checks .is_poisoned() and gets false
  2. Thread B calls .lock()
  3. Thread B panics and poisons the mutex
  4. Thread A calls .lock() and panics

Therefore, “check for poisoning” by itself is not a sufficiently powerful operation; if .lock() was panicking, you'd need a separate .lock_with_checking_for_poison().

1 Like

I think they've meant mutex.lock() to behave like mutex.lock().unwrap() does currently, and having the Result-returning method to be named something else (check not via boolean, but check by getting Result)

1 Like

lock_with_check_for_poisoning is indeed the operation I meant; just have the default behavior be a synonym for what is currently .lock().unwrap() and have an alternate name for current .lock().

Perhaps an altered interface for try_lock() or something which makes it return something like a HashMap Entry that you then specify further behavior on. Or just a method called .lock_or_poison() which seems like a reasonable balance of readable and not excessively long. But the exact interface is besides the point.