max / makenotwork
1 file changed,
+58 insertions,
-19 deletions
| @@ -302,8 +302,7 @@ impl Drop for TestDb { | |||
| 302 | 302 | fn drop(&mut self) { | |
| 303 | 303 | let admin_url = self.admin_url.clone(); | |
| 304 | 304 | let db_name = self.db_name.clone(); | |
| 305 | - | ||
| 306 | - | self.pool.close_event(); | |
| 305 | + | let pool = self.pool.clone(); | |
| 307 | 306 | ||
| 308 | 307 | std::thread::spawn(move || { | |
| 309 | 308 | let rt = tokio::runtime::Builder::new_current_thread() | |
| @@ -312,24 +311,64 @@ impl Drop for TestDb { | |||
| 312 | 311 | .expect("Failed to build cleanup runtime"); | |
| 313 | 312 | ||
| 314 | 313 | rt.block_on(async { | |
| 315 | - | if let Ok(mut conn) = PgConnection::connect(&admin_url).await { | |
| 316 | - | // Adopt the shared role so we can drop a clone it owns even | |
| 317 | - | // when our login role differs from the creator's. | |
| 318 | - | assume_shared_role(&mut conn).await; | |
| 319 | - | ||
| 320 | - | let _ = conn | |
| 321 | - | .execute( | |
| 322 | - | format!( | |
| 323 | - | "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '{}'", | |
| 324 | - | db_name | |
| 325 | - | ) | |
| 326 | - | .as_str(), | |
| 327 | - | ) | |
| 328 | - | .await; | |
| 314 | + | // Actually close the pool, and wait for it. Every one of its | |
| 315 | + | // live connections is a client of the database we're about to | |
| 316 | + | // drop, and `DROP DATABASE` refuses while any exist. Bounded: | |
| 317 | + | // `close()` waits for checked-out connections to come back, and | |
| 318 | + | // a test that leaked a guard would otherwise hang the suite | |
| 319 | + | // here forever. On timeout we fall through — the FORCE drop | |
| 320 | + | // below evicts whatever is left. | |
| 321 | + | if tokio::time::timeout(Duration::from_secs(5), pool.close()) | |
| 322 | + | .await | |
| 323 | + | .is_err() | |
| 324 | + | { | |
| 325 | + | eprintln!( | |
| 326 | + | "[test-harness] pool for {db_name} did not close in 5s; forcing" | |
| 327 | + | ); | |
| 328 | + | } | |
| 329 | 329 | ||
| 330 | - | let _ = conn | |
| 331 | - | .execute(format!("DROP DATABASE IF EXISTS \"{}\"", db_name).as_str()) | |
| 332 | - | .await; | |
| 330 | + | let Ok(mut conn) = PgConnection::connect(&admin_url).await else { | |
| 331 | + | eprintln!("[test-harness] LEAKED {db_name}: admin connect failed"); | |
| 332 | + | return; | |
| 333 | + | }; | |
| 334 | + | ||
| 335 | + | // Deliberately NO `SET ROLE mnw_test` here, unlike the create | |
| 336 | + | // path. Terminating a backend requires membership in the role | |
| 337 | + | // that *opened* it, and these were opened by our login role | |
| 338 | + | // (`max`, `sando`), not by `mnw_test` — which is a member of | |
| 339 | + | // neither. Assuming the shared role therefore turns both the | |
| 340 | + | // sweep below and FORCE's own eviction into "permission denied | |
| 341 | + | // to terminate process", leaking the clone. Dropping needs only | |
| 342 | + | // membership in the owning role (pg_has_role USAGE), which we | |
| 343 | + | // already have as the login role, so staying put satisfies both. | |
| 344 | + | let _ = conn | |
| 345 | + | .execute( | |
| 346 | + | format!( | |
| 347 | + | "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '{db_name}'" | |
| 348 | + | ) | |
| 349 | + | .as_str(), | |
| 350 | + | ) | |
| 351 | + | .await; | |
| 352 | + | ||
| 353 | + | // FORCE, and retry: a connection can still be mid-handshake | |
| 354 | + | // when we terminate, so it survives the sweep above and lands | |
| 355 | + | // in the split second before the drop. Bare `DROP DATABASE` | |
| 356 | + | // fails outright on that race; FORCE evicts it, and the retries | |
| 357 | + | // cover the case where another arrives. | |
| 358 | + | let drop_sql = format!("DROP DATABASE IF EXISTS \"{db_name}\" WITH (FORCE)"); | |
| 359 | + | for attempt in 0..4u32 { | |
| 360 | + | match conn.execute(drop_sql.as_str()).await { | |
| 361 | + | Ok(_) => return, | |
| 362 | + | Err(e) if attempt == 3 => { | |
| 363 | + | // Never silent: a swallowed failure here is exactly | |
| 364 | + | // how the cluster accumulated ~1000 stale clones. | |
| 365 | + | eprintln!("[test-harness] LEAKED {db_name}: drop failed: {e}"); | |
| 366 | + | } | |
| 367 | + | Err(_) => { | |
| 368 | + | tokio::time::sleep(Duration::from_millis(50 * (attempt + 1) as u64)) | |
| 369 | + | .await; | |
| 370 | + | } | |
| 371 | + | } | |
| 333 | 372 | } | |
| 334 | 373 | }); | |
| 335 | 374 | }) |