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.46k stars 1.55k forks source link

await_holding_lock reports explicitly dropped mutex guards as if they weren't dropped #6446

Open jazeved0 opened 3 years ago

jazeved0 commented 3 years ago

I tried this code:

#[deny(clippy::pedantic)]
pub async fn doesnt_work() {
    use std::collections::HashSet;
    use std::sync::Mutex;
    use std::time::Duration;

    let lock: Mutex<HashSet<u8>> = Mutex::new(HashSet::new());
    let mut set = lock.lock().unwrap();
    set.insert(0);
    drop(set);
    tokio::time::sleep(Duration::from_secs(1)).await;
}

I expected to see this happen: it should pass Clippy with no linting errors because the MutexGuard is explicitly dropped before the await point with a call to std::mem::drop.

Instead, this happened: Clippy failed the lint check because of a violation of clippy::await_holding_lock:

$ cargo clippy
...
error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await.
  --> src/lib.rs:8:9
   |
8  |     let mut set = lock.lock().unwrap();
   |         ^^^^^^^
   |
note: the lint level is defined here
  --> src/lib.rs:1:8
   |
1  | #[deny(clippy::pedantic)]
   |        ^^^^^^^^^^^^^^^^
   = note: `#[deny(clippy::await_holding_lock)]` implied by `#[deny(clippy::pedantic)]`
note: these are all the await points this lock is held through
  --> src/lib.rs:8:5
   |
8  | /     let mut set = lock.lock().unwrap();
9  | |     set.insert(0);
10 | |     drop(set);
11 | |     tokio::time::sleep(Duration::from_secs(1)).await;
12 | | }
   | |_^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock

error: aborting due to previous error

Running with --verbose does not provide any additional information.

Additionally, of note, the following code does pass Clippy:

#[deny(clippy::pedantic)]
pub async fn does_work() {
    use std::collections::HashSet;
    use std::sync::Mutex;
    use std::time::Duration;

    let lock: Mutex<HashSet<u8>> = Mutex::new(HashSet::new());
    {
        let mut set = lock.lock().unwrap();
        set.insert(0);
    }
    tokio::time::sleep(Duration::from_secs(1)).await;
}

Meta

Backtrace

``` $ RUST_BACKTRACE=1 cargo clippy Checking bug_report v0.1.0 (/home/jazev/dev/architus/logs/bug_report) error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await. --> src/lib.rs:8:9 | 8 | let mut set = lock.lock().unwrap(); | ^^^^^^^ | note: the lint level is defined here --> src/lib.rs:1:8 | 1 | #[deny(clippy::pedantic)] | ^^^^^^^^^^^^^^^^ = note: `#[deny(clippy::await_holding_lock)]` implied by `#[deny(clippy::pedantic)]` note: these are all the await points this lock is held through --> src/lib.rs:8:5 | 8 | / let mut set = lock.lock().unwrap(); 9 | | set.insert(0); 10 | | drop(set); 11 | | tokio::time::sleep(Duration::from_secs(1)).await; 12 | | } | |_^ = help: for further information visit https://rust-lang.github.io/rust- clippy/master/index.html#await_holding_lock error: aborting due to previous error error: could not compile `bug_report` To learn more, run the command again with --verbose. ```

ebroto commented 3 years ago

Thanks for reporting this.

This is a duplicate of #6353 as the underlying code is the same, but I will leave this open so other users can find the problem searching for either lint.

flip1995 commented 2 years ago

Fixing this would involve fixing/improving drop-tracking in rustc when it comes to building the generator_interior_types collection. drop-tracking is currently disabled by default though. So nothing much that could be done here on the Clippy side.

Sytten commented 5 months ago

Has anything changed in the last year+ concerning drop tracking? @flip1995

flip1995 commented 5 months ago

I don't know. With a quick grep though the Rust codebase, I couldn't find the drop_tracking option anymore. So it might be enabled by default now. But I'm not sure.

urkle commented 1 month ago

Drop tracking was enabled by default in rust 1.74 ( https://github.com/rust-lang/rust/releases/tag/1.74.0 )