fix(datatables): close the cleanup connection on drop instead of detaching it

Detaching released the pool permit while the session stayed alive, so
concurrent cleanups waiting on their locks could open as many connections as
they liked. Closing on drop covers the same cancellation and keeps them
counted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Diego Imbert
2026-09-21 10:44:43 +02:00
co-authored by Claude Opus 5
parent 9e1b9fee60
commit 3900e232ab
+10 -9
View File
@@ -1500,13 +1500,14 @@ pub async fn lock_instance_databases<'a>(
/// handed to anything yet.
///
/// The lock, the check and the drop share one connection: a second one taken from the pool while
/// the first is held could wait forever on a small pool. That connection is detached from the
/// pool, since a session lock outlives the future holding it: a cancellation between taking the
/// lock and releasing it would otherwise hand a locked session back to the pool, where every
/// later settings save waits on it. Detached, the connection is closed when it is dropped —
/// cancellation included — and the server releases the lock with the session.
/// the first is held could wait forever on a small pool. That connection is closed rather than
/// returned, since a session lock outlives the future holding it: a cancellation between taking
/// the lock and releasing it would otherwise hand a locked session back to the pool, where every
/// later settings save waits on it. Closing on drop covers the cancellation and still counts the
/// connection against the pool, which detaching it would not.
pub async fn drop_unused_instance_database(db: &DB, dbname: &str) -> error::Result<Cleanup> {
let mut conn = db.acquire().await?.detach();
let mut conn = db.acquire().await?;
conn.close_on_drop();
let key = format!("instance_database:{dbname}");
// A save blocked on this lock is about to name the database, but it may also roll back — a
// later validation of its own, a superadmin check, a cancelled request. So it is let through
@@ -1518,7 +1519,7 @@ pub async fn drop_unused_instance_database(db: &DB, dbname: &str) -> error::Resu
let dropped = loop {
if let Err(e) = sqlx::query("SELECT pg_advisory_lock(hashtext($1))")
.bind(&key)
.execute(&mut conn)
.execute(&mut *conn)
.await
{
break Err(e.into());
@@ -1536,8 +1537,8 @@ pub async fn drop_unused_instance_database(db: &DB, dbname: &str) -> error::Resu
Err(e) => break Err(e),
}
};
// Closing releases the lock with the session, so nothing here depends on the unlock landing.
let _ = sqlx::Connection::close(conn).await;
// Dropping closes the connection, and the server releases the lock with the session, so
// nothing here depends on an unlock landing.
dropped
}