blackbeam/mysql_async

Regression in 0.32: Pool::disconnect does not wait for conns to be dropped anymore

cloneable opened this issue · 6 comments

The following assert fails in a test program with 0.32. Works fine with 0.31.

// we're about to exit, so there better be no outstanding connections
assert_eq!(exchange.exist, 0);

#[tokio::main]
async fn main() -> mysql_async::Result<()> {
    let pool = mysql_async::Pool::new("mysql://root:password@127.0.0.1:3307/mysql");

    tokio::spawn({
        let pool = pool.clone();
        async move {
            let mut _conn = pool.get_conn().await.unwrap();
            tokio::time::sleep(std::time::Duration::from_secs(5)).await;
        }
    });

    pool.disconnect().await?;

    Ok(())
}

/// Async function that disconnects this pool from the server and resolves to `()`.
///
/// **Note:** This Future won't resolve until all active connections, taken from it,
/// are dropped or disonnected. Also all pending and new `GetConn`'s will resolve to error.
pub fn disconnect(self) -> DisconnectPool {

@cloneable, hi.

I can't reproduce the issue so it's hard to know whether 8c4b72a fixes it.
Could you please try to reproduce on the updated master?

@blackbeam, sorry, the code snippet above was bad and has a race condition. Please see if you can reproduce it with this taken from the README. The fix to master did not help.

use std::error::Error as StdError;
use tokio::task::JoinHandle;

#[tokio::main]
async fn main() -> Result<(), Box<dyn StdError + 'static>> {
    let pool = mysql_async::Pool::new("mysql://root:password@127.0.0.1:3307/mysql");

    let task: JoinHandle<mysql_async::Result<()>> = tokio::spawn({
        let pool = pool.clone();
        let mut conn = pool.get_conn().await?;
        async move {
            use mysql_async::prelude::*;

            #[derive(Debug, PartialEq, Eq, Clone)]
            struct Payment {
                customer_id: i32,
                amount: i32,
                account_name: Option<String>,
            }

            let payments = vec![Payment {
                customer_id: 1,
                amount: 2,
                account_name: None,
            }];

            r"CREATE TEMPORARY TABLE payment (
                    customer_id int not null,
                    amount tinyint(3) not null,
                    account_name text
                )"
            .ignore(&mut conn)
            .await?;

            r"INSERT INTO payment (customer_id, amount, account_name)
                  VALUES (:customer_id, :amount, :account_name)"
                .with(payments.iter().map(|payment| {
                    params! {
                        "customer_id" => payment.customer_id,
                        "amount" => payment.amount,
                        "account_name" => payment.account_name.as_ref(),
                    }
                }))
                .batch(&mut conn)
                .await?;

            drop(conn);

            Ok(())
        }
    });

    task.await??;

    pool.disconnect().await?;

    Ok(())
}
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`', $HOME/.cargo/git/checkouts/mysql_async-a79c69708a1aa355/8c4b72a/src/conn/pool/recycler.rs:212:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
   2: core::panicking::assert_failed_inner

If it matters, I'm currently testing against MariaDB 10.3.16. Gonna test against a newer one.

Same with MariaDB 10.11.2. Found this in the logs: [Warning] Aborted connection 4 to db: 'mysql' user: 'root' host: 'x.x.x.x' (Got an error reading communication packets), but that's probably due to the rust binary terminating.

Thanks for the snippet.
f23dca0 seem to fix it. Could you please also confirm?

Yes, this seems to work for me, too.

Thank you! Can be closed. Could you yank 0.32.0 from crates.io, so no one uses it?