Add synchronization for RoutingManager

mobile/src/main/java/be/mygod/vpnhotspot/RoutingManager.kt

@@ -24,10 +24,13 @@ abstract class RoutingManager(private val caller: Any, val downstream: String, p
             }
             set(value) = app.pref.edit().putString(KEY_MASQUERADE_MODE, value.name).apply()
 
+        /**
+         * Thread safety: needs protection by companion object!
+         */
         private val active = mutableMapOf<String, RoutingManager>()
 
-        fun clean(reinit: Boolean = true) {
+        fun clean(reinit: Boolean = true) = synchronized(this) {
-            if (!reinit && active.isEmpty()) return
+            if (!reinit && active.isEmpty()) return@synchronized
             for (manager in active.values) manager.routing?.stop()
             try {
                 Routing.clean()
@@ -36,7 +39,7 @@ abstract class RoutingManager(private val caller: Any, val downstream: String, p
                 SmartSnackbar.make(e).show()
                 return
             }
-            if (reinit) for (manager in active.values) manager.initRouting()
+            if (reinit) for (manager in active.values) manager.initRoutingLocked()
         }
     }
 
@@ -52,21 +55,28 @@ abstract class RoutingManager(private val caller: Any, val downstream: String, p
         }
     }
 
-    val started get() = active[downstream] === this
+    var started = false
+        private set
+    /**
+     * Thread safety: needs protection by companion object!
+     */
     private var routing: Routing? = null
 
-    fun start() = when (val other = active.putIfAbsent(downstream, this)) {
+    fun start() = synchronized(RoutingManager) {
-        null -> {
+        started = true
-            if (isWifi) WifiDoubleLock.acquire(this)
+        when (val other = active.putIfAbsent(downstream, this)) {
-            initRouting()
+            null -> {
+                if (isWifi) WifiDoubleLock.acquire(this)
+                initRoutingLocked()
+            }
+            this -> true    // already started
+            else -> error("Double routing detected for $downstream from $caller != ${other.caller}")
         }
-        this -> true    // already started
-        else -> error("Double routing detected for $downstream from $caller != ${other.caller}")
     }
 
     open fun ifaceHandler(iface: NetworkInterface) { }
 
-    private fun initRouting() = try {
+    private fun initRoutingLocked() = try {
         routing = Routing(caller, downstream, this::ifaceHandler).apply {
             try {
                 configure()
@@ -85,7 +95,8 @@ abstract class RoutingManager(private val caller: Any, val downstream: String, p
 
     protected abstract fun Routing.configure()
 
-    fun stop() {
+    fun stop() = synchronized(RoutingManager) {
+        started = false
         if (active.remove(downstream, this)) {
             if (isWifi) WifiDoubleLock.release(this)
             routing?.revert()

mobile/src/main/java/be/mygod/vpnhotspot/net/Routing.kt

@@ -331,6 +331,7 @@ class Routing(private val caller: Any, private val downstream: String,
         IpNeighbourMonitor.unregisterCallback(this)
         FallbackUpstreamMonitor.unregisterCallback(fallbackUpstream)
         UpstreamMonitor.unregisterCallback(upstream)
+        Timber.i("Stopped routing for $downstream by $caller")
     }
 
     fun commit() {
@@ -345,7 +346,6 @@ class Routing(private val caller: Any, private val downstream: String,
     }
     fun revert() {
         stop()
-        Timber.i("Stopped routing for $downstream by $caller")
         TrafficRecorder.update()    // record stats before exiting to prevent stats losing
         synchronized(this) { clients.values.forEach { it.close() } }
         currentDns?.transaction?.revert()