Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cloneable opened this issue Apr 12, 2023 · 6 comments

Comments

@cloneable
Copy link
Contributor

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 {

@blackbeam
Copy link
Owner

@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?

@cloneable
Copy link
Contributor Author

cloneable commented Apr 13, 2023

@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.

@cloneable
Copy link
Contributor Author

cloneable commented Apr 13, 2023

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.

@blackbeam
Copy link
Owner

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

@cloneable
Copy link
Contributor Author

Yes, this seems to work for me, too.

@cloneable
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants