From d1f1633c6038df3c6101cbfe9cc484d8e6ffbcb4 Mon Sep 17 00:00:00 2001
From: iequidoo <dgreshilov@gmail.com>
Date: Sat, 21 Mar 2026 14:45:34 -0300
Subject: [PATCH] refactor: Remove wal_checkpoint_mutex, lock write_mutex
 before getting sql connection instead

The original idea was to always lock `write_mutex` before acquiring an `InnerPool.semaphore` permit
to avoid ABBA deadlocks, but when refactoring a PR for b696a242fc71f87ebde4748acf7a7547b147cbc4,
that was forgotten.

This doesn't really change the program flow as we have `Context::housekeeping_mutex` anyway,
just simplifies the code.
---
 src/sql/pool.rs                | 19 -------------------
 src/sql/pool/wal_checkpoint.rs | 14 ++++++++------
 2 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/src/sql/pool.rs b/src/sql/pool.rs
index 0feb305e0..2a428cd8c 100644
--- a/src/sql/pool.rs
+++ b/src/sql/pool.rs
@@ -71,24 +71,6 @@ struct InnerPool {
     /// This mutex is locked when write connection
     /// is outside the pool.
     pub(crate) write_mutex: Arc<Mutex<()>>,
-
-    /// WAL checkpointing mutex.
-    ///
-    /// This mutex ensures that no more than one thread
-    /// runs WAL checkpointing at the same time.
-    ///
-    /// Normal procedures acquire either one read connection
-    /// or one write connection with a write mutex,
-    /// and return the resources without trying to acquire
-    /// more connections or trying to acquire write mutex
-    /// without returning the read connection first.
-    /// WAL checkpointing is special, it tries to acquire all
-    /// connections and the write mutex,
-    /// so two threads doing this at the same time
-    /// may result in a deadlock with one thread
-    /// waiting for a write lock and the other thread
-    /// waiting for a connection.
-    wal_checkpoint_mutex: Mutex<()>,
 }
 
 impl InnerPool {
@@ -209,7 +191,6 @@ impl Pool {
             connections: parking_lot::Mutex::new(connections),
             semaphore,
             write_mutex: Default::default(),
-            wal_checkpoint_mutex: Default::default(),
         });
         Pool { inner }
     }
diff --git a/src/sql/pool/wal_checkpoint.rs b/src/sql/pool/wal_checkpoint.rs
index c32e92656..0d95be93b 100644
--- a/src/sql/pool/wal_checkpoint.rs
+++ b/src/sql/pool/wal_checkpoint.rs
@@ -34,7 +34,6 @@ pub(crate) struct WalCheckpointStats {
 
 /// Runs a checkpoint operation in TRUNCATE mode, so the WAL file is truncated to 0 bytes.
 pub(super) async fn wal_checkpoint(pool: &Pool) -> Result<WalCheckpointStats> {
-    let _guard = pool.inner.wal_checkpoint_mutex.lock().await;
     let t_start = Time::now();
 
     // Do as much work as possible without blocking anybody.
@@ -48,22 +47,25 @@ pub(super) async fn wal_checkpoint(pool: &Pool) -> Result<WalCheckpointStats> {
         conn.query_row("PRAGMA wal_checkpoint(PASSIVE)", [], |_| Ok(()))
     })?;
 
-    // Kick out writers.
-    const _: () = assert!(Sql::N_DB_CONNECTIONS > 1, "Deadlock possible");
+    // Kick out writers. `write_mutex` should be locked before taking an `InnerPool.semaphore`
+    // permit to avoid ABBA deadlocks, so drop `conn` which holds a semaphore permit.
+    drop(conn);
     let _write_lock = Arc::clone(&pool.inner.write_mutex).lock_owned().await;
     let t_writers_blocked = Time::now();
+    let conn = pool.get(query_only).await?;
     // Ensure that all readers use the most recent database snapshot (are at the end of WAL) so
     // that `wal_checkpoint(FULL)` isn't blocked. We could use `PASSIVE` as well, but it's
     // documented poorly, https://www.sqlite.org/pragma.html#pragma_wal_checkpoint and
     // https://www.sqlite.org/c3ref/wal_checkpoint_v2.html don't tell how it interacts with new
     // readers.
-    let mut read_conns = Vec::with_capacity(crate::sql::Sql::N_DB_CONNECTIONS - 1);
-    for _ in 0..(crate::sql::Sql::N_DB_CONNECTIONS - 1) {
+    let mut read_conns = Vec::with_capacity(Sql::N_DB_CONNECTIONS - 1);
+    for _ in 0..(Sql::N_DB_CONNECTIONS - 1) {
         read_conns.push(pool.get(query_only).await?);
     }
     read_conns.clear();
     // Checkpoint the remaining WAL pages without blocking readers.
     let (pages_total, pages_checkpointed) = tokio::task::block_in_place(|| {
+        conn.query_row("PRAGMA table_list", [], |_| Ok(()))?;
         conn.query_row("PRAGMA wal_checkpoint(FULL)", [], |row| {
             let pages_total: i64 = row.get(1)?;
             let pages_checkpointed: i64 = row.get(2)?;
@@ -71,7 +73,7 @@ pub(super) async fn wal_checkpoint(pool: &Pool) -> Result<WalCheckpointStats> {
         })
     })?;
     // Kick out readers to avoid blocking/SQLITE_BUSY.
-    for _ in 0..(crate::sql::Sql::N_DB_CONNECTIONS - 1) {
+    for _ in 0..(Sql::N_DB_CONNECTIONS - 1) {
         read_conns.push(pool.get(query_only).await?);
     }
     let t_readers_blocked = Time::now();