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

await_holding_lock lint is erroneously triggered when the guard is explicitly dropped before the await. #9208

Open
peterjoel opened this issue Jul 19, 2022 · 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

@peterjoel
Copy link

peterjoel commented Jul 19, 2022

Summary

The code below triggers await_holding_lock. However, the guards are dropped explicitly before each .await so there should not be a problem.

Lint Name

await_holding_lock

Reproducer

I tried this code:

use std::{collections::*, sync::{Arc, RwLock}};
use thiserror::Error;
use tokio::sync::broadcast;

#[derive(Error, Debug, Clone, Default)]
#[error("There was an error")]
pub struct Error;

#[derive(Debug, Default)]
struct CacheEntry {
    prev_result: Option<Result<i32, Error>>,
    sender: Option<broadcast::Sender<Result<i32, Error>>>,
}

#[derive(Debug, Clone)]
pub struct Cache {
    cache: Arc<RwLock<HashMap<i32, CacheEntry>>>,
}

impl Cache {
    pub async fn retrieve(&self, key: i32) -> Result<i32, Error> {
        let guard = self.cache.read().unwrap();
        let val = guard.get(&key);
        match val {
            Some(entry) => match entry.prev_result.clone() {
                Some(result) => result,
                None => {
                    let mut receiver = entry.sender.as_ref().unwrap().subscribe();
                    drop(guard);
                    receiver.recv().await.unwrap()
                }
            },
            None => {
                drop(guard);
                let mut write_guard = self.cache.write().unwrap();
                let entry = write_guard.entry(key).or_default();
                let mut receiver = entry.sender.as_ref().unwrap().subscribe();
                drop(write_guard);
                receiver.recv().await.unwrap()
            }
        }
    }
}

I saw this happen:

warning: this `MutexGuard` is held across an `await` point
  --> src/lib.rs:22:13
   |
22 |         let guard = self.cache.read().unwrap();
   |             ^^^^^
   |
   = note: `#[warn(clippy::await_holding_lock)]` on by default
   = 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/lib.rs:22:9
   |
22 | /         let guard = self.cache.read().unwrap();
23 | |         let val = guard.get(&key);
24 | |         match val {
25 | |             Some(entry) => match entry.prev_result.clone() {
...  |
41 | |         }
42 | |     }
   | |_____^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock

warning: this `MutexGuard` is held across an `await` point
  --> src/lib.rs:35:21
   |
35 |                 let mut write_guard = self.cache.write().unwrap();
   |                     ^^^^^^^^^^^^^^^
   |
   = 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/lib.rs:35:17
   |
35 | /                 let mut write_guard = self.cache.write().unwrap();
36 | |                 let entry = write_guard.entry(key).or_default();
37 | |                 let mut receiver = entry.sender.as_ref().unwrap().subscribe();
38 | |                 drop(write_guard);
39 | |                 receiver.recv().await.unwrap()
40 | |             }
   | |_____________^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock

I expected to see this happen:

We expected no errors because the guards are explicitly dropped before each await point.

Version

rustc 1.62.0 (a8314ef7d 2022-06-27)
binary: rustc
commit-hash: a8314ef7d0ec7b75c336af2c9857bfaf43002bfc
commit-date: 2022-06-27
host: x86_64-unknown-linux-gnu
release: 1.62.0
LLVM version: 14.0.5

Additional Labels

No response

@peterjoel peterjoel 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 19, 2022
@jplatte
Copy link
Contributor

jplatte commented Mar 22, 2023

This is almost certainly caused by rust-lang/rust#57478, so this depends on rust-lang/rust#69663.

Also note that this lint was already generalized and put into rustc as must_not_suspend, however that is off by default because of the same bug.

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

2 participants