From a1ca4a84b57013953705f10fbe629d0740ccba23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Lucas?= Date: Wed, 10 Jun 2026 13:31:22 -0300 Subject: [PATCH] fix(storage): serialize msg_secret reads through the db semaphore get_msg_secret and get_msg_secret_with_ts ran on raw spawn_blocking, bypassing the single-permit db semaphore every write path acquires. A read racing a write transaction hits the shared-cache table lock on in-memory stores (SQLITE_LOCKED, which busy_timeout does not cover) and the caller treats the error as a missing secret, so an inbound addon could spuriously fail to find its parent secret. The write-behind drain made the overlap reachable: the enc-comment pipeline test flaked at roughly coin-flip rate per process. File-backed stores were shielded by WAL. Both reads now take the semaphore like the rest of the store; the flaky test passes 12/12 consecutive runs against this fix. --- storages/sqlite-storage/src/sqlite_store.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/storages/sqlite-storage/src/sqlite_store.rs b/storages/sqlite-storage/src/sqlite_store.rs index ee8f1dc83..b3be9e430 100644 --- a/storages/sqlite-storage/src/sqlite_store.rs +++ b/storages/sqlite-storage/src/sqlite_store.rs @@ -2869,12 +2869,15 @@ impl MsgSecretStore for SqliteStore { sender: &str, msg_id: &str, ) -> Result>> { + // Serialized through the db semaphore for the same reason as + // get_msg_secret_with_ts: a read racing a write transaction must wait, + // not error out as a phantom miss. let pool = self.pool.clone(); let device_id = self.device_id; let chat = chat.to_string(); let sender = sender.to_string(); let msg_id = msg_id.to_string(); - tokio::task::spawn_blocking(move || -> Result>> { + self.with_semaphore(move || -> Result>> { let mut conn = pool .get() .map_err(|e| StoreError::Connection(Box::new(e)))?; @@ -2890,7 +2893,6 @@ impl MsgSecretStore for SqliteStore { Ok(row) }) .await - .map_err(|e| StoreError::Database(Box::new(e)))? } async fn get_msg_secret_with_ts( @@ -2899,12 +2901,16 @@ impl MsgSecretStore for SqliteStore { sender: &str, msg_id: &str, ) -> Result, i64)>> { + // Serialized through the db semaphore: a raw read racing a write + // transaction hits the shared-cache table lock on in-memory stores + // (SQLITE_LOCKED is not covered by busy_timeout) and callers treat the + // error as a missing secret. let pool = self.pool.clone(); let device_id = self.device_id; let chat = chat.to_string(); let sender = sender.to_string(); let msg_id = msg_id.to_string(); - tokio::task::spawn_blocking(move || -> Result, i64)>> { + self.with_semaphore(move || -> Result, i64)>> { let mut conn = pool .get() .map_err(|e| StoreError::Connection(Box::new(e)))?; @@ -2920,7 +2926,6 @@ impl MsgSecretStore for SqliteStore { Ok(row) }) .await - .map_err(|e| StoreError::Database(Box::new(e)))? } async fn delete_expired_msg_secrets(&self, cutoff_timestamp: i64) -> Result {