From 97935eb30d6197b7fb3e0f314ddf652d52999526 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Fri, 10 Jul 2026 13:43:02 -0500 Subject: [PATCH] fix(database): force Room single-connection pool on all platforms (#6202) Co-authored-by: Claude Fable 5 --- .../core/database/DatabaseBuilder.kt | 13 ++--- .../core/database/MeshtasticDatabase.kt | 48 +++++++------------ .../core/database/DatabaseBuilder.kt | 2 +- .../core/database/DatabaseBuilder.kt | 2 +- 4 files changed, 22 insertions(+), 43 deletions(-) diff --git a/core/database/src/androidMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt b/core/database/src/androidMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt index 2b10b7ae5..05cff77f1 100644 --- a/core/database/src/androidMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt +++ b/core/database/src/androidMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt @@ -32,13 +32,8 @@ import org.meshtastic.core.common.ContextServices import org.meshtastic.core.database.MeshtasticDatabase.Companion.configureCommon /** - * Returns a [RoomDatabase.Builder] configured for Android with the given [dbName]. - * - * Android production deliberately opts out of Room KMP's multi-reader connection pool (`multiConnection = false`). - * Under coroutine cancellation churn (e.g. DB switches via `flatMapLatest`), the reader-pool permit semaphore can - * wedge: all reader connections report `Free` but `permits=0`, so every new read acquisition times out indefinitely. - * Single-connection mode serializes reads and writes through one connection, eliminating the separate reader permit - * pool entirely. JVM/iOS may still use the pool; see [MeshtasticDatabase.configureCommon]. + * Returns a [RoomDatabase.Builder] configured for Android with the given [dbName]. All platforms use Room KMP's + * single-connection pool to avoid the reader-pool permit wedge; see [MeshtasticDatabase.configureCommon]. */ actual fun getDatabaseBuilder(dbName: String): RoomDatabase.Builder { val dbFile = ContextServices.app.getDatabasePath(dbName) @@ -46,14 +41,14 @@ actual fun getDatabaseBuilder(dbName: String): RoomDatabase.Builder = Room.inMemoryDatabaseBuilder(factory = { MeshtasticDatabaseConstructor.initialize() }) - .configureCommon(multiConnection = false) + .configureCommon() .setDriver(BundledSQLiteDriver()) /** Returns the Android directory where database files are stored. */ diff --git a/core/database/src/commonMain/kotlin/org/meshtastic/core/database/MeshtasticDatabase.kt b/core/database/src/commonMain/kotlin/org/meshtastic/core/database/MeshtasticDatabase.kt index dffa2b34b..73076e4e7 100644 --- a/core/database/src/commonMain/kotlin/org/meshtastic/core/database/MeshtasticDatabase.kt +++ b/core/database/src/commonMain/kotlin/org/meshtastic/core/database/MeshtasticDatabase.kt @@ -153,41 +153,25 @@ abstract class MeshtasticDatabase : RoomDatabase() { /** * Configures a [RoomDatabase.Builder] with standard settings for this project. * - * @param multiConnection when `true` (default), opens a multi-reader connection pool (`maxNumOfReaders = 4`, - * `maxNumOfWriters = 1`) so reads can run concurrently. Pass `false` to explicitly force - * [setSingleConnectionPool], serializing all reads and writes through one connection. + * All platforms force [setSingleConnectionPool]. Without it, Room defaults to a 4-reader pool for named + * databases, and under coroutine cancellation churn (e.g. DB switches via `flatMapLatest`) the reader-pool + * permit semaphore can wedge: all reader connections report `Free` but `permits=0`, so every read acquisition + * times out indefinitely ("Error code: 5, Timed out attempting to acquire a reader connection"). Android hit + * this first; desktop (Flathub, 2026-07-10) reproduced the identical wedge in field logs, so JVM/iOS were moved + * off the multi-reader pool too. Single-connection eliminates the separate reader permit pool entirely. * - * **Android production passes `false`.** Without the explicit `setSingleConnectionPool()` call, Room defaults - * to a 4-reader pool for named databases regardless of whether `setMultipleConnectionPool` was called. Under - * coroutine cancellation churn (e.g. DB switches via `flatMapLatest`), the reader-pool permit semaphore can - * wedge: all reader connections report `Free` but `permits=0`, so every read acquisition times out - * indefinitely. Forcing single-connection eliminates the separate reader permit pool entirely. - * - * **In-memory databases (tests) pass `false`.** Room already serves an in-memory database (`name == null`) from - * a single connection regardless of the pool configuration, so this only makes the single-connection intent - * explicit and serializes the query dispatcher; it is not load-bearing for read-after-write. - * (`DeviceLinkRepositoryImplTest` is deterministic because it runs on the wall clock, not because of this flag; - * see that test's comments.) - * - * **JVM/iOS production uses `true`** (the default). Revisit if desktop/iOS field logs show similar - * pool-exhaustion patterns under cancellation churn. + * For in-memory databases (tests) this is a no-op for pooling — Room already serves `name == null` databases + * from a single connection — it just serializes the query dispatcher. */ @OptIn(ExperimentalCoroutinesApi::class) - fun RoomDatabase.Builder.configureCommon( - multiConnection: Boolean = true, - ): RoomDatabase.Builder = this.fallbackToDestructiveMigration(dropAllTables = false) - .apply { - if (multiConnection) { - setMultipleConnectionPool(maxNumOfReaders = 4, maxNumOfWriters = 1) - } else { - setSingleConnectionPool() - } - } - .setQueryCoroutineContext( - // limitedParallelism(1) has the same throughput ceiling as the single-connection pool - // (already serialized), so this only blocks the cancellation pileup — not real I/O concurrency. - if (multiConnection) ioDispatcher else ioDispatcher.limitedParallelism(1), - ) + fun RoomDatabase.Builder.configureCommon(): RoomDatabase.Builder = + this.fallbackToDestructiveMigration(dropAllTables = false) + .setSingleConnectionPool() + .setQueryCoroutineContext( + // limitedParallelism(1) has the same throughput ceiling as the single-connection pool + // (already serialized), so this only blocks the cancellation pileup — not real I/O concurrency. + ioDispatcher.limitedParallelism(1), + ) } } diff --git a/core/database/src/iosMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt b/core/database/src/iosMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt index 6cecf57f1..459c024ea 100644 --- a/core/database/src/iosMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt +++ b/core/database/src/iosMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt @@ -51,7 +51,7 @@ actual fun getDatabaseBuilder(dbName: String): RoomDatabase.Builder = Room.inMemoryDatabaseBuilder(factory = { MeshtasticDatabaseConstructor.initialize() }) - .configureCommon(multiConnection = false) + .configureCommon() .setDriver(BundledSQLiteDriver()) /** Returns the iOS directory where database files are stored. */ diff --git a/core/database/src/jvmMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt b/core/database/src/jvmMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt index 50e84898d..9686529d7 100644 --- a/core/database/src/jvmMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt +++ b/core/database/src/jvmMain/kotlin/org/meshtastic/core/database/DatabaseBuilder.kt @@ -57,7 +57,7 @@ actual fun getDatabaseBuilder(dbName: String): RoomDatabase.Builder = Room.inMemoryDatabaseBuilder(factory = { MeshtasticDatabaseConstructor.initialize() }) - .configureCommon(multiConnection = false) + .configureCommon() .setDriver(BundledSQLiteDriver()) /** Returns the JVM/Desktop directory where database files are stored. */