bug: reproducible test-db-race

* The first test thread that acquires the DO_CLEANUP permit can still run after a second test thread has already created their ephemeral test db. This will accidentally "cleanup" the currently in-use test db. Reproduce ========= Run the database containers: ```bash $ cd tests/ $ docker-compose up ``` Run the `test-db-race` tests. One test should fail. ```bash $ cd tests/ $ ./x.py -e mysql_8_tokio --test mysql-test-db-race test test_2 ... FAILED test test_1 ... ok failures: ---- test_2 stdout ---- created database _sqlx_test_database_5 thread 'test_2' panicked at 'failed to connect to test database: Database(MySqlDatabaseError { code: Some("42000"), number: 1049, message: "Unknown database '_sqlx_test_database_5'" })', /home/phlip9/dev/sqlx/sqlx-core/src/testing/mod.rs:245:10 $ ./x.py -e postgres_15_tokio --test postgres-test-db-race test test_1 ... FAILED test test_2 ... ok failures: thread 'test_1' panicked at 'failed to connect to test database: Database(PgDatabaseError { severity: Fatal, code: "3D000", message: "database \"_sqlx_test_69\" does not exist", detail: None, hint: None, position: None, where: None, schema: None, table: None, column: None, data_type: None, constraint: None, file: Some("postinit.c"), line: Some(941), routine: Some("InitPostgres") })', /home/phlip9/dev/sqlx/sqlx-core/src/testing/mod.rs:245:10 ```
phlip9 · Apr 13, 2023 · 60a5955 · 60a5955
1 parent 4f1ac1d
commit 60a5955
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 9 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -289,6 +289,11 @@ name = "mysql-migrate"
 path = "tests/mysql/migrate.rs"
 required-features = ["mysql", "macros", "migrate"]
 
+[[test]]
+name = "mysql-test-db-race"
+path = "tests/mysql/test-db-race.rs"
+required-features = ["mysql", "macros", "migrate"]
+
 #
 # PostgreSQL
 #
@@ -332,3 +337,8 @@ required-features = ["postgres", "macros", "migrate"]
 name = "postgres-migrate"
 path = "tests/postgres/migrate.rs"
 required-features = ["postgres", "macros", "migrate"]
+
+[[test]]
+name = "postgres-test-db-race"
+path = "tests/postgres/test-db-race.rs"
+required-features = ["postgres", "macros", "migrate"]
diff --git a/sqlx-core/src/testing/mod.rs b/sqlx-core/src/testing/mod.rs
@@ -212,6 +212,9 @@ where
             .await
             .expect("failed to connect to setup test database");
 
+        // help trigger the test-db-race
+        std::thread::sleep(std::time::Duration::from_secs(2));
+
         setup_test_db::<DB>(&test_context.connect_opts, &args).await;
 
         let res = test_fn(test_context.pool_opts, test_context.connect_opts).await;

diff --git a/sqlx-mysql/src/testing/mod.rs b/sqlx-mysql/src/testing/mod.rs
@@ -125,6 +125,9 @@ async fn test_context(args: &TestArgs) -> Result<TestContext<MySql>, Error> {
 
     // Only run cleanup if the test binary just started.
     if DO_CLEANUP.swap(false, Ordering::SeqCst) {
+        // help trigger the test-db-race
+        std::thread::sleep(Duration::from_secs(1));
+
         do_cleanup(&mut conn).await?;
     }
 

diff --git a/sqlx-postgres/src/testing/mod.rs b/sqlx-postgres/src/testing/mod.rs
@@ -135,6 +135,9 @@ async fn test_context(args: &TestArgs) -> Result<TestContext<Postgres>, Error> {
 
     // Only run cleanup if the test binary just started.
     if DO_CLEANUP.swap(false, Ordering::SeqCst) {
+        // help trigger the test-db-race
+        std::thread::sleep(Duration::from_secs(1));
+
         do_cleanup(&mut conn).await?;
     }
 

diff --git a/tests/mysql/test-db-race.rs b/tests/mysql/test-db-race.rs
@@ -0,0 +1,5 @@
+#[sqlx_macros::test]
+async fn test_1(_pool: sqlx::MySqlPool) {}
+
+#[sqlx_macros::test]
+async fn test_2(_pool: sqlx::MySqlPool) {}
diff --git a/tests/postgres/test-db-race.rs b/tests/postgres/test-db-race.rs
@@ -0,0 +1,5 @@
+#[sqlx_macros::test]
+async fn test_1(_pool: sqlx::PgPool) {}
+
+#[sqlx_macros::test]
+async fn test_2(_pool: sqlx::PgPool) {}
diff --git a/tests/x.py b/tests/x.py
@@ -130,7 +130,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
 for runtime in ["async-std", "tokio"]:
     for tls in ["native-tls", "rustls", "none"]:
         run(
-            f"cargo c --no-default-features --features all-databases,_unstable-all-types,macros,runtime-{runtime},tls-{tls}",
+            f"cargo c --no-default-features --features all-databases,_unstable-all-types,macros,migrate,runtime-{runtime},tls-{tls}",
             comment="check with async-std",
             tag=f"check_{runtime}_{tls}"
         )
@@ -159,7 +159,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
         #
 
         run(
-            f"cargo test --no-default-features --features any,sqlite,macros,_unstable-all-types,runtime-{runtime},tls-{tls}",
+            f"cargo test --no-default-features --features any,sqlite,macros,migrate,_unstable-all-types,runtime-{runtime},tls-{tls}",
             comment=f"test sqlite",
             service="sqlite",
             tag=f"sqlite" if runtime == "async-std" else f"sqlite_{runtime}",
@@ -171,7 +171,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
 
         for version in ["15", "14", "13", "12", "11"]:
             run(
-                f"cargo test --no-default-features --features any,postgres,macros,_unstable-all-types,runtime-{runtime},tls-{tls}",
+                f"cargo test --no-default-features --features any,postgres,macros,migrate,_unstable-all-types,runtime-{runtime},tls-{tls}",
                 comment=f"test postgres {version}",
                 service=f"postgres_{version}",
                 tag=f"postgres_{version}" if runtime == "async-std" else f"postgres_{version}_{runtime}",
@@ -180,7 +180,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
             if tls != "none":
                 ## +ssl
                 run(
-                    f"cargo test --no-default-features --features any,postgres,macros,_unstable-all-types,runtime-{runtime},tls-{tls}",
+                    f"cargo test --no-default-features --features any,postgres,macros,migrate,_unstable-all-types,runtime-{runtime},tls-{tls}",
                     comment=f"test postgres {version} ssl",
                     database_url_args="sslmode=verify-ca&sslrootcert=.%2Ftests%2Fcerts%2Fca.crt",
                     service=f"postgres_{version}",
@@ -189,7 +189,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
 
                 ## +client-ssl
                 run(
-                    f"cargo test --no-default-features --features any,postgres,macros,_unstable-all-types,runtime-{runtime},tls-{tls}",
+                    f"cargo test --no-default-features --features any,postgres,macros,migrate,_unstable-all-types,runtime-{runtime},tls-{tls}",
                     comment=f"test postgres {version}_client_ssl no-password",
                     database_url_args="sslmode=verify-ca&sslrootcert=.%2Ftests%2Fcerts%2Fca.crt&sslkey=.%2Ftests%2Fkeys%2Fclient.key&sslcert=.%2Ftests%2Fcerts%2Fclient.crt",
                     service=f"postgres_{version}_client_ssl",
@@ -202,7 +202,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
 
         for version in ["8", "5_7"]:
             run(
-                f"cargo test --no-default-features --features any,mysql,macros,_unstable-all-types,runtime-{runtime},tls-{tls}",
+                f"cargo test --no-default-features --features any,mysql,macros,migrate,_unstable-all-types,runtime-{runtime},tls-{tls}",
                 comment=f"test mysql {version}",
                 service=f"mysql_{version}",
                 tag=f"mysql_{version}" if runtime == "async-std" else f"mysql_{version}_{runtime}",
@@ -211,7 +211,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
             ## +client-ssl
             if tls != "none" and not(version == "5_7" and tls == "rustls"):
                 run(
-                    f"cargo test --no-default-features --features any,mysql,macros,_unstable-all-types,runtime-{runtime},tls-{tls}",
+                    f"cargo test --no-default-features --features any,mysql,macros,migrate,_unstable-all-types,runtime-{runtime},tls-{tls}",
                     comment=f"test mysql {version}_client_ssl no-password",
                     database_url_args="sslmode=verify_ca&ssl-ca=.%2Ftests%2Fcerts%2Fca.crt&ssl-key=.%2Ftests%2Fkeys%2Fclient.key&ssl-cert=.%2Ftests%2Fcerts%2Fclient.crt",
                     service=f"mysql_{version}_client_ssl",
@@ -224,7 +224,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
 
         for version in ["10_6", "10_5", "10_4", "10_3"]:
             run(
-                f"cargo test --no-default-features --features any,mysql,macros,_unstable-all-types,runtime-{runtime},tls-{tls}",
+                f"cargo test --no-default-features --features any,mysql,macros,migrate,_unstable-all-types,runtime-{runtime},tls-{tls}",
                 comment=f"test mariadb {version}",
                 service=f"mariadb_{version}",
                 tag=f"mariadb_{version}" if runtime == "async-std" else f"mariadb_{version}_{runtime}",
@@ -233,7 +233,7 @@ def run(command, comment=None, env=None, service=None, tag=None, args=None, data
             ## +client-ssl
             if tls != "none":
                 run(
-                    f"cargo test --no-default-features --features any,mysql,macros,_unstable-all-types,runtime-{runtime},tls-{tls}",
+                    f"cargo test --no-default-features --features any,mysql,macros,migrate,_unstable-all-types,runtime-{runtime},tls-{tls}",
                     comment=f"test mariadb {version}_client_ssl no-password",
                     database_url_args="sslmode=verify_ca&ssl-ca=.%2Ftests%2Fcerts%2Fca.crt&ssl-key=.%2Ftests%2Fkeys%2Fclient.key&ssl-cert=.%2Ftests%2Fcerts%2Fclient.crt",
                     service=f"mariadb_{version}_client_ssl",