[cargo clippy warning]warning: this `MutexGuard` is held across an `await` point

help
1

I am in the process of learning Rust.
This code runs fine with cargo run. However
When I run cargo clippy, I get the following warning and I am looking for a solution. await_holding_lock I understand the content, but I just can't seem to resolve the warning. I am sorry, but I am not sure how to improve this issue. I am sorry, but could you please tell me how to improve it?

  • Code
se anyhow::Result;
use tokio_postgres::{Client, NoTls};
use once_cell::sync::Lazy;
use std::sync::Mutex;
use crate::params::ConnectParams;

pub static CONNECT_PARAMS: Lazy<Mutex<ConnectParams>> =
    Lazy::new(||{
        let params = ConnectParams::new(
            "localhost".to_owned(), 5432, "rust_sample".to_owned(),
            "postgres".to_owned(), "admin".to_owned()
        );
        Mutex::new(params)
    });

pub struct AsyncSimpleClient;
impl AsyncSimpleClient {
    pub async fn connect() -> Result<Client> {
        let params = CONNECT_PARAMS.lock().unwrap();  // params lock here.
        let config = params.connect_string().clone();
        let (client, connection) = tokio_postgres::connect(config.as_str(), NoTls).await?;
        let handle = tokio::spawn(async move {
            if let Err(e) = connection.await {
                eprintln!("Connection error: {}", e);
            }
        });
        Ok(client)
    } // params drop here.
}
  • Warning
warning: this `MutexGuard` is held across an `await` point
  --> practice_collection/postgres/src/tokio_connect.rs:23:13
   |
23 |         let params = CONNECT_PARAMS.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
  --> practice_collection/postgres/src/tokio_connect.rs:23:9
   |
23 | /         let params = CONNECT_PARAMS.lock().unwrap();
24 | |         let config = params.connect_string().clone();
25 | |         let (client, connection) = tokio_postgres::connect(config.as_str(), NoTls).await?;
26 | |         let handle = tokio::spawn(async move {
...  |
32 | |         Ok(client)
33 | |     }
   | |_____^
   = 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
1 Like
2

Have you tried something like this yet?

- let params = CONNECT_PARAMS.lock().unwrap();
- let config = params.connect_string().clone();
+ let config = CONNECT_PARAMS.lock().unwrap().connect_string().clone();
2 Likes
3

Or also this if you don't want to make it a one-liner.

- let params = CONNECT_PARAMS.lock().unwrap();  // params lock here.
- let config = params.connect_string().clone();
+ let config = {
+     let params = CONNECT_PARAMS.lock().unwrap();  // params lock here.
+     params.connect_string().clone()
+     // params gets droped here.
+ }

Or even just

let params = CONNECT_PARAMS.lock().unwrap();  // params lock here.
let config = params.connect_string().clone();
+ std::mem::drop(params);
4

If I recall correctly, a call to drop may not be enough to help out the static analysis here, in particular the one that will eventually determine Send/Sync bounds of the generated future. If that is the case, i.e. if a std::sync::MutexGuard is still considered “held” over an await, and the future thus not Send, this will easily lead to actual compilation errors (not just clippy warnings) down the line if you try to execute the future in a multi-threaded executor.

For non-one-liner solutions there’s also

let config;
{
    let params = CONNECT_PARAMS.lock().unwrap();  // params lock here.
    config = params.connect_string().clone();
} // params drop here.

which scales nicely to cases where more than one variable should be defined using params.

2 Likes
5

I just tried it and am amazed that the warnings disappeared with flying colors!

1 Like
6

Assuming my caveat that explicit drop(…) might not really work actually applies, the suggestion in the lint to consider “ensuring the MutexGuard is dropped before calling await” is maybe suboptimal, as what you’d really want is to ensure the scope of the variable holding the MutexGuard ends before calling await.

Someone might want to test my assumption, and open an issue on the clippy repo :slight_smile:

1 Like
7

I also like this drop method.
However, I was thinking of avoiding the use of std::mem::drop if possible.

8

Thank you for the detailed explanation.
I also thought it was great to raise the issue on the clippy side.