Repository: rust
Issue: rustc should suggest using async version of Mutex
Labels: C-enhancement,A-diagnostics,T-compiler,A-async-await,A-suggestion-diagnostics,AsyncAwait-Triaged

If one accidentally uses `std::sync::Mutex` in asynchronous code and holds a `MutexGuard` across an await, then the future is marked `!Send`, and you can't spawn it off to run on another thread - all correct. Rustc even gives you an excellent error message, pointing out the `MutexGuard` as the reason the future is not `Send`.

But people new to asynchronous programming are not going to immediately realize that there is such a thing as an 'async-friendly mutex'. You need to be aware that ordinary mutexes insist on being unlocked on the same thread that locked them; and that executors move tasks from one thread to another; and that the solution is not to make ordinary mutexes more complex but to create a new mutex type altogether. These make sense in hindsight, but I'll bet that they leap to mind only for a small group of elite users. (But probably a majority of the people who will ever read this bug. Ahem.)

So I think rustc should provide extra help when the value held across an `await`, and thus causing a future not to be `Send`, is a `MutexGuard`, pointing out that one must use an asynchronous version of `Mutex` if one needs to hold guards across an await. It's awkward to suggest `tokio::sync::Mutex` or `async_std::sync::Mutex`, but surely there's some diplomatic way to phrase it that is still explicit enough to be helpful.

Perhaps this could be generalized to other types. For example, if the offending value is an `Rc`, the help should suggest `Arc`.

Here's an illustration of what I mean:

```
use std::future::Future;
use std::sync::Mutex;

fn fake_spawn<F: Future + Send + 'static>(f: F) { }

async fn wrong_mutex() {
    let m = Mutex::new(1);
    let mut guard = m.lock().unwrap();
    (async { }).await;
    *guard += 1;
}

fn main() {
    fake_spawn(wrong_mutex());
    //~^ERROR: future cannot be sent between threads safely
}
```

The error message is great:
```
error: future cannot be sent between threads safely
  --> src/main.rs:14:5
   |
4  | fn fake_spawn<F: Future + Send + 'static>(f: F) { }
   |    ----------             ---- required by this bound in `fake_spawn`
...
14 |     fake_spawn(wrong_mutex());
   |     ^^^^^^^^^^ future returned by `wrong_mutex` is not `Send`
   |
   = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::sync::MutexGuard<'_, i32>`
note: future is not `Send` as this value is used across an await
  --> src/main.rs:9:5
   |
8  |     let mut guard = m.lock().unwrap();
   |         --------- has type `std::sync::MutexGuard<'_, i32>`
9  |     (async { }).await;
   |     ^^^^^^^^^^^^^^^^^ await occurs here, with `mut guard` maybe used later
10 |     *guard += 1;
11 | }
   | - `mut guard` is later dropped here
```
I just wish it included:
```
help: If you need to hold a mutex guard while you're awaiting, you must use an async-aware version of the `Mutex` type.
help: Many asynchronous foundation crates provide such a `Mutex` type.
```

<!-- TRIAGEBOT_START -->

<!-- TRIAGEBOT_ASSIGN_START -->
This issue has been assigned to @LucioFranco via [this comment](https://github.com/rust-lang/rust/issues/71072#issuecomment-658306268).
<!-- TRIAGEBOT_ASSIGN_DATA_START$${"user":"LucioFranco"}$$TRIAGEBOT_ASSIGN_DATA_END -->

<!-- TRIAGEBOT_ASSIGN_END -->
<!-- TRIAGEBOT_END -->