Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bjackman opened this issue Jul 8, 2024 · 1 comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@bjackman
Copy link

bjackman commented Jul 8, 2024

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 bjackman added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jul 8, 2024
@bjackman
Copy link
Author

bjackman commented Jul 8, 2024

Perhaps related: #9683

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

1 participant