From 7b5819021a16ab8a734bbd63879bc01007c8292c Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Fri, 3 Jul 2026 10:54:48 -0500 Subject: [PATCH] fix(sync): gate PruneWorker & BackfillWorker on the encrypted-cache lock PruneWorker and BackfillWorker were the only two pre-auth background DB entry points that opened the database without first checking EncryptedCacheGuard.isCacheLocked(). With encryptCache + appLock both on and a headless cold start where the user hasn't authenticated (WorkManager after reboot, or the periodic backfill/prune window while locked), the first DAO call runs prepareCache() -> resolvePassphrase() -> session.await(), parking the worker thread until unlock and serializing other DB openers behind the held prepareCache mutex. Self-heals on unlock, but wastes wakelock/battery and makes zero progress while locked. Switch both workers' MailPruner/MailBackfiller injection to dagger.Lazy and add the isCacheLocked() guard before resolving it, mirroring SyncWorker/SendWorker. Add JVM regression tests (locked -> retry with the Lazy dep never resolved; unlocked -> runs; failure -> retry). Closes #224 Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/data/sync/BackfillWorker.kt | 32 ++++++---- .../org/libremail/data/sync/PruneWorker.kt | 21 +++++-- .../libremail/data/sync/BackfillWorkerTest.kt | 57 ++++++++++++++++++ .../libremail/data/sync/PruneWorkerTest.kt | 58 +++++++++++++++++++ 4 files changed, 153 insertions(+), 15 deletions(-) create mode 100644 app/src/test/kotlin/org/libremail/data/sync/BackfillWorkerTest.kt create mode 100644 app/src/test/kotlin/org/libremail/data/sync/PruneWorkerTest.kt diff --git a/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt index 6d44e14..fd4ae7c 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt @@ -5,9 +5,11 @@ import android.content.Context import androidx.hilt.work.HiltWorker import androidx.work.CoroutineWorker import androidx.work.WorkerParameters +import dagger.Lazy import dagger.assisted.Assisted import dagger.assisted.AssistedInject import kotlinx.coroutines.CancellationException +import org.libremail.data.security.EncryptedCacheGuard /** * Runs one bounded slice of the full-history backfill (issue #12). Cancellable (WorkManager stops it @@ -19,16 +21,26 @@ import kotlinx.coroutines.CancellationException class BackfillWorker @AssistedInject constructor( @Assisted appContext: Context, @Assisted workerParams: WorkerParameters, - private val backfiller: MailBackfiller, + // Lazy: resolving MailBackfiller builds the Room DB graph, whose first query blocks while the + // encrypted cache is locked. Resolve it only after the cache-lock check passes, so a locked run + // fails fast instead of parking this thread on an unsatisfiable passphrase await (mirrors SyncWorker). + private val backfiller: Lazy, + private val cacheGuard: EncryptedCacheGuard, ) : CoroutineWorker(appContext, workerParams) { - override suspend fun doWork(): Result = runCatching { - // Chain bounded slices back-to-back while history remains, so a large mailbox isn't limited to - // one slice per periodic run. runBackfill() returns true while any folder still has pages left; - // isStopped lets WorkManager end a long run gracefully (the periodic schedule resumes it). - while (backfiller.runBackfill() && !isStopped) { /* page the next slice */ } - }.fold( - onSuccess = { Result.success() }, - onFailure = { error -> if (error is CancellationException) throw error else Result.retry() }, - ) + override suspend fun doWork(): Result { + // Can't open the encrypted DB without the user present — retry later rather than parking a + // WorkManager thread (which also wedges the shared serial executor) on an unsatisfiable await. + if (cacheGuard.isCacheLocked()) return Result.retry() + return runCatching { + // Chain bounded slices back-to-back while history remains, so a large mailbox isn't limited + // to one slice per periodic run. runBackfill() returns true while any folder still has pages + // left; isStopped lets WorkManager end a long run gracefully (the periodic schedule resumes). + val mailBackfiller = backfiller.get() + while (mailBackfiller.runBackfill() && !isStopped) { /* page the next slice */ } + }.fold( + onSuccess = { Result.success() }, + onFailure = { error -> if (error is CancellationException) throw error else Result.retry() }, + ) + } } diff --git a/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt index a6dc3ba..a0f8c86 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt @@ -5,8 +5,10 @@ import android.content.Context import androidx.hilt.work.HiltWorker import androidx.work.CoroutineWorker import androidx.work.WorkerParameters +import dagger.Lazy import dagger.assisted.Assisted import dagger.assisted.AssistedInject +import org.libremail.data.security.EncryptedCacheGuard /** * Enforces device-only retention (issue #13) by running [MailPruner]. Purely local — it never @@ -16,11 +18,20 @@ import dagger.assisted.AssistedInject class PruneWorker @AssistedInject constructor( @Assisted appContext: Context, @Assisted workerParams: WorkerParameters, - private val pruner: MailPruner, + // Lazy: resolving MailPruner builds the Room DB graph, whose first query blocks while the encrypted + // cache is locked. Resolve it only after the cache-lock check passes, so a locked run fails fast + // instead of parking this thread on an unsatisfiable passphrase await (mirrors SyncWorker/SendWorker). + private val pruner: Lazy, + private val cacheGuard: EncryptedCacheGuard, ) : CoroutineWorker(appContext, workerParams) { - override suspend fun doWork(): Result = runCatching { pruner.prune() }.fold( - onSuccess = { Result.success() }, - onFailure = { Result.retry() }, - ) + override suspend fun doWork(): Result { + // Can't open the encrypted DB without the user present — retry later rather than parking a + // WorkManager thread (which also wedges the shared serial executor) on an unsatisfiable await. + if (cacheGuard.isCacheLocked()) return Result.retry() + return runCatching { pruner.get().prune() }.fold( + onSuccess = { Result.success() }, + onFailure = { Result.retry() }, + ) + } } diff --git a/app/src/test/kotlin/org/libremail/data/sync/BackfillWorkerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/BackfillWorkerTest.kt new file mode 100644 index 0000000..b45ed6e --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/BackfillWorkerTest.kt @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import androidx.work.ListenableWorker.Result +import dagger.Lazy +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.libremail.data.security.EncryptedCacheGuard +import kotlin.test.assertEquals + +/** + * [BackfillWorker] must defer while the encrypted cache is locked rather than park this WorkManager + * thread opening the DB. It gates on [EncryptedCacheGuard], resolving the (`Lazy`) [MailBackfiller] + * only once unlocked — the same pre-auth invariant `SyncWorker`/`SendWorker` enforce. + */ +class BackfillWorkerTest { + + private val backfiller = mockk() + private val lazyBackfiller = mockk> { every { get() } returns backfiller } + private val cacheGuard = mockk() + + private fun worker() = BackfillWorker(mockk(relaxed = true), mockk(relaxed = true), lazyBackfiller, cacheGuard) + + @Test + fun `retries without resolving the backfiller when the cache is locked`() = runTest { + coEvery { cacheGuard.isCacheLocked() } returns true + + assertEquals(Result.retry(), worker().doWork()) + + verify(exactly = 0) { lazyBackfiller.get() } + coVerify(exactly = 0) { backfiller.runBackfill(any()) } + } + + @Test + fun `chains slices to completion and succeeds when the cache is unlocked`() = runTest { + coEvery { cacheGuard.isCacheLocked() } returns false + // true then false: one slice still has pages, the next reports done — the worker loops until false. + coEvery { backfiller.runBackfill(any()) } returnsMany listOf(true, false) + + assertEquals(Result.success(), worker().doWork()) + + coVerify(exactly = 2) { backfiller.runBackfill(any()) } + } + + @Test + fun `retries when backfilling throws`() = runTest { + coEvery { cacheGuard.isCacheLocked() } returns false + coEvery { backfiller.runBackfill(any()) } throws IllegalStateException("boom") + + assertEquals(Result.retry(), worker().doWork()) + } +} diff --git a/app/src/test/kotlin/org/libremail/data/sync/PruneWorkerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/PruneWorkerTest.kt new file mode 100644 index 0000000..71fae4d --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/PruneWorkerTest.kt @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import androidx.work.ListenableWorker.Result +import dagger.Lazy +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.libremail.data.security.EncryptedCacheGuard +import kotlin.test.assertEquals + +/** + * [PruneWorker] must not touch the database while the encrypted cache is locked: opening it would park + * this WorkManager thread on an unsatisfiable passphrase await (and wedge the shared serial executor). + * It gates on [EncryptedCacheGuard] and only resolves the (`Lazy`) [MailPruner] once unlocked — the + * invariant every pre-auth DB entry point shares with `SyncWorker`/`SendWorker`. + */ +class PruneWorkerTest { + + private val pruner = mockk() + private val lazyPruner = mockk> { every { get() } returns pruner } + private val cacheGuard = mockk() + + private fun worker() = PruneWorker(mockk(relaxed = true), mockk(relaxed = true), lazyPruner, cacheGuard) + + @Test + fun `retries without resolving the pruner when the cache is locked`() = runTest { + coEvery { cacheGuard.isCacheLocked() } returns true + + assertEquals(Result.retry(), worker().doWork()) + + // The whole point of the guard: the DB-backed dependency is never even resolved while locked. + verify(exactly = 0) { lazyPruner.get() } + coVerify(exactly = 0) { pruner.prune(any()) } + } + + @Test + fun `prunes and succeeds when the cache is unlocked`() = runTest { + coEvery { cacheGuard.isCacheLocked() } returns false + coEvery { pruner.prune(any()) } returns 0 + + assertEquals(Result.success(), worker().doWork()) + + coVerify(exactly = 1) { pruner.prune(any()) } + } + + @Test + fun `retries when pruning throws`() = runTest { + coEvery { cacheGuard.isCacheLocked() } returns false + coEvery { pruner.prune(any()) } throws IllegalStateException("boom") + + assertEquals(Result.retry(), worker().doWork()) + } +}