Ensure `ListenerReady` handler is registered first

Previously there could be a race condition because the
`ServiceConnection` constructor would register itself as an event
listener on the service without registering a handler for the
`ListenerReady` event sent after the registration is complete. The
`MainActivity` would then register the handler, and things would work
because the service would send multiple events before sending the
`ListenerReady`, giving time for the handler to be registered before the
event is received.

This commit changes that in order to avoid the race condition. Now the
`ServiceConnection` constructor must receive a callback as a parameter,
and this callback will be used when registering for the `ListenerReady`
event. This allows the constructor to enforce that the handler for the
event is configured before the request for registration is sent.

2 files changed, 13 insertions, 11 deletions

diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
index 6569c155d2..46f3aa135e 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
@@ -16,7 +16,6 @@ import androidx.fragment.app.FragmentManager
 import net.mullvad.mullvadvpn.BuildConfig
 import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.dataproxy.MullvadProblemReport
-import net.mullvad.mullvadvpn.ipc.Event
 import net.mullvad.mullvadvpn.service.MullvadVpnService
 import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnection
 import net.mullvad.talpid.util.EventNotifier
@@ -54,17 +53,13 @@ open class MainActivity : FragmentActivity() {
                 android.util.Log.d("mullvad", "UI connection to the service changed: $service")
                 serviceConnection?.onDestroy()
 
-                val newConnection = service?.let { safeService ->
-                    ServiceConnection(safeService)
+                serviceConnection = service?.let { safeService ->
+                    ServiceConnection(safeService) { connection ->
+                        serviceNotifier.notify(connection)
+                    }
                 }
 
-                serviceConnection = newConnection
-
-                if (newConnection != null) {
-                    newConnection.dispatcher.registerHandler(Event.ListenerReady::class) { _ ->
-                        serviceNotifier.notify(newConnection)
-                    }
-                } else {
+                if (service == null) {
                     serviceNotifier.notify(null)
                 }
 
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnection.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnection.kt
index f24ca088f4..9fba60681f 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnection.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnection.kt
@@ -20,7 +20,10 @@ import org.koin.core.scope.get
 // The properties of this class can be used to send events to the service, to listen for events from
 // the service and to get values received from events.
 @OptIn(KoinApiExtension::class)
-class ServiceConnection(private val service: ServiceInstance) : KoinScopeComponent {
+class ServiceConnection(
+    private val service: ServiceInstance,
+    onServiceReady: (ServiceConnection) -> Unit
+) : KoinScopeComponent {
     override val scope = getKoin().createScope(
         SERVICE_CONNECTION_SCOPE,
         named(SERVICE_CONNECTION_SCOPE), this
@@ -47,6 +50,10 @@ class ServiceConnection(private val service: ServiceInstance) : KoinScopeCompone
     var relayListListener = RelayListListener(service.messenger, dispatcher, settingsListener)
 
     init {
+        dispatcher.registerHandler(Event.ListenerReady::class) { _ ->
+            onServiceReady(this@ServiceConnection)
+        }
+
         registerListener()
     }