From 7a628052ba9f9312f177abc558142bb938d95f33 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Sat, 11 Jul 2026 09:29:56 -0400 Subject: [PATCH 1/2] fix(auth): prevent DB connection-pool-closed crash on concurrent login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cold start with degraded network could race two soft-logins: app startup (FlipcashApp.init) and an FCM push handler (NotificationService) both call AuthManager.init() when no account cluster is established yet. The second login re-runs FlipcashDatabase.init(), which unconditionally closed and rebuilt the per-user database — tearing down the connection pool out from under an active Room query (e.g. CashScreenViewModel's balance Flow) and throwing: IllegalStateException: Cannot perform this operation because the connection pool has been closed. Fixes: - FlipcashDatabase.init() is now @Synchronized and idempotent: it early-returns when an instance for the same DB already exists. Keyed on the DB name (not isOpen(), which is false while Room lazily opens the file — exactly the race window). Account-switch still rebuilds. - AuthManager.init() is serialized with a Mutex and short-circuits when already LoggedIn, so a second concurrent soft-login can't churn the DB lifecycle. Adds FlipcashDatabaseInitTest and AuthManager init-guard tests. --- .../com/flipcash/app/auth/AuthManager.kt | 40 +++++++++---- .../com/flipcash/app/auth/AuthManagerTest.kt | 32 +++++++++++ .../app/persistence/FlipcashDatabase.kt | 15 ++++- .../persistence/FlipcashDatabaseInitTest.kt | 57 +++++++++++++++++++ 4 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/FlipcashDatabaseInitTest.kt diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt index e9af88d80..a277dcf7d 100644 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt @@ -32,6 +32,8 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import javax.inject.Inject import javax.inject.Singleton @@ -73,6 +75,14 @@ class AuthManager @Inject constructor( private var softLoginDisabled: Boolean = false + /** + * Serializes [init] so concurrent callers (app startup and the FCM push + * handler both call it when no account is established) can't launch two + * soft-logins at once — a second login re-inits the per-user database and + * would close the connection pool out from under active Room queries. + */ + private val initMutex = Mutex() + /** * Entropy for the account being switched to. Set before logout so App.kt's * auth guard can navigate to Login(entropy) instead of seedless Login(). @@ -93,18 +103,28 @@ class AuthManager @Inject constructor( fun init(onInitialized: () -> Unit = { }) { launch { - when (val result = credentialManager.lookup() - .also { taggedTrace("lookup result: ${it::class.simpleName}") }) { - is LookupResult.ExistingAccountFound -> { - val token = result.entropy - softLogin(token) - .onSuccess { onInitialized() } + initMutex.withLock { + // A concurrent init already logged us in (e.g. app startup won the + // race against an FCM push). Don't run a second soft-login that + // would re-init the database and close its connection pool mid-query. + if (userManager.authState is AuthState.LoggedIn) { + onInitialized() + return@withLock } - LookupResult.NoAccountFound -> Unit - is LookupResult.TemporaryAccountCreated -> { - userManager.establish(entropy = result.entropy) - userManager.set(AuthState.Onboarding(result.resumePoint)) + when (val result = credentialManager.lookup() + .also { taggedTrace("lookup result: ${it::class.simpleName}") }) { + is LookupResult.ExistingAccountFound -> { + val token = result.entropy + softLogin(token) + .onSuccess { onInitialized() } + } + + LookupResult.NoAccountFound -> Unit + is LookupResult.TemporaryAccountCreated -> { + userManager.establish(entropy = result.entropy) + userManager.set(AuthState.Onboarding(result.resumePoint)) + } } } } diff --git a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt index d7411d366..30fa3e6a9 100644 --- a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt @@ -3,6 +3,7 @@ package com.flipcash.app.auth import androidx.core.app.NotificationManagerCompat import com.flipcash.app.appsettings.AppSettingsCoordinator import com.flipcash.app.auth.internal.credentials.AccountMetadata +import com.flipcash.app.auth.internal.credentials.LookupResult import com.flipcash.app.auth.internal.credentials.PassphraseCredentialManager import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.featureflags.FeatureFlagController @@ -372,4 +373,35 @@ class AuthManagerTest { assertTrue(result.isSuccess) verify(exactly = 0) { userManager.set(AuthState.Ready) } } + + @Test + fun `init skips soft-login when already logged in`() = runTest { + // Simulates the FCM push handler calling init after app startup already + // authenticated: a second soft-login would re-init the DB and could close + // its connection pool out from under active Room queries. + every { userManager.authState } returns AuthState.Ready + + var initializedCalled = false + authManager.init { initializedCalled = true } + + assertTrue(initializedCalled) + coVerify(exactly = 0) { credentialManager.lookup() } + verify(exactly = 0) { persistence.openDatabase(any()) } + } + + @Test + fun `init soft-logs-in when no account is established`() = runTest { + val entropy = "dGVzdGVudHJvcHkxMjM0NQ==" + val accountMetadata: AccountMetadata = mockk(relaxed = true) + every { accountMetadata.id } returns listOf(1, 2, 3) + + every { userManager.authState } returns AuthState.Unknown + coEvery { credentialManager.lookup() } returns LookupResult.ExistingAccountFound(entropy) + coEvery { credentialManager.login(entropy, any()) } returns Result.success(accountMetadata) + + authManager.init() + + coVerify { credentialManager.lookup() } + verify { persistence.openDatabase(entropy) } + } } diff --git a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/FlipcashDatabase.kt b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/FlipcashDatabase.kt index 028c7ba48..1beb27ebd 100644 --- a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/FlipcashDatabase.kt +++ b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/FlipcashDatabase.kt @@ -167,11 +167,24 @@ abstract class FlipcashDatabase : RoomDatabase() { fun isOpen() = instance?.isOpen == true + @Synchronized fun init(context: Context, entropyB64: String) { val dbUniqueName = Base58.encode(entropyB64.toByteArray().subByteArray(0, 6)) + val targetName = "$dbNamePrefix-$dbUniqueName.db" + + // Same-user DB already built: don't tear down the existing instance. + // A concurrent or soft re-login (e.g. app startup racing an FCM push + // that also calls AuthManager.init) must not close the DB out from + // under in-flight Room queries such as CashScreenViewModel's balance + // Flow — doing so throws "connection pool has been closed". Key on the + // DB name, not isOpen(): Room opens the file lazily, so the instance + // can be live-but-not-yet-open in exactly this racy window. + // closeDb() nulls `instance`, so a non-null instance here is current. + if (instance != null && dbName == targetName) return + trace("database init start $dbUniqueName", type = TraceType.Process) instance?.close() - dbName = "$dbNamePrefix-$dbUniqueName.db" + dbName = targetName instance = Room.databaseBuilder(context, FlipcashDatabase::class.java, dbName) diff --git a/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/FlipcashDatabaseInitTest.kt b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/FlipcashDatabaseInitTest.kt new file mode 100644 index 000000000..ff8804718 --- /dev/null +++ b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/FlipcashDatabaseInitTest.kt @@ -0,0 +1,57 @@ +package com.flipcash.app.persistence + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertNotSame +import kotlin.test.assertSame +import kotlin.test.assertTrue + +@RunWith(RobolectricTestRunner::class) +class FlipcashDatabaseInitTest { + + private val context = ApplicationProvider.getApplicationContext() + + @After + fun tearDown() { + FlipcashDatabase.closeDb() + } + + @Test + fun `re-init with same entropy keeps the same open instance`() { + FlipcashDatabase.init(context, ENTROPY_A) + val first = FlipcashDatabase.requireInstance() + // Force the (lazily-opened) connection pool open, as an active Room query would. + first.openHelper.writableDatabase + assertTrue(first.isOpen) + + // A soft re-login (or a concurrent AuthManager.init) must not tear down + // the live connection pool that active Room queries depend on. + FlipcashDatabase.init(context, ENTROPY_A) + val second = FlipcashDatabase.requireInstance() + + assertSame(first, second) + assertTrue(first.isOpen) + } + + @Test + fun `init with different entropy rebuilds the database`() { + FlipcashDatabase.init(context, ENTROPY_A) + val first = FlipcashDatabase.requireInstance() + + FlipcashDatabase.init(context, ENTROPY_B) + val second = FlipcashDatabase.requireInstance() + + assertNotSame(first, second) + second.openHelper.writableDatabase + assertTrue(second.isOpen) + } + + private companion object { + const val ENTROPY_A = "aaaaaaaaaaaaaaaaaaaaaaaa" + const val ENTROPY_B = "bbbbbbbbbbbbbbbbbbbbbbbb" + } +} From 4a621f69d402f47efb913cf7acc93f6a7ff7517b Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Sat, 11 Jul 2026 21:58:46 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(auth):=20don't=20close=20the=20DB=20on?= =?UTF-8?q?=20logout=20=E2=80=94=20fixes=20SQLITE=5FMISUSE=20crash?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second connection-closed crash was reported (SQLException code 21 "connection is closed") from CashScreenViewModel's Room flow — same family as the connection-pool crash this branch fixes, but a different trigger: logout instead of login re-init. On logout, resetStateForUser() called persistence.close() while the just-dismissed Cash screen's stateIn(WhileSubscribed(5000)) flows were still active (up to 5s), so an in-flight Room query hit the closed connection and threw. Now that FlipcashDatabase.init() is idempotent + @Synchronized, it owns the DB lifecycle end to end, so: - resetStateForUser() no longer closes the DB. A same-user re-login reuses the open instance; a different user's login rebuilds it. - PersistenceProvider.openDatabase() no longer short-circuits on isOpen(): with the DB now left open after logout, that guard would hand a different user the previous user's database. It always delegates to the (idempotent, name-keyed) init() instead. Adds PersistenceProviderTest covering same-user reuse and different-user swap-while-open (account isolation). Updates AuthManagerTest to assert the DB is not closed on logout. --- .../com/flipcash/app/auth/AuthManager.kt | 7 ++- .../com/flipcash/app/auth/AuthManagerTest.kt | 8 ++- .../persistence/provider/build.gradle.kts | 4 ++ .../app/persistence/PersistenceProvider.kt | 7 ++- .../persistence/PersistenceProviderTest.kt | 51 +++++++++++++++++++ 5 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 apps/flipcash/shared/persistence/provider/src/test/kotlin/com/flipcash/app/persistence/PersistenceProviderTest.kt diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt index a277dcf7d..f5c1722c7 100644 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt @@ -315,7 +315,12 @@ class AuthManager @Inject constructor( notificationManager.cancelAll() userManager.clear() tokenCoordinator.reset() - persistence.close() + // Intentionally do NOT close the Room DB here. Closing it while the just- + // dismissed Cash screen's stateIn(WhileSubscribed(5000)) flows are still + // active tears the connection pool out from under an in-flight Room query + // -> "SQLException: connection is closed" (SQLITE_MISUSE). The DB lifecycle + // is owned by FlipcashDatabase.init(): a same-user re-login reuses the open + // instance, and a different user's login rebuilds it for their entropy. featureFlags.reset() appSettings.reset() userFlags.clearAll() diff --git a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt index 30fa3e6a9..9668edcef 100644 --- a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt @@ -147,7 +147,10 @@ class AuthManagerTest { verify { userManager.clear() } verify { notificationManager.cancelAll() } coVerify { tokenCoordinator.reset() } - verify { persistence.close() } + // The DB is intentionally left open on logout/reset — closing it races + // still-active Cash screen Room flows. FlipcashDatabase.init() owns the + // lifecycle (reuse on same-user re-login, rebuild for a different user). + verify(exactly = 0) { persistence.close() } verify { featureFlagController.reset() } verify { appSettings.reset() } verify { userFlags.clearAll() } @@ -207,7 +210,8 @@ class AuthManagerTest { verify { userManager.clear() } verify { notificationManager.cancelAll() } coVerify { tokenCoordinator.reset() } - verify { persistence.close() } + // DB stays open on logout — see resetStateForUser; init() owns lifecycle. + verify(exactly = 0) { persistence.close() } verify { featureFlagController.reset() } verify { appSettings.reset() } verify { userFlags.clearAll() } diff --git a/apps/flipcash/shared/persistence/provider/build.gradle.kts b/apps/flipcash/shared/persistence/provider/build.gradle.kts index fa7ef1d75..7f3b34024 100644 --- a/apps/flipcash/shared/persistence/provider/build.gradle.kts +++ b/apps/flipcash/shared/persistence/provider/build.gradle.kts @@ -15,4 +15,8 @@ android { dependencies { implementation(project(":apps:flipcash:shared:persistence:db")) + + testImplementation(kotlin("test")) + testImplementation(libs.bundles.unit.testing) + testImplementation(libs.robolectric) } diff --git a/apps/flipcash/shared/persistence/provider/src/main/kotlin/com/flipcash/app/persistence/PersistenceProvider.kt b/apps/flipcash/shared/persistence/provider/src/main/kotlin/com/flipcash/app/persistence/PersistenceProvider.kt index 4e9145489..19f582f47 100644 --- a/apps/flipcash/shared/persistence/provider/src/main/kotlin/com/flipcash/app/persistence/PersistenceProvider.kt +++ b/apps/flipcash/shared/persistence/provider/src/main/kotlin/com/flipcash/app/persistence/PersistenceProvider.kt @@ -10,7 +10,12 @@ class PersistenceProvider @Inject constructor( @ApplicationContext private val context: Context ) { fun openDatabase(entropy: String) { - if (FlipcashDatabase.isOpen()) return + // Always delegate to init(): it is idempotent (no-op when this user's DB + // is already open) and rebuilds when the entropy/DB name differs. We must + // NOT short-circuit on isOpen() — after logout we intentionally leave the + // DB open (see AuthManager.resetStateForUser), so a *different* user + // signing in has to reach init() to swap to their own database. Guarding + // on isOpen() here would hand the new user the previous user's data. FlipcashDatabase.init(context, entropy) } diff --git a/apps/flipcash/shared/persistence/provider/src/test/kotlin/com/flipcash/app/persistence/PersistenceProviderTest.kt b/apps/flipcash/shared/persistence/provider/src/test/kotlin/com/flipcash/app/persistence/PersistenceProviderTest.kt new file mode 100644 index 000000000..f20117aed --- /dev/null +++ b/apps/flipcash/shared/persistence/provider/src/test/kotlin/com/flipcash/app/persistence/PersistenceProviderTest.kt @@ -0,0 +1,51 @@ +package com.flipcash.app.persistence + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertNotSame +import kotlin.test.assertSame + +@RunWith(RobolectricTestRunner::class) +class PersistenceProviderTest { + + private val context = ApplicationProvider.getApplicationContext() + private val provider = PersistenceProvider(context) + + @After + fun tearDown() { + FlipcashDatabase.closeDb() + } + + @Test + fun `openDatabase for the same user reuses the open instance`() { + provider.openDatabase(ENTROPY_A) + val first = FlipcashDatabase.requireInstance() + + provider.openDatabase(ENTROPY_A) + + assertSame(first, FlipcashDatabase.requireInstance()) + } + + @Test + fun `openDatabase for a different user swaps the DB even while one is open`() { + // Regression: after logout the DB is intentionally left open, so + // openDatabase must NOT short-circuit on isOpen() — a different user + // signing in has to reach init() and get their own database, not the + // previous user's. + provider.openDatabase(ENTROPY_A) + val first = FlipcashDatabase.requireInstance() + + provider.openDatabase(ENTROPY_B) + + assertNotSame(first, FlipcashDatabase.requireInstance()) + } + + private companion object { + const val ENTROPY_A = "aaaaaaaaaaaaaaaaaaaaaaaa" + const val ENTROPY_B = "bbbbbbbbbbbbbbbbbbbbbbbb" + } +}