Confusion with MutexGuards and how clippy thinks about them

I'm currently a bit confused about MutexGuards and how clippy disagrees with my understanding of what is happening there. The following example is taken from my code and stripped down to the relevant parts (at least by my understanding):

let cars: Arc<Mutex<VecDeque<Cars>>> = Arc::new(Mutex::new(VecDeque::new()));
// [...]
// Add elements to `cars`
// [...]
while let Some(mut car) = cars.lock().unwrap().pop_front() {
	car.crash().await.unwrap();
}

When asking clippy about the code above it warns about a MutexGuard held across an await point.

warning: this `MutexGuard` is held across an `await` point
   --> src/main.rs:202:31
    |
202 |     while let Some(mut car) = cars.lock().unwrap().pop_front() {
    |                               ^^^^^^^^^^^^^^^^^^^^
    |
    = 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:202:5
    |
202 | /     while let Some(mut car) = cars.lock().unwrap().pop_front() {
203 | |         car.crash().await.unwrap();
204 | |     }
    | |_____^
    = 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 do understand where clippy might seem to see the problem, but for my understanding after extracting an element from the cars vector by locking it and calling the pop_front() method the vector is locked again before entering the inner part of the while loop.

I tried further and came to this code snippet but clippy is still not with me (actually the code is pretty much identical to the one above...):

loop {
    if let Some(mut car) = cars.lock().unwrap().pop_front() {
        car.crash().await.unwrap();
    } else {
        break;
    }
}

With a slight modification clippy seems finally to be happy with me:

loop {
    let cars = cars.lock().unwrap().pop_front();
    if let Some(mut car) = cars {
        car.crash().await.unwrap();
    } else {
        break;
    }
}

Where is the actual difference between the last two snippets? At least for me, the former is just a cleaned up version of the latter one.

Temporary values are kept alive for the duration of the statement defining them, which in this case is the entire if let including the block with the await.

3 Likes

This has to do with the way while let is desugared (see the reference) and the way drop scopes work (see the reference). It's a little weird because while behaves differently than while let.

2 Likes

Ah, I think I got it. Thanks :slight_smile:

Should also work:

use std::iter;
for car in iter::from_fn(|| cars.lock().unwrap().pop_front()) {
    car.crash().await.unwrap();
}
3 Likes

Nice, works too :slight_smile:

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.