Merge branch 'master' into r

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

@@ -394,7 +394,7 @@ class RepeaterService : Service(), CoroutineScope, WifiP2pManager.ChannelListene
             persistNextGroup = false
         }
         check(routingManager == null)
-        routingManager = object : RoutingManager.LocalOnly(this, group.`interface`!!) {
+        routingManager = object : RoutingManager.LocalOnly(this@RepeaterService, group.`interface`!!) {
             override fun ifaceHandler(iface: NetworkInterface) {
                 iface.hardwareAddress?.let { lastMac = MacAddressCompat.bytesToString(it) }
             }

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

@@ -25,10 +25,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()
@@ -37,7 +40,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()
         }
     }
 
@@ -53,29 +56,36 @@ 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
     private var isWifi = forceWifi || TetherType.ofInterface(downstream).isWifi
 
-    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)) {
-            if (!forceWifi && Build.VERSION.SDK_INT >= 30) TetherType.listener[this] = {
+            null -> {
-                val isWifiNow = TetherType.ofInterface(downstream).isWifi
+                if (isWifi) WifiDoubleLock.acquire(this)
-                if (isWifi != isWifiNow) {
+                if (!forceWifi && Build.VERSION.SDK_INT >= 30) TetherType.listener[this] = {
-                    if (isWifi) WifiDoubleLock.release(this) else WifiDoubleLock.acquire(this)
+                    val isWifiNow = TetherType.ofInterface(downstream).isWifi
-                    isWifi = isWifiNow
+                    if (isWifi != isWifiNow) {
+                        if (isWifi) WifiDoubleLock.release(this) else WifiDoubleLock.acquire(this)
+                        isWifi = isWifiNow
+                    }
                 }
+                initRoutingLocked()
             }
-            initRouting()
+            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()
@@ -94,7 +104,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 (!forceWifi && Build.VERSION.SDK_INT >= 30) TetherType.listener -= this
             if (isWifi) WifiDoubleLock.release(this)

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

@@ -129,6 +129,8 @@ class Routing(private val caller: Any, private val downstream: String,
     private val hostSubnet = "${hostAddress.address.hostAddress}/${hostAddress.networkPrefixLength}"
     private val transaction = RootSession.beginTransaction()
 
+    @Volatile
+    private var stopped = false
     private var masqueradeMode = MasqueradeMode.None
 
     private val upstreams = HashSet<String>()
@@ -164,6 +166,7 @@ class Routing(private val caller: Any, private val downstream: String,
         var dns: List<InetAddress> = emptyList()
 
         override fun onAvailable(ifname: String, properties: LinkProperties) = synchronized(this@Routing) {
+            if (stopped) return
             val subrouting = subrouting
             when {
                 subrouting != null -> check(subrouting.upstream == ifname) { "${subrouting.upstream} != $ifname" }
@@ -181,6 +184,7 @@ class Routing(private val caller: Any, private val downstream: String,
         }
 
         override fun onLost() = synchronized(this@Routing) {
+            if (stopped) return
             val subrouting = subrouting ?: return
             // we could be removing fallback subrouting which no collision could ever happen, check before removing
             subrouting.upstream?.let { check(upstreams.remove(it)) }
@@ -193,6 +197,7 @@ class Routing(private val caller: Any, private val downstream: String,
     private val fallbackUpstream = object : Upstream(RULE_PRIORITY_UPSTREAM_FALLBACK) {
         var fallbackInactive = true
         override fun onFallback() = synchronized(this@Routing) {
+            if (stopped) return
             fallbackInactive = false
             check(subrouting == null)
             subrouting = try {
@@ -231,6 +236,7 @@ class Routing(private val caller: Any, private val downstream: String,
     }
     private val clients = mutableMapOf<InetAddress, Client>()
     override fun onIpNeighbourAvailable(neighbours: Collection<IpNeighbour>) = synchronized(this) {
+        if (stopped) return
         val toRemove = HashSet(clients.keys)
         for (neighbour in neighbours) {
             if (neighbour.dev != downstream || neighbour.ip !is Inet4Address ||
@@ -328,9 +334,11 @@ class Routing(private val caller: Any, private val downstream: String,
     }
 
     fun stop() {
+        stopped = true
         IpNeighbourMonitor.unregisterCallback(this)
         FallbackUpstreamMonitor.unregisterCallback(fallbackUpstream)
         UpstreamMonitor.unregisterCallback(upstream)
+        Timber.i("Stopped routing for $downstream by $caller")
     }
 
     fun commit() {
@@ -345,7 +353,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()

mobile/src/main/java/be/mygod/vpnhotspot/net/monitor/IpMonitor.kt

@@ -70,7 +70,7 @@ abstract class IpMonitor : Runnable {
         }
         try {
             process.inputStream.bufferedReader().forEachLine {
-                if (errorMatcher.matches(it)) {
+                if (errorMatcher.containsMatchIn(it)) {
                     Timber.w(it)
                     process.destroy()   // move on to next mode
                 } else processLine(it)
@@ -119,7 +119,7 @@ abstract class IpMonitor : Runnable {
         }
         process.inputStream.bufferedReader().useLines {
             processLines(it.map { line ->
-                if (errorMatcher.matches(line)) throw IOException(line)
+                if (errorMatcher.containsMatchIn(line)) throw IOException(line)
                 line
             })
         }
@@ -134,15 +134,17 @@ abstract class IpMonitor : Runnable {
         }
         try {
             val command = "ip $monitoredObject"
-            RootSession.use {
+            RootSession.use { shell ->
-                val result = it.execQuiet(command)
+                val result = shell.execQuiet(command)
                 RootSession.checkOutput(command, result, false)
-                if (result.out.any { errorMatcher.matches(it) }) throw IOException(result.out.joinToString("\n"))
+                if (result.out.any { errorMatcher.containsMatchIn(it) }) {
+                    throw IOException(result.out.joinToString("\n"))
+                }
                 processLines(result.out.asSequence())
             }
         } catch (e: RuntimeException) {
-            app.logEvent("ip_su_poll_failure")
+            app.logEvent("ip_su_poll_failure") { param("cause", e.message.toString()) }
-            Timber.w(e)
+            Timber.d(e)
         }
     }