lock_api: Add a with(&self, closure) method to Mutex?

[phil-opp]

What do you think about adding a Mutex::with function that takes a closure, as an alternative way to access the locked data? The usage would look like this:

let mutex = Mutex::new(String::new("HELLO"));
mutex.with(|s: &mut String| s.make_ascii_lowercase());

This would be roughly equivalent to mutex.lock().make_ascii_lowercase(), but would it make visually more clear that the lock is only held for a single operation.

I'm happy to create a simple pull request for this if you like.

[Amanieu]

This has been suggested before, but I just don't see much value over simply creating a new scope:

{
    let s = mutex.lock();
    s.make_ascii_lowercase()
}

My main worry here is that we may end up with too many ways of doing the same thing, which can be confusing to new users.

[phil-opp]

My main problem with the RAII based API is that it is not obvious where the lock is freed from a first sight. As a result, the lock is often held for longer periods than needed, for example when a previously short function becomes longer over time.

Another drawback of the approach is that we always operate on MutexGuard types instead of on &mut references directly. This leads to various papercuts:

• We need to make the MutexGuard binding mutable before we can DerefMut it (it needs to be let mut s in your example)
• We can't directly compare a MutexGuard<T> to a T. Instead we need to add a strange looking dereferencing operator: assert_eq!(*locked_data, expected_data).
• We need to use methods like MutexGuard::map instead of just doing &mut data.field

Since both APIs have their advantages and drawbacks, I think it makes sense to support both approaches.

Regarding the potential for confusion: I think this is something that can be easily averted by documentation. We just need to document that both approaches are equivalent and that it's only a matter of personal preference.

[faern]

A problem with personal preference is that it creates inconsistencies. I'm with Amanieu in that it's better to have a single way of doing stuff that everyone can get used to. Then the paper cuts will be well known and people will easily read the code anyway. If there are many ways people will read code slower, because different code that does the same thing will look different, instead of always looking the same.

[phil-opp]

I think it's more a convenience method than an inconsistency. It behaves as expected and has no influence at all on the RAII based API. So it's kind of similar to the Iterator::for_each method (which is equivalent to a for loop): It provides a different way of achieving the same thing because it has advantages in some situations.

[phil-opp]

I tried to find prior art for such a method and found rust-lang/rust#61976, which proposed adding a similar method to the Mutex of the standard library. It was closed because it was unclear how to handle poisoned locks. Given that there is no poisoning in lock_api, I still think it's worth discussing such an addition.