diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 5d35ca1..07ca905 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -91,6 +91,19 @@ android:exported="false" android:foregroundServiceType="dataSync" /> + + + () .setConstraints(networkConstraint) .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) .build() - workManager.enqueueUniqueWork(ONESHOT_WORK, ExistingWorkPolicy.REPLACE, request) + return workManager.enqueueUniqueWork(ONESHOT_WORK, ExistingWorkPolicy.REPLACE, request) } /** diff --git a/app/src/main/kotlin/org/libremail/restart/ProcessRestarter.kt b/app/src/main/kotlin/org/libremail/restart/ProcessRestarter.kt new file mode 100644 index 0000000..34edbd6 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/restart/ProcessRestarter.kt @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.restart + +import android.content.Context +import android.content.Intent +import android.os.Process +import dagger.hilt.android.qualifiers.ApplicationContext +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Relaunches the whole app in a brand-new process by handing off to [RestartActivity], a trampoline + * that runs in the separate `:restart` process. Because the trampoline survives the current process + * being killed, the relaunch it issues cannot be dropped by ActivityManager scheduling it into the + * dying process — the failure mode of a same-process "startActivity then exit(0)" restart. + * + * Used by the app-lock key-invalidation recovery to bounce the process so the cache is wiped safely at + * the next cold start (before Room reopens it). + */ +@Singleton +class ProcessRestarter @Inject constructor(@ApplicationContext private val context: Context) { + + /** + * Start the [RestartActivity] trampoline in the `:restart` process, passing it this (main) process + * PID so it can kill us and relaunch from the outside. Returns immediately; the actual kill + + * relaunch happens in the trampoline process moments later. + */ + fun restart() { + val trampoline = Intent(context, RestartActivity::class.java).apply { + // Required because we may be started from a non-Activity (Application) context. + addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + putExtra(RestartActivity.EXTRA_ORIGINAL_PID, Process.myPid()) + } + context.startActivity(trampoline) + } + + companion object { + /** + * The `android:process` suffix of [RestartActivity] (must match AndroidManifest.xml). The + * Application uses it to skip its normal startup work when it is spun up in this aux process. + */ + const val PROCESS_SUFFIX = ":restart" + } +} diff --git a/app/src/main/kotlin/org/libremail/restart/RestartActivity.kt b/app/src/main/kotlin/org/libremail/restart/RestartActivity.kt new file mode 100644 index 0000000..3144024 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/restart/RestartActivity.kt @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.restart + +import android.app.Activity +import android.content.Intent +import android.os.Bundle +import android.os.Process +import android.util.Log + +/** + * Separate-process trampoline that performs an app relaunch from OUTSIDE the process being killed. + * + * Declared in the manifest with `android:process=":restart"`, so Android runs it in its own process. + * That is the whole point: it kills the original (main) process by PID and only THEN starts the main + * launcher activity, so the relaunch is scheduled from a process that is NOT the one being torn down. + * A same-process "startActivity then Runtime.exit(0)" restart races ActivityManager — the relaunch can + * be scheduled into the dying process and silently dropped, so the app just closes. Issuing it from a + * surviving process (the ProcessPhoenix pattern) makes the relaunch reliable. + * + * DEVICE-ONLY: the multi-process kill/relaunch cannot be exercised in JVM unit tests; see + * [ProcessRestarter] for the (testable) intent/targeting seam and AppLockViewModelTest for the + * ordering guarantees around it. + */ +class RestartActivity : Activity() { + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + // Kill the original main process FIRST so the relaunch below spins up a genuinely fresh + // process — one whose cold DatabaseModule performs the pending cache wipe before Room opens. + // If we relaunched while the old process were still alive, ActivityManager could route the + // launch back into it and the wipe-on-cold-start would never run. + val originalPid = intent.getIntExtra(EXTRA_ORIGINAL_PID, INVALID_PID) + if (originalPid > INVALID_PID && originalPid != Process.myPid()) { + Process.killProcess(originalPid) + } + + val launchIntent = packageManager.getLaunchIntentForPackage(packageName) + ?.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK) + if (launchIntent != null) { + startActivity(launchIntent) + } else { + Log.w(TAG, "no launch intent for $packageName; cannot relaunch after restart") + } + + finish() + // Tear down this trampoline process too: its only job was to issue the relaunch from outside + // the dying main process. + Runtime.getRuntime().exit(0) + } + + companion object { + /** Extra carrying the PID of the main process to kill, so the relaunch starts a fresh one. */ + const val EXTRA_ORIGINAL_PID = "org.libremail.restart.ORIGINAL_PID" + + private const val INVALID_PID = -1 + private const val TAG = "LibreMailRestart" + } +} diff --git a/app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt b/app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt index 90129d8..946f76c 100644 --- a/app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt @@ -2,16 +2,18 @@ package org.libremail.ui.lock import android.content.Context -import android.content.Intent import android.os.SystemClock import android.security.keystore.KeyPermanentlyInvalidatedException import android.security.keystore.UserNotAuthenticatedException import android.util.Log +import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import androidx.work.Operation import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -30,6 +32,10 @@ import org.libremail.data.security.LockState import org.libremail.data.security.PassphraseSession import org.libremail.data.settings.SettingsRepository import org.libremail.data.sync.SyncScheduler +import org.libremail.restart.ProcessRestarter +import java.util.concurrent.ExecutionException +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException import javax.inject.Inject /** UI state of the app-lock gate that wraps the whole app. */ @@ -73,6 +79,10 @@ class AppLockViewModel @Inject constructor( private val databaseKeyCipher: DatabaseKeyCipher, private val session: PassphraseSession, private val syncScheduler: SyncScheduler, + // Issues the key-invalidation recovery relaunch from a separate ":restart" process that survives + // this process being killed, so the relaunch can't be dropped by ActivityManager scheduling it + // into the dying process (the same-process "startActivity then exit(0)" race). + private val processRestarter: ProcessRestarter, // Application-scoped (see SecurityModule): the gate is injected rather than owned by this // Activity-scoped ViewModel so the inactivity grace window survives Activity recreation — Back on // the task root finishes the Activity and clears its ViewModelStore on API 29/30, which would @@ -83,6 +93,12 @@ class AppLockViewModel @Inject constructor( private val _uiState = MutableStateFlow(AppLockUiState.Checking) val uiState: StateFlow = _uiState.asStateFlow() + // The dispatcher for blocking Keystore/DataStore/WorkManager work pushed off the main thread. + // Injectable so the recovery flow (clearCacheAndRestart) runs on the test scheduler and its + // ordering — enqueue durably persisted BEFORE the restart — is deterministically verifiable. + @VisibleForTesting + internal var defaultDispatcher: CoroutineDispatcher = Dispatchers.Default + // Cached so onBackground / onForeground can cover the content synchronously (before the async // settings read) whenever app-lock is on — so no stale mailbox frame renders on resume. @Volatile private var appLockEnabledCached = false @@ -125,7 +141,7 @@ class AppLockViewModel @Inject constructor( // App-lock is on: cover any showing content while we resolve, so no stale mailbox frame // renders before the (async) decision lands. if (_uiState.value == AppLockUiState.Unlocked) _uiState.value = AppLockUiState.Checking - val action = withContext(Dispatchers.Default) { + val action = withContext(defaultDispatcher) { KeyInvalidationPolicy.decide( appLockEnabled = true, encryptCacheEnabled = settings.encryptCache, @@ -173,7 +189,7 @@ class AppLockViewModel @Inject constructor( /** Called by the host after a successful `BiometricPrompt`. */ fun onAuthenticated() { viewModelScope.launch { - when (withContext(Dispatchers.Default) { unlockOrArm() }) { + when (withContext(defaultDispatcher) { unlockOrArm() }) { UnlockResult.OK -> { gate.onAuthenticated() publish() @@ -258,25 +274,52 @@ class AppLockViewModel @Inject constructor( } private suspend fun clearCacheAndRestart(disableAppLock: Boolean) { - withContext(Dispatchers.Default) { + withContext(defaultDispatcher) { // Record the wipe intent BEFORE flipping app-lock off, so a crash between the two writes // leaves the wipe still pending (recoverable) rather than a disabled gate over a stale key. + // Both are DataStore edits that only return once durably committed, so they survive the + // restart below without further ceremony. databaseKeyStore.setClearPending() if (disableAppLock) settingsRepository.setAppLock(false) - syncScheduler.syncNow() // persisted by WorkManager; survives the restart + // Enqueue the post-wipe re-sync and BLOCK until WorkManager has durably persisted its + // WorkSpec before we hand off to the restart. syncNow() only *schedules* the insert on + // WorkManager's serial task executor; killing the process (via restartProcess) can race + // that async insert and drop the re-sync, leaving an empty mailbox after the wipe until the + // next periodic sync. Awaiting the enqueue Operation makes "survives the restart" real. + awaitSyncEnqueue(syncScheduler.syncNow()) } restartProcess() } + /** + * Block until WorkManager confirms the re-sync WorkSpec is durably persisted, bounded by + * [SYNC_ENQUEUE_TIMEOUT_SECONDS] so a stuck insert can never wedge recovery. A timeout/failure is + * logged and we restart anyway: the periodic sync will still eventually refill the wiped cache, so + * a best-effort wait is strictly better than the previous fire-and-forget enqueue. Runs on + * [defaultDispatcher] (never the main thread) because [Operation.result]'s get blocks. + */ + private fun awaitSyncEnqueue(operation: Operation) { + try { + operation.result.get(SYNC_ENQUEUE_TIMEOUT_SECONDS, TimeUnit.SECONDS) + } catch (e: TimeoutException) { + Log.w(TAG, "re-sync enqueue not confirmed within timeout; restarting anyway", e) + } catch (e: ExecutionException) { + Log.w(TAG, "re-sync enqueue failed; restarting anyway", e) + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + Log.w(TAG, "interrupted awaiting re-sync enqueue; restarting anyway", e) + } + } + /** * Relaunch the app in a fresh process so [org.libremail.di.DatabaseModule] wipes the cache before - * Room reopens it. DEVICE-ONLY: process restart cannot be exercised in JVM unit tests. + * Room reopens it. Delegates to [ProcessRestarter], which issues the relaunch from a separate + * process that survives this one being killed — a same-process "startActivity then exit(0)" is + * unreliable because ActivityManager may schedule the relaunch into the dying process and drop it. + * DEVICE-ONLY end to end: the multi-process kill/relaunch cannot be exercised in JVM unit tests. */ private fun restartProcess() { - val intent = context.packageManager.getLaunchIntentForPackage(context.packageName) - ?.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK) - if (intent != null) context.startActivity(intent) - Runtime.getRuntime().exit(0) + processRestarter.restart() } // Monotonic clock so a wall-clock change can't extend the inactivity grace window. @@ -284,5 +327,10 @@ class AppLockViewModel @Inject constructor( private companion object { const val TAG = "LibreMailAppLock" + + // Upper bound on waiting for WorkManager to persist the re-sync WorkSpec. The insert is + // normally sub-second; this only caps a pathological stall so recovery can't hang before the + // restart. On timeout we restart anyway (the periodic sync still refills the cache later). + const val SYNC_ENQUEUE_TIMEOUT_SECONDS = 5L } } diff --git a/app/src/test/kotlin/org/libremail/data/sync/SyncSchedulerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/SyncSchedulerTest.kt index 24092ed..81d267c 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/SyncSchedulerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/SyncSchedulerTest.kt @@ -4,12 +4,15 @@ package org.libremail.data.sync import androidx.work.ExistingPeriodicWorkPolicy import androidx.work.ExistingWorkPolicy import androidx.work.OneTimeWorkRequest +import androidx.work.Operation import androidx.work.PeriodicWorkRequest import androidx.work.WorkManager +import io.mockk.every import io.mockk.mockk import io.mockk.verify import org.junit.Test import javax.inject.Provider +import kotlin.test.assertSame /** * The enqueue methods are thin wrappers over WorkManager, so these tests pin the one thing that carries @@ -79,6 +82,18 @@ class SyncSchedulerTest { } } + // The app-lock key-invalidation recovery restart awaits this Operation before killing the process + // (see AppLockViewModel), so the WorkSpec is durably persisted and the post-wipe re-sync survives. + @Test + fun `syncNow returns the enqueue operation so callers can await durable persistence`() { + val operation = mockk() + every { + workManager.enqueueUniqueWork(any(), any(), any()) + } returns operation + + assertSame(operation, scheduler.syncNow()) + } + @Test fun `backfillNow keeps an already-running backfill`() { scheduler.backfillNow() diff --git a/app/src/test/kotlin/org/libremail/ui/lock/AppLockViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/lock/AppLockViewModelTest.kt index 7efcaa0..61dcf56 100644 --- a/app/src/test/kotlin/org/libremail/ui/lock/AppLockViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/lock/AppLockViewModelTest.kt @@ -2,8 +2,13 @@ package org.libremail.ui.lock import android.os.SystemClock +import android.util.Log +import androidx.work.Operation +import com.google.common.util.concurrent.ListenableFuture +import io.mockk.coVerifyOrder import io.mockk.every import io.mockk.mockk +import io.mockk.mockkObject import io.mockk.mockkStatic import io.mockk.unmockkAll import io.mockk.verify @@ -11,6 +16,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain @@ -18,8 +24,14 @@ import org.junit.After import org.junit.Before import org.junit.Test import org.libremail.data.security.AppLockGate +import org.libremail.data.security.DatabaseKeyStore +import org.libremail.data.security.KeyInvalidationPolicy +import org.libremail.data.security.LockAction import org.libremail.data.settings.AppSettings import org.libremail.data.settings.SettingsRepository +import org.libremail.data.sync.SyncScheduler +import org.libremail.restart.ProcessRestarter +import java.util.concurrent.TimeoutException import kotlin.test.assertEquals import kotlin.test.assertIs @@ -29,6 +41,11 @@ import kotlin.test.assertIs * itself (which Back on the task root would drop on API 29/30). These tests exercise the synchronous * paths that delegate to the injected gate; the grace math itself is covered exhaustively — and * deterministically — by AppLockGateTest. Broader ViewModel coverage is issue #100. + * + * The recovery-restart tests (#99) pin the ordering that makes the key-invalidation "clear + re-sync" + * safe: the re-sync enqueue must be durably persisted (its WorkManager Operation awaited) BEFORE the + * process is restarted, and a stuck enqueue must never wedge recovery. The separate-process relaunch + * itself is device-only; here we assert the ViewModel's orchestration around ProcessRestarter. */ @OptIn(ExperimentalCoroutinesApi::class) class AppLockViewModelTest { @@ -44,19 +61,27 @@ class AppLockViewModelTest { unmockkAll() } - private fun viewModel(gate: AppLockGate, appLock: Boolean = true): AppLockViewModel { - val settings = mockk() - every { settings.settings } returns flowOf(AppSettings(appLock = appLock)) + private fun viewModel( + gate: AppLockGate, + appLock: Boolean = true, + settingsRepository: SettingsRepository = mockk(relaxed = true), + databaseKeyStore: DatabaseKeyStore = mockk(relaxed = true), + syncScheduler: SyncScheduler = mockk(relaxed = true), + processRestarter: ProcessRestarter = mockk(relaxed = true), + ): AppLockViewModel { + every { settingsRepository.settings } returns flowOf(AppSettings(appLock = appLock)) return AppLockViewModel( context = mockk(relaxed = true), - settingsRepository = settings, + settingsRepository = settingsRepository, appLockManager = mockk(relaxed = true), - databaseKeyStore = mockk(relaxed = true), + databaseKeyStore = databaseKeyStore, databaseKeyCipher = mockk(relaxed = true), session = mockk(relaxed = true), - syncScheduler = mockk(relaxed = true), + syncScheduler = syncScheduler, + processRestarter = processRestarter, gate = gate, - ) + // Run the off-main recovery work on the test scheduler so its ordering is deterministic. + ).also { it.defaultDispatcher = dispatcher } } @Test @@ -84,4 +109,79 @@ class AppLockViewModelTest { val state = assertIs(vm.uiState.value) assertEquals("boom", state.error) } + + @Test + fun `recovery persists the re-sync enqueue before restarting`() = runTest(dispatcher) { + val (future, syncScheduler) = enqueueingScheduler() + val databaseKeyStore = mockk(relaxed = true) + val processRestarter = mockk(relaxed = true) + val vm = clearOnForegroundViewModel( + databaseKeyStore = databaseKeyStore, + syncScheduler = syncScheduler, + processRestarter = processRestarter, + ) + + vm.onForeground() + advanceUntilIdle() + + // The re-sync WorkSpec must be durably persisted (the enqueue Operation awaited) BEFORE the + // process is torn down. Otherwise WorkManager's async insert races the process death, the + // enqueue is lost, and the just-wiped cache never refills until the next periodic sync. + coVerifyOrder { + databaseKeyStore.setClearPending() + syncScheduler.syncNow() + future.get(any(), any()) + processRestarter.restart() + } + } + + @Test + fun `recovery still restarts when the re-sync enqueue await times out`() = runTest(dispatcher) { + val (future, syncScheduler) = enqueueingScheduler() + every { future.get(any(), any()) } throws TimeoutException("stuck insert") + val processRestarter = mockk(relaxed = true) + val vm = clearOnForegroundViewModel(syncScheduler = syncScheduler, processRestarter = processRestarter) + + vm.onForeground() + advanceUntilIdle() + + // A stuck WorkManager insert must not wedge recovery: the timeout is swallowed and we restart + // anyway (the periodic sync will still refill the wiped cache later). + verify { future.get(any(), any()) } + verify { processRestarter.restart() } + } + + /** A [SyncScheduler] whose `syncNow()` returns an [Operation] whose result future can be stubbed. */ + private fun enqueueingScheduler(): Pair, SyncScheduler> { + val future = mockk>() + every { future.get(any(), any()) } returns Operation.SUCCESS + val operation = mockk { every { result } returns future } + val syncScheduler = mockk { every { syncNow() } returns operation } + return future to syncScheduler + } + + /** + * A ViewModel whose next `onForeground()` resolves to a cache-clear + restart: the pure decision + * table is stubbed to CLEAR_AND_REQUIRE_AUTH so the test drives the recovery path deterministically + * without reproducing the full key-invalidation device state (that logic is KeyInvalidationPolicyTest). + */ + private fun clearOnForegroundViewModel( + databaseKeyStore: DatabaseKeyStore = mockk(relaxed = true), + syncScheduler: SyncScheduler = mockk(relaxed = true), + processRestarter: ProcessRestarter = mockk(relaxed = true), + ): AppLockViewModel { + mockkStatic(SystemClock::class) + every { SystemClock.elapsedRealtime() } returns 1_000L + // android.util.Log is a no-op stub that throws "not mocked" in JVM tests; the timeout path logs. + mockkStatic(Log::class) + every { Log.w(any(), any(), any()) } returns 0 + mockkObject(KeyInvalidationPolicy) + every { KeyInvalidationPolicy.decide(any(), any(), any(), any()) } returns LockAction.CLEAR_AND_REQUIRE_AUTH + return viewModel( + gate = mockk(relaxed = true), + databaseKeyStore = databaseKeyStore, + syncScheduler = syncScheduler, + processRestarter = processRestarter, + ) + } }