rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.3k stars 1.52k forks source link

False positive using async-condvar-fair / MutexGuard moved into future that is then awaited #13075

Open bjackman opened 2 months ago

bjackman commented 2 months ago

Summary

When using this library the warning this MutexGuard is held across an await point fires, but I don't believe any mutex is held across await

Lint Name

await_holding_lock

Reproducer

I tried this code:

use async_condvar_fair::Condvar;
use parking_lot::Mutex;

//... 

pub struct Pools<T> {
    cond: Condvar,
    resources: Mutex<(Vec<usize>, Vec<T>)>,
}

// ...
impl<T: Send> Pools<T> {
    // ...
    pub async fn get<I: IntoIterator<Item = usize>>(&self, token_counts: I) -> Resources<T> {
        let mut guard = self.resources.lock();
        loop {
            // ...
            if some_embarrassing_garbage() {
                return more_embarrassing_garbage();
            }
            guard = self.cond.wait(guard).await;
        }
    }

I saw this happen:

❯❯  cargo clippy
warning: this `MutexGuard` is held across an `await` point
  --> src/resource.rs:38:13
   |
38 |         let mut guard = self.resources.lock();
   |             ^^^^^^^^^
   |
   = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await
note: these are all the `await` points this lock is held through
  --> src/resource.rs:59:60
   |
59 |             guard = self.cond.wait::<MutexGuard<_>>(guard).await;
   |                                                            ^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
   = note: `#[warn(clippy::await_holding_lock)]` on by default

warning: `local-ci` (bin "local-ci") generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s

I do not expect any warning here because I believe the guard is moved into the future before it is awaited. (The code implementing the future will release the lock, I believe that even if it didn't, that would be a bug in the library and not something for Clippy to warn about here).

Version

rustc 1.79.0 (129f3b996 2024-06-10)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: x86_64-unknown-linux-gnu
release: 1.79.0
LLVM version: 18.1.7

Additional Labels

No response

bjackman commented 2 months ago

Perhaps related: https://github.com/rust-lang/rust-clippy/issues/9683