Merge branch 'fix-race-condition-spawning-multiple-daemons'

3 files changed, 138 insertions, 62 deletions

diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/DaemonInstance.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/DaemonInstance.kt
new file mode 100644
index 0000000000..f3323e1aa3
--- /dev/null
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/DaemonInstance.kt
@@ -0,0 +1,110 @@
+package net.mullvad.mullvadvpn.service
+
+import java.io.File
+import kotlin.properties.Delegates.observable
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.GlobalScope
+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
+
+private const val API_IP_ADDRESS_FILE = "api-ip-address.txt"
+private const val RELAYS_FILE = "relays.json"
+
+class DaemonInstance(val vpnService: MullvadVpnService, val listener: (MullvadDaemon?) -> Unit) {
+    private enum class Command {
+        START,
+        STOP,
+    }
+
+    private val commandChannel = spawnActor()
+
+    private var daemon by observable<MullvadDaemon?>(null) { _, _, newInstance ->
+        listener(newInstance)
+    }
+
+    fun start() {
+        commandChannel.sendBlocking(Command.START)
+    }
+
+    fun stop() {
+        commandChannel.sendBlocking(Command.STOP)
+    }
+
+    fun onDestroy() {
+        commandChannel.close()
+    }
+
+    private fun spawnActor() = GlobalScope.actor<Command>(Dispatchers.Default, Channel.UNLIMITED) {
+        var isRunning = true
+
+        prepareFiles()
+        vpnService.splitTunneling.join()
+
+        while (isRunning) {
+            if (!waitForCommand(channel, Command.START)) {
+                break
+            }
+
+            startDaemon()
+
+            isRunning = waitForCommand(channel, Command.STOP)
+
+            stopDaemon()
+        }
+    }
+
+    private suspend fun waitForCommand(
+        channel: ReceiveChannel<Command>,
+        command: Command
+    ): Boolean {
+        try {
+            while (channel.receive() != command) {
+                // Wait for command
+            }
+
+            return true
+        } catch (exception: ClosedReceiveChannelException) {
+            return false
+        }
+    }
+
+    private fun prepareFiles() {
+        FileMigrator(File("/data/data/net.mullvad.mullvadvpn"), vpnService.filesDir).apply {
+            migrate(RELAYS_FILE)
+            migrate("settings.json")
+            migrate("daemon.log")
+            migrate("daemon.old.log")
+            migrate("wireguard.log")
+            migrate("wireguard.old.log")
+        }
+
+        val shouldOverwriteRelayList =
+            lastUpdatedTime() > File(vpnService.filesDir, RELAYS_FILE).lastModified()
+
+        FileResourceExtractor(vpnService).apply {
+            extract(API_IP_ADDRESS_FILE, false)
+            extract(RELAYS_FILE, shouldOverwriteRelayList)
+        }
+    }
+
+    private fun startDaemon() {
+        daemon = MullvadDaemon(vpnService).apply {
+            onDaemonStopped = {
+                daemon = null
+            }
+        }
+    }
+
+    private fun stopDaemon() {
+        daemon?.shutdown()
+    }
+
+    private fun lastUpdatedTime(): Long {
+        return vpnService.run {
+            packageManager.getPackageInfo(packageName, 0).lastUpdateTime
+        }
+    }
+}
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
index f7eba5c1da..c3749c17f4 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
@@ -7,7 +7,6 @@ import android.net.VpnService
 import android.os.Binder
 import android.os.IBinder
 import android.util.Log
-import java.io.File
 import kotlin.properties.Delegates.observable
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.Dispatchers
@@ -21,9 +20,6 @@ import net.mullvad.mullvadvpn.ui.MainActivity
 import net.mullvad.talpid.TalpidVpnService
 import net.mullvad.talpid.util.EventNotifier
 
-private const val API_IP_ADDRESS_FILE = "api-ip-address.txt"
-private const val RELAYS_FILE = "relays.json"
-
 class MullvadVpnService : TalpidVpnService() {
     companion object {
         private val TAG = "mullvad"
@@ -45,12 +41,10 @@ class MullvadVpnService : TalpidVpnService() {
     private val binder = LocalBinder()
     private val serviceNotifier = EventNotifier<ServiceInstance?>(null)
 
-    private val splitTunneling = CompletableDeferred<SplitTunneling>()
-
     private var isStopping = false
     private var shouldStop = false
 
-    private var startDaemonJob: Job? = null
+    private var setUpDaemonJob: Job? = null
 
     private var instance by observable<ServiceInstance?>(null) { _, oldInstance, newInstance ->
         if (newInstance != oldInstance) {
@@ -69,6 +63,7 @@ class MullvadVpnService : TalpidVpnService() {
         oldNotification?.onDestroy()
     }
 
+    private lateinit var daemonInstance: DaemonInstance
     private lateinit var keyguardManager: KeyguardManager
     private lateinit var notificationManager: ForegroundNotificationManager
     private lateinit var tunnelStateUpdater: TunnelStateUpdater
@@ -90,6 +85,8 @@ class MullvadVpnService : TalpidVpnService() {
         notificationManager.lockedToForeground = isUiVisible or isBound
     }
 
+    internal val splitTunneling = CompletableDeferred<SplitTunneling>()
+
     override fun onCreate() {
         super.onCreate()
         Log.d(TAG, "Initializing service")
@@ -102,7 +99,11 @@ class MullvadVpnService : TalpidVpnService() {
 
         notificationManager.acknowledgeStartForegroundService()
 
-        setUp()
+        daemonInstance = DaemonInstance(this) { daemon ->
+            handleDaemonInstance(daemon)
+        }
+
+        daemonInstance.start()
     }
 
     override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
@@ -171,8 +172,8 @@ class MullvadVpnService : TalpidVpnService() {
 
     override fun onDestroy() {
         Log.d(TAG, "Service has stopped")
-        tearDown()
         notificationManager.onDestroy()
+        daemonInstance.onDestroy()
         super.onDestroy()
     }
 
@@ -196,27 +197,22 @@ class MullvadVpnService : TalpidVpnService() {
         )
     }
 
-    private fun setUp() {
-        startDaemonJob?.cancel()
-        startDaemonJob = startDaemon()
-    }
-
-    private fun startDaemon() = GlobalScope.launch(Dispatchers.Default) {
-        Log.d(TAG, "Starting daemon")
-        prepareFiles()
-        splitTunneling.await()
+    private fun handleDaemonInstance(daemon: MullvadDaemon?) {
+        setUpDaemonJob?.cancel()
 
-        val daemon = MullvadDaemon(this@MullvadVpnService).apply {
-            onDaemonStopped = {
-                Log.d(TAG, "Daemon has stopped")
-                instance = null
+        if (daemon != null) {
+            setUpDaemonJob = setUpDaemon(daemon)
+        } else {
+            Log.d(TAG, "Daemon has stopped")
+            instance = null
 
-                if (!isStopping) {
-                    restart()
-                }
+            if (!isStopping) {
+                restart()
             }
         }
+    }
 
+    private fun setUpDaemon(daemon: MullvadDaemon) = GlobalScope.launch(Dispatchers.Default) {
         val settings = daemon.getSettings()
 
         if (settings != null) {
@@ -226,25 +222,6 @@ class MullvadVpnService : TalpidVpnService() {
         }
     }
 
-    private fun prepareFiles() {
-        FileMigrator(File("/data/data/net.mullvad.mullvadvpn"), filesDir).apply {
-            migrate(RELAYS_FILE)
-            migrate("settings.json")
-            migrate("daemon.log")
-            migrate("daemon.old.log")
-            migrate("wireguard.log")
-            migrate("wireguard.old.log")
-        }
-
-        val shouldOverwriteRelayList =
-            lastUpdatedTime() > File(filesDir, RELAYS_FILE).lastModified()
-
-        FileResourceExtractor(this).apply {
-            extract(API_IP_ADDRESS_FILE, false)
-            extract(RELAYS_FILE, shouldOverwriteRelayList)
-        }
-    }
-
     private suspend fun setUpInstance(daemon: MullvadDaemon, settings: Settings) {
         val settingsListener = SettingsListener(daemon, settings)
         val connectionProxy = ConnectionProxy(this, daemon)
@@ -271,24 +248,16 @@ class MullvadVpnService : TalpidVpnService() {
         Log.d(TAG, "Stopping service")
         isStopping = true
         shouldStop = true
-        stopDaemon()
+        daemonInstance.stop()
         stopSelf()
     }
 
-    private fun stopDaemon() {
-        Log.d(TAG, "Stopping daemon")
-        startDaemonJob?.cancel()
-        instance?.daemon?.shutdown()
-    }
-
-    private fun tearDown() {
-        stopDaemon()
-    }
-
     private fun restart() {
         Log.d(TAG, "Restarting service")
-        tearDown()
-        setUp()
+        daemonInstance.apply {
+            stop()
+            start()
+        }
     }
 
     private fun handlePendingAction(connectionProxy: ConnectionProxy, settings: Settings) {
@@ -315,8 +284,4 @@ class MullvadVpnService : TalpidVpnService() {
 
         startActivity(intent)
     }
-
-    private fun lastUpdatedTime(): Long {
-        return packageManager.getPackageInfo(packageName, 0).lastUpdateTime
-    }
 }
diff --git a/mullvad-jni/src/lib.rs b/mullvad-jni/src/lib.rs
index dd45177cdf..b8d50a55d6 100644
--- a/mullvad-jni/src/lib.rs
+++ b/mullvad-jni/src/lib.rs
@@ -128,6 +128,8 @@ pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_initial
 
     match start_logging(&resource_dir) {
         Ok(()) => {
+            version::log_version();
+
             LOAD_CLASSES.call_once(|| env.preload_classes(classes::CLASSES.iter().cloned()));
 
             if let Err(error) = initialize(&env, &this, &vpnService, cache_dir, resource_dir) {
@@ -156,7 +158,6 @@ fn initialize_logging(log_dir: &Path) -> Result<(), String> {
         .map_err(|error| error.display_chain_with_msg("Failed to start logger"))?;
     exception_logging::enable();
     log_panics::init();
-    version::log_version();
 
     Ok(())
 }