fix(upgrade): Align the round-4 restore, recorder sentinel and contention test

Restore the consumed sponsor marker only when no newer launch was armed
meanwhile. Make the recorder's monotonic base a nullable sentinel so a
boot-adjacent elapsedRealtime of 0 is not mistaken for a resumed session,
and clear it on stop. Pin the contention test's collector subscription and
assert it actually echoed.
This commit is contained in:
darken
2026-08-03 20:28:01 +02:00
committed by Matthias Urhahn
parent 9b60ed945f
commit 495f58c23b
5 changed files with 118 additions and 9 deletions
@@ -173,10 +173,16 @@ class UpgradeViewModel @Inject constructor(
} catch (e: Exception) {
// The marker was consumed above; neither a failed entitlement read nor a failed write may
// eat the user's valid sponsor visit — restore it so the next return/resume can retry the
// unlock. Rethrow unconditionally: cancellation is not swallowed, other errors surface via
// the normal error path. A restored marker after a successful persist is harmless — the
// next evaluation hits the quiet isPro path.
handle[KEY_SPONSOR_PRESSED_AT] = pressedAt
// unlock. Conditional: the user may have armed a NEWER launch while this attempt was
// suspended, and that one must survive. The contains-check has a small check-then-act
// window against a concurrent new arm; accepted — the create-only transaction owns data
// integrity, a wrong winner only changes which REAL visit's timestamp gates the unlock.
// Rethrow unconditionally: cancellation is not swallowed, other errors surface via the
// normal error path. A restored marker after a successful persist is harmless — the next
// evaluation hits the quiet isPro path.
if (!handle.contains(KEY_SPONSOR_PRESSED_AT)) {
handle[KEY_SPONSOR_PRESSED_AT] = pressedAt
}
throw e
}
}
@@ -121,7 +121,7 @@ class RecorderModule @Inject constructor(
recorder = newRecorder,
currentLogDir = sessionDir,
recordingStartedAt = startTime,
recordingStartedAtMonotonic = if (isResume) 0L else timeSource.elapsedRealtime(),
recordingStartedAtMonotonic = if (isResume) null else timeSource.elapsedRealtime(),
persistedLogDir = null,
)
} else if (!shouldRecord && isRecording) {
@@ -137,6 +137,7 @@ class RecorderModule @Inject constructor(
recorder = null,
currentLogDir = null,
recordingStartedAt = 0L,
recordingStartedAtMonotonic = null,
)
} else {
this
@@ -238,9 +239,10 @@ class RecorderModule @Inject constructor(
if (!currentState.isRecording) return StopResult.NotRecording
val logDir = currentState.currentLogDir ?: return StopResult.NotRecording
val elapsed = if (currentState.recordingStartedAtMonotonic > 0L) {
val startedAtMono = currentState.recordingStartedAtMonotonic
val elapsed = if (startedAtMono != null) {
// Live session: monotonic, immune to wall-clock adjustments mid-recording.
timeSource.elapsedRealtime() - currentState.recordingStartedAtMonotonic
timeSource.elapsedRealtime() - startedAtMono
} else {
// Resumed session: the trigger file persists wall time only — it has to survive reboots,
// which monotonic time does not.
@@ -266,10 +268,10 @@ class RecorderModule @Inject constructor(
internal val recorder: Recorder? = null,
val currentLogDir: File? = null,
val recordingStartedAt: Long = 0L,
// Monotonic base for the duration heuristic, 0L when there is none: a resumed session's
// Monotonic base for the duration heuristic, null when there is none: a resumed session's
// only start time is the persisted wall clock, and a monotonic value from a previous
// process or boot is meaningless.
val recordingStartedAtMonotonic: Long = 0L,
internal val recordingStartedAtMonotonic: Long? = null,
internal val persistedLogDir: File? = null,
) {
val isRecording: Boolean
@@ -172,6 +172,53 @@ class RecorderModuleDurationTest : BaseTest() {
}
}
@Test
fun `a recording started at monotonic zero still uses the monotonic path`() {
// Boot-adjacent start: elapsedRealtime() is legitimately 0 right after boot. A 0L sentinel
// reads as "resumed" and diverts to the wall clock, so a clock correction would turn three
// seconds of recording into an hour and skip the prompt.
val timeSource = TestTimeSource(elapsedRealtimeMs = 0L)
withModule(timeSource) { module ->
module.startRecorder()
timeSource.elapsedRealtimeMs += 3_000L
timeSource.wallNow = timeSource.wallNow.plus(Duration.ofHours(1))
module.requestStopRecorder() shouldBe RecorderModule.StopResult.TooShort
module.state.first().isRecording shouldBe true
}
}
@Test
fun `a live session carries a monotonic base and a stop clears it`() {
val timeSource = TestTimeSource(elapsedRealtimeMs = 100_000L)
withModule(timeSource) { module ->
module.startRecorder()
module.state.first { it.isRecording }.recordingStartedAtMonotonic shouldBe 100_000L
timeSource.advanceBy(Duration.ofSeconds(10))
module.requestStopRecorder().shouldBeInstanceOf<RecorderModule.StopResult.Stopped>()
// A stale base left on the stopped state would be a lie about a session that is over.
module.state.first { !it.isRecording }.recordingStartedAtMonotonic shouldBe null
}
}
@Test
fun `a resumed session carries no monotonic base`() {
// Nothing monotonic survives a process death or reboot, so the resumed state has no base
// at all — the wall-clock fallback is the only measurement it can make.
val timeSource = TestTimeSource(elapsedRealtimeMs = 100_000L)
seedTriggerFile(timeSource.currentTimeMillis() - 20_000L)
withModule(timeSource) { module ->
module.state.first { it.isRecording }.recordingStartedAtMonotonic shouldBe null
module.requestStopRecorder().shouldBeInstanceOf<RecorderModule.StopResult.Stopped>()
module.state.first { !it.isRecording }.recordingStartedAtMonotonic shouldBe null
}
}
@Test
fun `a resumed session measures from the persisted start time`() {
// Resumed after a process death: there is no monotonic base to measure against, so the
@@ -2,11 +2,13 @@ package eu.darken.capod.common.flow
import eu.darken.capod.common.collections.mutate
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.matchers.ints.shouldBeGreaterThan
import io.kotest.matchers.shouldBe
import io.kotest.matchers.types.shouldBeInstanceOf
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.mockk
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
@@ -304,8 +306,10 @@ class DynamicStateFlowTest : BaseTest() {
// own, keeping the producer busy between the other callers' updates. The echo updates
// are value-neutral so the final count stays exact, and capped so the echo terminates.
val echoes = AtomicInteger(0)
val subscribed = CompletableDeferred<Unit>()
scope.launch {
hotData.flow.collect { value ->
subscribed.complete(Unit)
if (value % 2 == 1 && echoes.getAndIncrement() < 100) {
hotData.updateAsync { this + 0 }
}
@@ -314,6 +318,10 @@ class DynamicStateFlowTest : BaseTest() {
runBlocking {
withTimeout(20_000) {
// The contention only means anything with the reactive collector actually
// attached: its first received value proves the subscription exists.
subscribed.await()
val workers = (1..2).map {
launch(Dispatchers.IO) {
repeat(100) { hotData.updateBlocking { this + 1 } }
@@ -323,6 +331,9 @@ class DynamicStateFlowTest : BaseTest() {
workers.all { it.isCompleted } shouldBe true
hotData.flow.first() shouldBe 200
// Non-vacuity: without a single echo there was no successor update to displace
// an awaited State, and the test would pass for the wrong reason.
echoes.get() shouldBeGreaterThan 0
}
}
} finally {
@@ -1,5 +1,6 @@
package eu.darken.capod.common.upgrade.ui
import android.os.SystemClock
import androidx.lifecycle.SavedStateHandle
import eu.darken.capod.R
import eu.darken.capod.common.navigation.NavEvent
@@ -14,6 +15,7 @@ import io.mockk.coVerify
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
@@ -407,6 +409,47 @@ class FossUpgradeViewModelTest : BaseTest() {
errorCollector.cancel()
}
@Test
fun `a newer sponsor launch survives a failed older attempt`() = runTest2(context = testDispatcher) {
// The restore must not clobber a launch armed while the old attempt was still suspended:
// the newer visit is the one the user is actually waiting on.
val repo = mockRepo()
val gate = CompletableDeferred<Unit>()
coEvery { repo.persistUpgrade() } coAnswers {
gate.await()
throw IOException("write failed")
}
val handle = SavedStateHandle()
val vm = buildVm(repo = repo, handle = handle)
val errors = mutableListOf<Throwable>()
val errorCollector = launch(start = CoroutineStart.UNDISPATCHED) { vm.errorEvents.collect { errors.add(it) } }
vm.goGithubSponsors()
ShadowSystemClock.advanceBy(Duration.ofSeconds(6))
vm.checkSponsorReturn()
advanceUntilIdle()
// Consumed and parked in the write.
vm.hasPendingSponsorLaunch() shouldBe false
// A second sponsor visit while the first attempt is still hanging.
ShadowSystemClock.advanceBy(Duration.ofSeconds(30))
val newerPressedAt = SystemClock.elapsedRealtime()
vm.goGithubSponsors()
advanceUntilIdle()
gate.complete(Unit)
advanceUntilIdle()
vm.hasPendingSponsorLaunch() shouldBe true
// Mirrors the ViewModel's private KEY_SPONSOR_PRESSED_AT: the newer timestamp must still be
// the one stored, the failed older attempt must not have written its own back over it.
handle.get<Long>("sponsor_pressed_at") shouldBe newerPressedAt
errors.single().shouldBeInstanceOf<IOException>()
errorCollector.cancel()
}
@Test
fun `a sponsor page that never opened arms nothing and a later retry still works`() = runTest2(
context = testDispatcher,