From 916fc4bc6bf9ad5d9a98b605169af8c463133ac1 Mon Sep 17 00:00:00 2001 From: Thomas SharedInbox Date: Sat, 6 Jun 2026 23:37:03 +0200 Subject: [PATCH] fix: swallow SQLITE_BUSY when setting WAL mode to prevent crash on startup (#508) A WorkManager background task may have the database open when the foreground app starts. Executing PRAGMA journal_mode = WAL on the second connection then fails with SQLITE_BUSY_SNAPSHOT (extended code 261, primary code 5), crashing the app before it renders. Two changes: 1. Move PRAGMA busy_timeout = 5000 before the WAL pragma so SQLite auto-retries plain SQLITE_BUSY (code 5) for up to 5 s. 2. Extract setup logic into _setupPragmas and catch SqliteException with resultCode == 5 (covers both SQLITE_BUSY and SQLITE_BUSY_SNAPSHOT). SQLITE_BUSY_SNAPSHOT only occurs when the DB is already in WAL mode, so the pragma is a no-op and it is safe to continue. Adds a regression test that opens a second connection while a read transaction holds a WAL snapshot open and verifies setupPragmasForTesting does not throw. Co-Authored-By: Claude Sonnet 4.6 --- lib/data/db/database.dart | 37 +++++++++++++++++++++++++---------- test/unit/migration_test.dart | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/lib/data/db/database.dart b/lib/data/db/database.dart index 103df36..93d3939 100644 --- a/lib/data/db/database.dart +++ b/lib/data/db/database.dart @@ -7,6 +7,7 @@ import 'package:flutter/services.dart'; import 'package:path/path.dart' as p; import 'package:path_provider/path_provider.dart'; import 'package:sharedinbox/core/db_schema_version.dart'; +import 'package:sqlite3/sqlite3.dart' show Database; part 'database.g.dart'; @@ -793,18 +794,34 @@ Future resolveDatabasePathForTesting() => _resolveDatabasePath(); void resetDatabasePathForTesting() => _dbPath = null; Future androidFallbackPathForTesting() => _androidFallbackPath(); +/// Configures PRAGMAs on a newly opened SQLite connection. +/// +/// busy_timeout must come first so subsequent statements retry on SQLITE_BUSY +/// instead of immediately failing. +/// +/// journal_mode = WAL is wrapped in a try/catch because a concurrent +/// WorkManager background task may already have the DB open when the app +/// starts. SQLITE_BUSY_SNAPSHOT (extended code 261, primary code 5) is +/// returned in that situation; it only occurs when the DB is already in WAL +/// mode, so the pragma would be a no-op anyway and it is safe to continue. +void _setupPragmas(Database db) { + db.execute('PRAGMA busy_timeout = 5000;'); + try { + db.execute('PRAGMA journal_mode = WAL;'); + } on SqliteException catch (e) { + // resultCode strips the extended bits: both SQLITE_BUSY (5) and + // SQLITE_BUSY_SNAPSHOT (261) reduce to 5. Re-throw anything else. + if (e.resultCode != 5) rethrow; + } +} + LazyDatabase _openConnection() { return LazyDatabase(() async { final file = File(await _resolveDatabasePath()); - return NativeDatabase.createInBackground( - file, - setup: (db) { - // WAL lets readers and writers proceed concurrently (different account - // sync loops share the same DB). busy_timeout makes SQLite retry for - // up to 5 s instead of immediately returning SQLITE_BUSY. - db.execute('PRAGMA journal_mode = WAL;'); - db.execute('PRAGMA busy_timeout = 5000;'); - }, - ); + return NativeDatabase.createInBackground(file, setup: _setupPragmas); }); } + +// Exposed so tests can run the exact production setup logic on a raw +// sqlite3 connection (same pattern as resolveDatabasePathForTesting). +void setupPragmasForTesting(Database db) => _setupPragmas(db); diff --git a/test/unit/migration_test.dart b/test/unit/migration_test.dart index e6e375f..a807456 100644 --- a/test/unit/migration_test.dart +++ b/test/unit/migration_test.dart @@ -510,4 +510,40 @@ void main() { await db.close(); }); }); + + // Regression test for https://codeberg.org/guettli/sharedinbox/issues/508: + // _openConnection's setup callback must not crash when PRAGMA journal_mode = + // WAL fails with SQLITE_BUSY_SNAPSHOT (extended code 261, primary code 5) + // because a WorkManager background task already has the DB open in WAL mode. + group('WAL setup (#508)', () { + test( + 'setupPragmasForTesting does not throw when WAL is already active and ' + 'another connection holds an open read transaction', + () { + final dbFile = File('test_wal_busy_508.db'); + if (dbFile.existsSync()) dbFile.deleteSync(); + addTearDown(() { + if (dbFile.existsSync()) dbFile.deleteSync(); + }); + + // conn1: enable WAL and keep a read transaction open — simulates a + // WorkManager background task that opened the DB before the foreground + // app starts. + final conn1 = sqlite.sqlite3.open(dbFile.path); + conn1.execute('PRAGMA journal_mode = WAL;'); + conn1.execute('BEGIN;'); + conn1.select('SELECT 1;'); + + // conn2: run the exact production setup through setupPragmasForTesting. + // This must not throw even though conn1 holds an open transaction and + // the DB is already in WAL mode. + final conn2 = sqlite.sqlite3.open(dbFile.path); + expect(() => setupPragmasForTesting(conn2), returnsNormally); + + conn1.execute('ROLLBACK;'); + conn1.dispose(); + conn2.dispose(); + }, + ); + }); }