From 3900e232ab67a4e6e20a983e0b18414fdb75ddf2 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Mon, 21 Sep 2026 10:44:43 +0200 Subject: [PATCH] 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) --- backend/windmill-common/src/lib.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index 4d7714165a..3e74d8e17f 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -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 { - 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 }