From 84e6402031fdcfa2edf1125df1ea96d5df10806a Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 2 Jul 2026 23:04:26 +0200 Subject: [PATCH] fix(contacts): serialize sync/teardown and stop retrying on lost permission - The periodic job and the immediate 'Sync now'/foreground runs had no mutual exclusion, so two overlapping reconciles could each see no managed calendar and both create one (duplicate 'Birthdays', doubled events); likewise a disable-teardown racing an in-flight sync got its calendars recreated right after deletion. A lifecycle Mutex now makes sync() and teardown() atomic, and sync() re-reads the enabled flag inside the lock so a teardown always wins. - The foreground resume trigger shared a unique work name with enable/'Sync now' under ExistingWorkPolicy.REPLACE, so a debounced foreground enqueue could cancel-and-swallow a pending enable sync (feature on, no calendars for up to a day). It now uses its own work name. - doWork() mapped every exception to retry(), so a revoked calendar permission retried with backoff forever and never surfaced. A SecurityException now parks the feature in the stalled state instead. Co-Authored-By: Claude Fable 5 --- .../data/contacts/SpecialDatesScheduler.kt | 19 +++++++++++++++--- .../data/contacts/SpecialDatesSyncEngine.kt | 20 ++++++++++++++----- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/contacts/SpecialDatesScheduler.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/contacts/SpecialDatesScheduler.kt index a906f9b..ba5cff5 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/contacts/SpecialDatesScheduler.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/contacts/SpecialDatesScheduler.kt @@ -30,6 +30,7 @@ object SpecialDatesScheduler { private const val WORK_NAME = "special-dates-sync" private const val WORK_NAME_NOW = "special-dates-sync-now" + private const val WORK_NAME_FOREGROUND = "special-dates-sync-foreground" private const val KEY_FOREGROUND = "foreground" /** Enqueue (or cancel) the daily periodic reconcile to match [enabled]. */ @@ -38,6 +39,7 @@ object SpecialDatesScheduler { if (!enabled) { workManager.cancelUniqueWork(WORK_NAME) workManager.cancelUniqueWork(WORK_NAME_NOW) + workManager.cancelUniqueWork(WORK_NAME_FOREGROUND) return } val request = PeriodicWorkRequestBuilder(1, TimeUnit.DAYS) @@ -48,15 +50,19 @@ object SpecialDatesScheduler { } /** - * Run one reconcile immediately. [foreground] runs are debounced (the app can - * resume often); enable/"Sync now" runs ([foreground] false) always sync. + * Run one reconcile immediately. [foreground] runs (the app resuming) are + * debounced inside the worker and use their own work name, so a frequent + * foreground resync can never REPLACE — and swallow — a pending enable / + * "Sync now" run, which always syncs. The engine serializes the two if they + * overlap. */ fun runNow(context: Context, foreground: Boolean = false) { val request = OneTimeWorkRequestBuilder() .setInputData(Data.Builder().putBoolean(KEY_FOREGROUND, foreground).build()) .build() + val name = if (foreground) WORK_NAME_FOREGROUND else WORK_NAME_NOW WorkManager.getInstance(context) - .enqueueUniqueWork(WORK_NAME_NOW, ExistingWorkPolicy.REPLACE, request) + .enqueueUniqueWork(name, ExistingWorkPolicy.REPLACE, request) } internal const val INPUT_FOREGROUND = KEY_FOREGROUND @@ -107,6 +113,13 @@ class SpecialDatesSyncWorker( } if (foreground) prefs.setSpecialDatesLastForegroundSync(now) Result.success() + } catch (e: SecurityException) { + // A revoked calendar/contacts permission won't fix itself on retry — + // park the feature in a stalled state (surfaced in settings) instead + // of retrying with backoff forever. + Log.w(TAG, "Special-dates sync lacks a required permission", e) + prefs.recordSpecialDatesRun(now, SpecialDatesStalledReason.PermissionRevoked) + Result.success() } catch (e: Exception) { Log.w(TAG, "Special-dates sync failed", e) Result.retry() diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/contacts/SpecialDatesSyncEngine.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/contacts/SpecialDatesSyncEngine.kt index 082d3bc..74ba06a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/contacts/SpecialDatesSyncEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/contacts/SpecialDatesSyncEngine.kt @@ -13,10 +13,12 @@ import de.jeanlucmakiola.calendula.domain.SimpleRecurrence import de.jeanlucmakiola.calendula.domain.contacts.ContactSpecialDate import de.jeanlucmakiola.calendula.domain.contacts.SpecialDateType import de.jeanlucmakiola.calendula.domain.contacts.anchorDate -import de.jeanlucmakiola.calendula.domain.contacts.managedEventUid +import de.jeanlucmakiola.calendula.domain.contacts.managedUid import de.jeanlucmakiola.calendula.domain.contacts.renderSpecialDateTitle import de.jeanlucmakiola.calendula.domain.toRRule import kotlinx.coroutines.flow.first +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.datetime.LocalDateTime import kotlinx.datetime.LocalTime import java.time.ZoneId @@ -64,11 +66,19 @@ class SpecialDatesSyncEngine @Inject constructor( private val spec: SpecialDatesCalendarSpec, ) { + // Serializes calendar-lifecycle work: two overlapping syncs (the daily job + // racing a "Sync now"/foreground run) would each see no managed calendar and + // both create one; a teardown racing an in-flight sync would delete calendars + // the sync then recreates. Holding this for the whole of sync()/teardown() + // makes those check-then-act sequences atomic, and — because sync() re-reads + // the enabled flag inside the lock — a teardown always wins the race. + private val lifecycleMutex = Mutex() + /** * Reconcile every enabled type against the device's contacts. Returns why it * stopped early (disabled / permission gone) or [SpecialDatesSyncResult.Success]. */ - suspend fun sync(): SpecialDatesSyncResult { + suspend fun sync(): SpecialDatesSyncResult = lifecycleMutex.withLock { if (!prefs.specialDatesEnabled.first()) return SpecialDatesSyncResult.Disabled if (!contacts.hasPermission()) return SpecialDatesSyncResult.PermissionMissing @@ -91,7 +101,7 @@ class SpecialDatesSyncEngine @Inject constructor( reminderCtx = reminderCtx, ) } - return SpecialDatesSyncResult.Success + SpecialDatesSyncResult.Success } /** @@ -117,7 +127,7 @@ class SpecialDatesSyncEngine @Inject constructor( } /** Delete every managed calendar and forget its id — used when the feature is turned off. */ - suspend fun teardown() { + suspend fun teardown() = lifecycleMutex.withLock { calendars.findManagedCalendars().forEach { calendars.deleteCalendar(it.id) } SpecialDateType.entries.forEach { prefs.setSpecialDatesCalendarId(it, null) } } @@ -211,7 +221,7 @@ class SpecialDatesSyncEngine @Inject constructor( val dtStartMillis = form.toWriteTimes(ZoneId.systemDefault()).dtStartMillis return BuiltManagedEvent( desired = ManagedEventDesired( - uid = managedEventUid(type, sd.lookupKey), + uid = sd.managedUid(), title = title, dtStartMillis = dtStartMillis, rrule = YEARLY_RRULE,