Refactor `LocationInfoCache` into an actor

1 files changed, 52 insertions, 51 deletions

diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/LocationInfoCache.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/LocationInfoCache.kt
index 38e0dc209a..b123271fef 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/LocationInfoCache.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/LocationInfoCache.kt
@@ -2,10 +2,13 @@ package net.mullvad.mullvadvpn.service
 
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.GlobalScope
-import kotlinx.coroutines.Job
-import kotlinx.coroutines.async
-import kotlinx.coroutines.delay
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.TimeoutCancellationException
+import kotlinx.coroutines.channels.Channel
+import kotlinx.coroutines.channels.ClosedReceiveChannelException
+import kotlinx.coroutines.channels.ReceiveChannel
+import kotlinx.coroutines.channels.actor
+import kotlinx.coroutines.channels.sendBlocking
+import kotlinx.coroutines.withTimeout
 import net.mullvad.mullvadvpn.model.GeoIpLocation
 import net.mullvad.mullvadvpn.model.TunnelState
 import net.mullvad.mullvadvpn.relaylist.Relay
@@ -21,13 +24,11 @@ class LocationInfoCache(
     val connectionProxy: ConnectionProxy,
     val connectivityListener: ConnectivityListener
 ) {
-    private var activeFetch: Job? = null
+    private val fetchRequestChannel = runFetcher()
+
     private var lastKnownRealLocation: GeoIpLocation? = null
     private var selectedRelayLocation: GeoIpLocation? = null
 
-    private var fetchIdCounter = 0L
-    private var fetchIdIsActive = false
-
     var onNewLocation: ((GeoIpLocation?) -> Unit)? = null
         set(value) {
             field = value
@@ -43,17 +44,16 @@ class LocationInfoCache(
     var state: TunnelState = TunnelState.Disconnected()
         set(value) {
             field = value
-            cancelFetch()
 
             when (value) {
                 is TunnelState.Disconnected -> {
                     location = lastKnownRealLocation
-                    fetchLocation(true)
+                    fetchRequestChannel.sendBlocking(true)
                 }
                 is TunnelState.Connecting -> location = value.location
                 is TunnelState.Connected -> {
                     location = value.location
-                    fetchLocation(false)
+                    fetchRequestChannel.sendBlocking(false)
                 }
                 is TunnelState.Disconnecting -> {
                     when (value.actionAfterDisconnect) {
@@ -77,7 +77,7 @@ class LocationInfoCache(
     init {
         connectivityListener.connectivityNotifier.subscribe(this) { isConnected ->
             if (isConnected && state is TunnelState.Disconnected) {
-                fetchLocation(true)
+                fetchRequestChannel.sendBlocking(true)
             }
         }
 
@@ -89,7 +89,7 @@ class LocationInfoCache(
     fun onDestroy() {
         connectivityListener.connectivityNotifier.unsubscribe(this)
         connectionProxy.onStateChange.unsubscribe(this)
-        cancelFetch()
+        fetchRequestChannel.close()
     }
 
     private fun updateSelectedRelayLocation(relayItem: RelayItem?) {
@@ -113,56 +113,57 @@ class LocationInfoCache(
         }
     }
 
-    private fun newFetchId(): Long {
-        synchronized(this) {
-            if (fetchIdIsActive) {
-                fetchIdCounter += 1
-            } else {
-                fetchIdIsActive = true
-            }
-
-            return fetchIdCounter
+    private fun runFetcher() = GlobalScope.actor<Boolean>(Dispatchers.Default, Channel.CONFLATED) {
+        try {
+            fetcherLoop(channel)
+        } catch (exception: ClosedReceiveChannelException) {
         }
     }
 
-    private fun cancelFetch() {
-        synchronized(this) {
-            if (fetchIdIsActive) {
-                fetchIdCounter += 1
-                fetchIdIsActive = false
-            }
+    private suspend fun fetcherLoop(channel: ReceiveChannel<Boolean>) {
+        val delays = ExponentialBackoff().apply {
+            scale = 50
+            cap = 30 /* min */ * 60 /* s */ * 1000 /* ms */
+            count = 17 // ceil(log2(cap / scale) + 1)
         }
-    }
 
-    private fun fetchLocation(isRealLocation: Boolean) {
-        val fetchId = newFetchId()
-        val previousFetch = activeFetch
+        while (true) {
+            var isRealLocation = channel.receive()
+            var newLocation = daemon.getCurrentLocation()
 
-        activeFetch = GlobalScope.launch(Dispatchers.Default) {
-            val delays = ExponentialBackoff().apply {
-                scale = 50
-                cap = 30 /* min */ * 60 /* s */ * 1000 /* ms */
-                count = 17 // ceil(log2(cap / scale) + 1)
+            while (newLocation == null || !channel.isEmpty) {
+                isRealLocation = delayOrReceive(delays, channel, isRealLocation)
+                newLocation = daemon.getCurrentLocation()
             }
 
-            var newLocation: GeoIpLocation? = null
-
-            previousFetch?.join()
+            handleNewLocation(newLocation, isRealLocation)
+            delays.reset()
+        }
+    }
 
-            while (newLocation == null && fetchId == fetchIdCounter) {
-                delay(delays.next())
-                newLocation = daemon.getCurrentLocation()
+    private suspend fun delayOrReceive(
+        delays: ExponentialBackoff,
+        channel: ReceiveChannel<Boolean>,
+        currentValue: Boolean
+    ): Boolean {
+        try {
+            val newValue = withTimeout(delays.next()) {
+                channel.receive()
             }
 
-            synchronized(this@LocationInfoCache) {
-                if (newLocation != null && fetchId == fetchIdCounter) {
-                    location = newLocation
+            delays.reset()
 
-                    if (isRealLocation) {
-                        lastKnownRealLocation = newLocation
-                    }
-                }
-            }
+            return newValue
+        } catch (timeOut: TimeoutCancellationException) {
+            return currentValue
         }
     }
+
+    private fun handleNewLocation(newLocation: GeoIpLocation, isRealLocation: Boolean) {
+        if (isRealLocation) {
+            lastKnownRealLocation = newLocation
+        }
+
+        location = newLocation
+    }
 }