Repository: diesel-rs/diesel
File: diesel/src/r2d2.rs (Rust)

Code before:
        // check that we remove a connection from the pool that was
        // open during panicking
        #[allow(unreachable_code, unused_variables)]
        std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
            let conn = pool.get();
            assert_eq!(acquire_count.load(Ordering::Relaxed), 2);
            assert_eq!(release_count.load(Ordering::Relaxed), 1);
            assert_eq!(checkin_count.load(Ordering::Relaxed), 2);
            assert_eq!(checkout_count.load(Ordering::Relaxed), 3);
            panic!();
            std::mem::drop(conn);
        }))
        .unwrap_err();

        // we are not interested in the acquire count here
        // as the pool opens a new connection in the background
        // that could lead to this test failing if that happens to fast
        // (which is sometimes the case for sqlite)
        //assert_eq!(acquire_count.load(Ordering::Relaxed), 2);
        assert_eq!(release_count.load(Ordering::Relaxed), 2);
        assert_eq!(checkin_count.load(Ordering::Relaxed), 3);
        assert_eq!(checkout_count.load(Ordering::Relaxed), 3);
    }

    #[cfg(feature = "postgres")]
    #[test]
    fn verify_that_begin_test_transaction_works_with_pools() {
        use crate::prelude::*;
        use crate::r2d2::*;

        table! {
            users {
                id -> Integer,
                name -> Text,
            }
        }

        #[derive(Debug)]
        struct TestConnectionCustomizer;

        impl<E> CustomizeConnection<PgConnection, E> for TestConnectionCustomizer {
            fn on_acquire(&self, conn: &mut PgConnection) -> Result<(), E> {
                conn.begin_test_transaction()
                    .expect("Failed to start test transaction");

                Ok(())
            }
        }

        let manager = ConnectionManager::<PgConnection>::new(database_url());
        let pool = Pool::builder()

Review comment (bug):
This is unfortunate. Do we know what we're waiting for? A timed delay like this has the potential to be unreliable, e.g., if the test system is slow for some reason. If possible it would be best to wait in a loop, testing for whatever condition is needed to avoid the segfault.

Code after:

        // check that we remove a connection from the pool that was
        // open during panicking
        #[allow(unreachable_code, unused_variables)]
        std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
            let conn = pool.get();
            assert_eq!(acquire_count.load(Ordering::Relaxed), 2);
            assert_eq!(release_count.load(Ordering::Relaxed), 1);
            assert_eq!(checkin_count.load(Ordering::Relaxed), 2);
            assert_eq!(checkout_count.load(Ordering::Relaxed), 3);
            panic!();
            std::mem::drop(conn);
        }))
        .unwrap_err();

        // we are not interested in the acquire count here
        // as the pool opens a new connection in the background
        // that could lead to this test failing if that happens to fast
        // (which is sometimes the case for sqlite)
        //assert_eq!(acquire_count.load(Ordering::Relaxed), 2);
        assert_eq!(release_count.load(Ordering::Relaxed), 2);
        assert_eq!(checkin_count.load(Ordering::Relaxed), 3);
        assert_eq!(checkout_count.load(Ordering::Relaxed), 3);
        // this is required to workaround a segfault while shutting down
        // the pool
        std::thread::sleep(Duration::from_millis(100));
    }

    #[cfg(feature = "postgres")]
    #[test]
    fn verify_that_begin_test_transaction_works_with_pools() {
        use crate::prelude::*;
        use crate::r2d2::*;

        table! {
            users {
                id -> Integer,
                name -> Text,
            }
        }

        #[derive(Debug)]
        struct TestConnectionCustomizer;

        impl<E> CustomizeConnection<PgConnection, E> for TestConnectionCustomizer {
            fn on_acquire(&self, conn: &mut PgConnection) -> Result<(), E> {
                conn.begin_test_transaction()
                    .expect("Failed to start test transaction");

                Ok(())
            }