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.43k stars 1.54k forks source link

False positive for `await_holding_lock` on Rust 1.61 beta #8777

Open umanwizard opened 2 years ago

umanwizard commented 2 years ago

Summary

This program triggers await_holding_lock, but the lock is not actually held across an await point (it is dropped at the end of its enclosing scope, which is the block in the condition position of the while let loop).

struct S;

impl S {
    fn get_int(&mut self) -> Option<usize> {
        Some(42)
    }
}

use std::sync::Mutex;
use std::sync::Arc;
use std::time::Duration;

#[tokio::main]
async fn main() {
    let s = Arc::new(Mutex::new(S));
    while let Some(sz) = {
        let mut guard = s.lock().unwrap();
        guard.get_int()
    } {
        println!("got int {sz}");
        tokio::time::sleep(Duration::from_secs(1)).await;
    }
}

Lint Name

No response

Reproducer

I tried this code:

struct S;

impl S {
    fn get_int(&mut self) -> Option<usize> {
        Some(42)
    }
}

use std::sync::Mutex;
use std::sync::Arc;
use std::time::Duration;

#[tokio::main]
async fn main() {
    let s = Arc::new(Mutex::new(S));
    while let Some(sz) = {
        let mut guard = s.lock().unwrap();
        guard.get_int()
    } {
        println!("got int {sz}");
        tokio::time::sleep(Duration::from_secs(1)).await;
    }
}

I saw this happen:

warning: this `MutexGuard` is held across an `await` point
  --> src/main.rs:18:9
   |
18 |         guard.get_int()
   |         ^^^^^
   |
   = 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/main.rs:16:5
   |
16 | /     while let Some(sz) = {
17 | |         let mut guard = s.lock().unwrap();
18 | |         guard.get_int()
19 | |     } {
20 | |         println!("got int {sz}");
21 | |         tokio::time::sleep(Duration::from_secs(1)).await;
22 | |     }
   | |_____^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock

I expected to see this happen:

(no output)

Version

rustc 1.61.0-beta.4 (69a6d12e9 2022-04-25)
binary: rustc
commit-hash: 69a6d12e9f0372e3c6d82bc7c7e410dab02d0500
commit-date: 2022-04-25
host: x86_64-unknown-linux-gnu
release: 1.61.0-beta.4
LLVM version: 14.0.0

Additional Labels

No response

kaimast commented 2 years ago

Maybe this lint should be disabled by default until this bug is fixed?

umanwizard commented 2 years ago

Yes please -- buggy lints are worse than nothing IMO.

kaimast commented 11 months ago

This bug still persists one year later. It would be really great if you could disable the lint by default.