rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

False-positive `await_holding_lock` with manual `drop` of `MutexGuard`

Open Swatinem opened this issue 3 years ago • 3 comments

Summary

I manually drop my MutexGuard right before the await point, therefore the lock is not held across await points.

Lint Name

await_holding_lock

Reproducer

I tried this code:

async fn test_clippy_mutex_await() {
    let mutex = std::sync::Mutex::new(());
    let lock = mutex.lock().unwrap();
    drop(lock);

    (async {}).await;
}

I saw this happen:

warning: this `MutexGuard` is held across an `await` point
  --> crates\symbolicator-cache\src\lib.rs:91:9
   |
91 |     let lock = mutex.lock().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
  --> crates\symbolicator-cache\src\lib.rs:91:5
   |
91 | /     let lock = mutex.lock().unwrap();
92 | |     drop(lock);
93 | |
94 | |     (async {}).await;
95 | | }
   | |_^
   = 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

I expected to see this happen:

This should not error as the MutexGuard is not alive at the await point.

Version

rustc 1.66.0-nightly (dcb376115 2022-10-20)
binary: rustc
commit-hash: dcb376115066d111dbf5f13d5ac2a2dbe8c12add
commit-date: 2022-10-20
host: x86_64-pc-windows-msvc
release: 1.66.0-nightly
LLVM version: 15.0.2

Additional Labels

No response

Swatinem avatar Oct 21 '22 11:10 Swatinem

I'm not sure if this use case is common enough to make the lint handle it. I would throw an allow in this particular case.

kraktus avatar Oct 21 '22 11:10 kraktus

I’m doing that, and it is a good workaround for me.

I still think its a valid false-positive, even if the solution right now is "not worth the effort fixing".

Swatinem avatar Oct 21 '22 11:10 Swatinem

Wanted to add that this was affecting me also today, perhaps this is more common than suggested by kraktus. (we ran into it in a resource management loop, which is for us at least a somewhat common pattern)

davidv1992 avatar Feb 22 '24 13:02 davidv1992

I also came across this for a loop. The lock is needed at the start and end of the loop. By dropping the lock manually, the mutex only needs to be locked once per iteration instead of twice.

let mut lock = mutex.lock().unwrap();
loop {
	let future = start_computation(&lock);
	drop(lock);

	let result = future.await;

	lock = mutex.lock().unwrap();
	apply_result(&lock, result);
}

Playground

zroug avatar Mar 10 '24 04:03 zroug