diff --git a/android/src/main/java/com/tailscale/ipn/ui/notifier/HealthNotifier.kt b/android/src/main/java/com/tailscale/ipn/ui/notifier/HealthNotifier.kt index 412bff0fa3..d31784b303 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/notifier/HealthNotifier.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/notifier/HealthNotifier.kt @@ -43,6 +43,13 @@ class HealthNotifier( // notification "wantrunning-false") + // These must be initialized before the init block below, which launches a coroutine that can + // immediately call dropAllWarnings() (reading currentWarnings) on a background dispatcher. If + // the collector observes the initial non-Running ipn state before these property initializers + // run, it would read a null StateFlow and crash with an NPE (see startup init-order race). + val currentWarnings: StateFlow> = MutableStateFlow(setOf()) + val currentIcon: StateFlow = MutableStateFlow(null) + init { // This roughly matches the iOS/macOS implementation in terms of debouncing, and ignoring // health warnings in various states. @@ -68,9 +75,6 @@ class HealthNotifier( } } - val currentWarnings: StateFlow> = MutableStateFlow(setOf()) - val currentIcon: StateFlow = MutableStateFlow(null) - private fun notifyHealthUpdated(warnings: Array) { val warningsBeforeAdd = currentWarnings.value val currentWarnableCodes = warnings.map { it.WarnableCode }.toSet() diff --git a/android/src/test/kotlin/com/tailcale/ipn/ui/notifier/HealthNotifierTest.kt b/android/src/test/kotlin/com/tailcale/ipn/ui/notifier/HealthNotifierTest.kt index 7211861c73..8fb2c4e086 100644 --- a/android/src/test/kotlin/com/tailcale/ipn/ui/notifier/HealthNotifierTest.kt +++ b/android/src/test/kotlin/com/tailcale/ipn/ui/notifier/HealthNotifierTest.kt @@ -10,7 +10,10 @@ import com.tailscale.ipn.ui.model.Health.UnhealthyState import com.tailscale.ipn.ui.model.Ipn import com.tailscale.ipn.util.TSLog import com.tailscale.ipn.util.TSLog.LibtailscaleWrapper +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runCurrent @@ -123,6 +126,27 @@ class HealthNotifierTest { assertTrue(notifier.currentWarnings.value.isEmpty()) } + /** + * Regression test for a startup init-order race: the init block launches a collector that + * immediately calls dropAllWarnings() (reading currentWarnings) when the initial ipn state is not + * Running. If currentWarnings/currentIcon are declared after the init block, that read hits a + * null StateFlow and NPEs. Using Dispatchers.Unconfined runs the launched coroutine eagerly + * inside the constructor, deterministically reproducing the race on the old ordering. + */ + @Test + fun constructionWithNonRunningStateDoesNotCrash() { + val scope = CoroutineScope(Dispatchers.Unconfined) + try { + val healthFlow = MutableStateFlow(healthState(derpWarning())) + val ipnFlow = MutableStateFlow(Ipn.State.Stopped) + val notifier = HealthNotifier(healthFlow, ipnFlow, scope) + assertTrue(notifier.currentWarnings.value.isEmpty()) + assertNull(notifier.currentIcon.value) + } finally { + scope.cancel() + } + } + @Test fun staleWarningFromStoppedDoesNotPersistAfterToggleOn() = runTest { val (healthFlow, ipnFlow, notifier) = createRunningNotifier()