From 2612da81c33116bf4f96b1df87ccd632422a2fae Mon Sep 17 00:00:00 2001 From: zarzet Date: Sat, 8 Aug 2026 00:01:00 +0700 Subject: [PATCH] fix(db): prevent concurrent startup locks --- lib/services/app_state_database.dart | 28 ++---- lib/services/history_database.dart | 21 +++-- .../library_collections_database.dart | 21 +++-- lib/services/library_database.dart | 21 +++-- lib/services/sqlite_helpers.dart | 57 +++++++++++- test/sqlite_helpers_test.dart | 91 +++++++++++++++++++ 6 files changed, 185 insertions(+), 54 deletions(-) create mode 100644 test/sqlite_helpers_test.dart diff --git a/lib/services/app_state_database.dart b/lib/services/app_state_database.dart index d16978c1..c7c24a88 100644 --- a/lib/services/app_state_database.dart +++ b/lib/services/app_state_database.dart @@ -1,9 +1,8 @@ import 'dart:convert'; -import 'package:path/path.dart'; -import 'package:path_provider/path_provider.dart'; import 'package:shared_preferences/shared_preferences.dart'; import 'package:sqflite/sqflite.dart'; +import 'package:spotiflac_android/services/sqlite_helpers.dart' as sqlite; import 'package:spotiflac_android/utils/logger.dart'; final _log = AppLogger('AppStateDb'); @@ -25,31 +24,20 @@ const _recentMigrationKey = 'app_state_migrated_recent_to_sqlite_v1'; class AppStateDatabase { static final AppStateDatabase instance = AppStateDatabase._init(); - static Database? _database; + static final sqlite.SingleFlightInitializer _database = + sqlite.SingleFlightInitializer(); final Future _prefs = SharedPreferences.getInstance(); AppStateDatabase._init(); - Future get database async { - if (_database != null) return _database!; - _database = await _initDb(); - return _database!; - } + Future get database => _database.getOrCreate(_initDb); - Future _initDb() async { - final dbPath = await getApplicationDocumentsDirectory(); - final path = join(dbPath.path, _dbFileName); - - _log.i('Initializing app state database at: $path'); - - return openDatabase( - path, + Future _initDb() { + return sqlite.openAppDatabase( + _dbFileName, version: _dbVersion, - onConfigure: (db) async { - await db.rawQuery('PRAGMA journal_mode = WAL'); - await db.execute('PRAGMA synchronous = NORMAL'); - }, + incrementalAutoVacuum: false, onCreate: _createDb, onUpgrade: _upgradeDb, ); diff --git a/lib/services/history_database.dart b/lib/services/history_database.dart index 99af6246..bb0ab487 100644 --- a/lib/services/history_database.dart +++ b/lib/services/history_database.dart @@ -67,19 +67,20 @@ class HistoryBatchLookupRequest { class HistoryDatabase { static const int schemaVersion = 10; static final HistoryDatabase instance = HistoryDatabase._init(); - static Database? _database; + static final sqlite.SingleFlightInitializer _database = + sqlite.SingleFlightInitializer(); HistoryDatabase._init(); - Future get database async { - if (_database != null) return _database!; - _database = await sqlite.openAppDatabase( - 'history.db', - version: schemaVersion, - onCreate: _createDB, - onUpgrade: _upgradeDB, + Future get database { + return _database.getOrCreate( + () => sqlite.openAppDatabase( + 'history.db', + version: schemaVersion, + onCreate: _createDB, + onUpgrade: _upgradeDB, + ), ); - return _database!; } Future _createDB(Database db, int version) async { @@ -895,7 +896,7 @@ class HistoryDatabase { Future close() async { final db = await database; await db.close(); - _database = null; + _database.reset(); } Future updateFilePath( diff --git a/lib/services/library_collections_database.dart b/lib/services/library_collections_database.dart index 62185833..060b37a9 100644 --- a/lib/services/library_collections_database.dart +++ b/lib/services/library_collections_database.dart @@ -60,22 +60,23 @@ class PlaylistPickerSummaryRow { class LibraryCollectionsDatabase { static final LibraryCollectionsDatabase instance = LibraryCollectionsDatabase._init(); - static Database? _database; + static final sqlite.SingleFlightInitializer _database = + sqlite.SingleFlightInitializer(); final Future _prefs = SharedPreferences.getInstance(); LibraryCollectionsDatabase._init(); - Future get database async { - if (_database != null) return _database!; - _database = await sqlite.openAppDatabase( - _dbFileName, - version: _dbVersion, - foreignKeys: true, - onCreate: _createDb, - onUpgrade: _upgradeDb, + Future get database { + return _database.getOrCreate( + () => sqlite.openAppDatabase( + _dbFileName, + version: _dbVersion, + foreignKeys: true, + onCreate: _createDb, + onUpgrade: _upgradeDb, + ), ); - return _database!; } Future _createDb(Database db, int version) async { diff --git a/lib/services/library_database.dart b/lib/services/library_database.dart index 5ed4e61c..6c09498e 100644 --- a/lib/services/library_database.dart +++ b/lib/services/library_database.dart @@ -18,20 +18,21 @@ class LibraryDatabase { static final LibraryDatabase instance = LibraryDatabase._init(); static const int schemaVersion = 9; static const int audioMetadataScanVersion = 1; - static Database? _database; + static final sqlite.SingleFlightInitializer _database = + sqlite.SingleFlightInitializer(); bool _historyAttached = false; LibraryDatabase._init(); - Future get database async { - if (_database != null) return _database!; - _database = await sqlite.openAppDatabase( - 'local_library.db', - version: schemaVersion, - onCreate: _createDB, - onUpgrade: _upgradeDB, + Future get database { + return _database.getOrCreate( + () => sqlite.openAppDatabase( + 'local_library.db', + version: schemaVersion, + onCreate: _createDB, + onUpgrade: _upgradeDB, + ), ); - return _database!; } Future _ensureHistoryAttached(Database db) async { @@ -1013,7 +1014,7 @@ class LibraryDatabase { Future close() async { final db = await database; await db.close(); - _database = null; + _database.reset(); _historyAttached = false; } diff --git a/lib/services/sqlite_helpers.dart b/lib/services/sqlite_helpers.dart index 2c5be566..39d6d7b8 100644 --- a/lib/services/sqlite_helpers.dart +++ b/lib/services/sqlite_helpers.dart @@ -6,6 +6,39 @@ import 'package:sqflite/sqflite.dart'; final _log = AppLogger('AppSqlite'); +/// Caches an asynchronously-created value while also coalescing concurrent +/// callers onto the same in-flight initialization. +class SingleFlightInitializer { + T? _value; + Future? _initializing; + + Future getOrCreate(Future Function() create) { + final value = _value; + if (value != null) return Future.value(value); + + final initializing = _initializing; + if (initializing != null) return initializing; + + late final Future future; + future = Future.sync(create) + .then((value) { + _value = value; + return value; + }) + .whenComplete(() { + if (identical(_initializing, future)) { + _initializing = null; + } + }); + _initializing = future; + return future; + } + + void reset() { + _value = null; + } +} + /// Opens a database file in the app documents directory with the shared /// WAL + synchronous=NORMAL configuration. Future openAppDatabase( @@ -15,6 +48,7 @@ Future openAppDatabase( required Future Function(Database db, int oldVersion, int newVersion) onUpgrade, bool foreignKeys = false, + bool incrementalAutoVacuum = true, }) async { final dbPath = await getApplicationDocumentsDirectory(); final path = join(dbPath.path, fileName); @@ -25,13 +59,28 @@ Future openAppDatabase( path, version: version, onConfigure: (db) async { + // Set this before any other PRAGMA so transient writer contention waits + // instead of immediately surfacing SQLITE_BUSY during startup. + await db.rawQuery('PRAGMA busy_timeout = 5000'); if (foreignKeys) { await db.execute('PRAGMA foreign_keys = ON'); } - // Without auto_vacuum the file never shrinks after deletes — its size - // is a permanent high-water mark. Only takes effect on newly created - // databases; existing files keep their mode until a manual VACUUM. - await db.execute('PRAGMA auto_vacuum = INCREMENTAL'); + if (incrementalAutoVacuum) { + final tables = await db.rawQuery(''' + SELECT 1 + FROM sqlite_master + WHERE type = 'table' + AND name NOT LIKE 'sqlite_%' + AND name != 'android_metadata' + LIMIT 1 + '''); + // Changing from NONE must happen outside a transaction and before the + // first application table is created. Android may already have added + // its internal android_metadata table at this point. + if (tables.isEmpty) { + await db.execute('PRAGMA auto_vacuum = INCREMENTAL'); + } + } await db.rawQuery('PRAGMA journal_mode = WAL'); await db.execute('PRAGMA synchronous = NORMAL'); }, diff --git a/test/sqlite_helpers_test.dart b/test/sqlite_helpers_test.dart new file mode 100644 index 00000000..f13d58dc --- /dev/null +++ b/test/sqlite_helpers_test.dart @@ -0,0 +1,91 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/sqlite_helpers.dart'; + +void main() { + group('SingleFlightInitializer', () { + test('coalesces concurrent initialization and caches the result', () async { + final initializer = SingleFlightInitializer(); + final gate = Completer(); + var calls = 0; + + Future create() async { + calls++; + await gate.future; + return 42; + } + + final first = initializer.getOrCreate(create); + final second = initializer.getOrCreate(create); + + expect(identical(first, second), isTrue); + expect(calls, 1); + + gate.complete(); + expect(await Future.wait([first, second]), [42, 42]); + expect(await initializer.getOrCreate(create), 42); + expect(calls, 1); + }); + + test('allows retry after initialization fails', () async { + final initializer = SingleFlightInitializer(); + var calls = 0; + + Future create() { + calls++; + if (calls == 1) throw StateError('first attempt failed'); + return Future.value(7); + } + + await expectLater(initializer.getOrCreate(create), throwsStateError); + expect(await initializer.getOrCreate(create), 7); + expect(calls, 2); + }); + + test('reset permits a fresh initialization', () async { + final initializer = SingleFlightInitializer(); + var value = 1; + + expect(await initializer.getOrCreate(() async => value), 1); + value = 2; + expect(await initializer.getOrCreate(() async => value), 1); + + initializer.reset(); + expect(await initializer.getOrCreate(() async => value), 2); + }); + }); + + group('SQLite startup configuration', () { + final source = File('lib/services/sqlite_helpers.dart').readAsStringSync(); + + test('waits for transient locks before configuring the connection', () { + final configureIndex = source.indexOf('onConfigure:'); + final busyTimeoutIndex = source.indexOf('PRAGMA busy_timeout'); + final journalModeIndex = source.indexOf('PRAGMA journal_mode'); + + expect(configureIndex, greaterThanOrEqualTo(0)); + expect(busyTimeoutIndex, greaterThan(configureIndex)); + expect(busyTimeoutIndex, lessThan(journalModeIndex)); + }); + + test('sets incremental auto-vacuum only before app tables exist', () { + final schemaCheckIndex = source.indexOf('FROM sqlite_master'); + final emptySchemaGuardIndex = source.indexOf('if (tables.isEmpty)'); + final autoVacuumMatches = RegExp( + r"PRAGMA auto_vacuum = INCREMENTAL", + ).allMatches(source).toList(); + final onCreateIndex = source.indexOf('onCreate:'); + + expect(autoVacuumMatches, hasLength(1)); + expect(schemaCheckIndex, greaterThanOrEqualTo(0)); + expect(emptySchemaGuardIndex, greaterThan(schemaCheckIndex)); + expect( + autoVacuumMatches.single.start, + greaterThan(emptySchemaGuardIndex), + ); + expect(autoVacuumMatches.single.start, lessThan(onCreateIndex)); + }); + }); +}