diff --git a/src/ai/handlers.rs b/src/ai/handlers.rs index 2be4e85..b53d05e 100644 --- a/src/ai/handlers.rs +++ b/src/ai/handlers.rs @@ -2129,15 +2129,7 @@ mod turn_replay_tests { use actix_web::{App, web::Data}; use std::sync::Arc; - /// Serialize `AppState::test_state()` construction across the parallel - /// tests in this module: each build opens ~10 DAO connections to the one - /// shared `DATABASE_URL` file, and doing several at once races the WAL - /// `journal_mode` switch into a spurious "database is locked". The test - /// bodies themselves still run in parallel; only the open is gated. - static DB_INIT: std::sync::Mutex<()> = std::sync::Mutex::new(()); - fn build_state() -> Data { - let _guard = DB_INIT.lock().unwrap_or_else(|p| p.into_inner()); Data::new(AppState::test_state()) } diff --git a/src/database/mod.rs b/src/database/mod.rs index 3fe7468..afa62f0 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -167,7 +167,11 @@ impl UserDao for SqliteUserDao { pub fn connect() -> SqliteConnection { let db_url = dotenv::var("DATABASE_URL").expect("DATABASE_URL must be set"); - let mut conn = SqliteConnection::establish(&db_url).expect("Error connecting to DB"); + connect_to(&db_url) +} + +fn connect_to(db_url: &str) -> SqliteConnection { + let mut conn = SqliteConnection::establish(db_url).expect("Error connecting to DB"); // Each DAO opens its own connection (13+ across the app) and they all // share one DB file. Without WAL, a writer holds an exclusive lock // that blocks readers — `load_persons` racing the face-watch write @@ -186,13 +190,33 @@ pub fn connect() -> SqliteConnection { // automatically and code that previously did manual two-step // cleanup (delete child rows, then parent) becomes redundant but // still correct. - conn.batch_execute( - "PRAGMA journal_mode = WAL; \ - PRAGMA busy_timeout = 5000; \ - PRAGMA synchronous = NORMAL; \ - PRAGMA foreign_keys = ON;", - ) - .expect("set sqlite pragmas"); + // + // busy_timeout goes first so the WAL switch waits out ordinary + // contention. That isn't enough on its own: while the file is still in + // rollback mode, a connection that holds a shared lock and needs to + // upgrade gets SQLITE_BUSY immediately (deadlock avoidance skips the + // busy handler), so several connections opening a fresh file at once + // still fail with "database is locked" (#157). Retry for the same 5 s + // budget; once one connection has switched the file to WAL (persistent), + // the pragma is a no-op for the rest. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + loop { + match conn.batch_execute( + "PRAGMA busy_timeout = 5000; \ + PRAGMA journal_mode = WAL; \ + PRAGMA synchronous = NORMAL; \ + PRAGMA foreign_keys = ON;", + ) { + Ok(()) => break, + Err(e) + if e.to_string().contains("database is locked") + && std::time::Instant::now() < deadline => + { + std::thread::sleep(std::time::Duration::from_millis(10)); + } + Err(e) => panic!("set sqlite pragmas: {e:?}"), + } + } conn } @@ -2564,6 +2588,37 @@ impl ExifDao for SqliteExifDao { } } +#[cfg(test)] +mod connect_tests { + use super::connect_to; + + /// Many connections opening one fresh DB file at once (what parallel + /// `AppState::test_state()` builds do) must all get their pragmas set; + /// the WAL switch has to wait out contention, not fail with + /// "database is locked" (#157). + #[test] + fn concurrent_connects_to_fresh_file_all_succeed() { + for _ in 0..20 { + let dir = tempfile::tempdir().unwrap(); + let url = dir.path().join("t.db").to_string_lossy().into_owned(); + let barrier = std::sync::Arc::new(std::sync::Barrier::new(16)); + let handles: Vec<_> = (0..16) + .map(|_| { + let url = url.clone(); + let barrier = barrier.clone(); + std::thread::spawn(move || { + barrier.wait(); + connect_to(&url); + }) + }) + .collect(); + for h in handles { + h.join().expect("connect panicked"); + } + } + } +} + #[cfg(test)] mod exif_dao_tests { use super::*;