Fix tunnel hang on LTE -> WiFi handover (#5977)

* fix: restart the core on network change in all run modes

* Update CoreServiceManager.kt

---------

Co-authored-by: 2dust <31833384+2dust@users.noreply.github.com>

V2rayNG/app/src/main/java/com/v2ray/ang/contracts/ServiceControl.kt

@@ -1,6 +1,7 @@
 package com.v2ray.ang.contracts
 
 import android.app.Service
+import android.net.Network
 
 interface ServiceControl {
     /**
@@ -25,4 +26,13 @@ interface ServiceControl {
      * @return True if the socket is protected, false otherwise.
      */
     fun vpnProtect(socket: Int): Boolean
+
+    /**
+     * Declares the networks the tunnel runs on top of.
+     * Only meaningful for the VPN service, the other run modes have no interface to report.
+     *
+     * @param networks The upstream networks, null to let the system pick.
+     * @return True if the networks were accepted.
+     */
+    fun setUnderlyingNetworks(networks: Array<Network>?): Boolean = false
 }

V2rayNG/app/src/main/java/com/v2ray/ang/core/CoreServiceManager.kt

@@ -25,11 +25,13 @@ import com.v2ray.ang.handler.SpeedtestManager
 import com.v2ray.ang.helper.MessageHelper
 import com.v2ray.ang.service.DialerNativeService
 import com.v2ray.ang.service.DialerWebviewService
+import com.v2ray.ang.service.NetworkMonitor
 import com.v2ray.ang.util.LogUtil
 import com.v2ray.ang.util.Utils
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import kotlin.jvm.Volatile
 import libv2ray.CoreCallbackHandler
 import libv2ray.CoreController
 import libv2ray.ProcessFinder
@@ -43,6 +45,13 @@ object CoreServiceManager {
     private var currentConfig: ProfileItem? = null
     private var processFinder: XrayProcessFinder? = null
     private var browserDialer: IDialerService? = null
+    private var networkMonitor: NetworkMonitor? = null
+
+    @Volatile
+    private var isReloading = false
+
+    /** Tun descriptor the core was started with, null in the proxy only and root run modes. */
+    private var currentVpnInterface: ParcelFileDescriptor? = null
 
     var serviceControl: SoftReference<ServiceControl>? = null
         set(value) {
@@ -98,6 +107,19 @@ object CoreServiceManager {
 
     @Throws(Exception::class)
     private fun doStartCoreLoop(service: Service, vpnInterface: ParcelFileDescriptor?) {
+        val mFilter = IntentFilter(AppConfig.BROADCAST_ACTION_SERVICE)
+        mFilter.addAction(Intent.ACTION_SCREEN_ON)
+        mFilter.addAction(Intent.ACTION_SCREEN_OFF)
+        mFilter.addAction(Intent.ACTION_USER_PRESENT)
+        ContextCompat.registerReceiver(service, mMsgReceive, mFilter, Utils.receiverFlags())
+
+        currentVpnInterface = vpnInterface
+        launchCore(service, vpnInterface)
+        startNetworkMonitor(service)
+    }
+
+    @Throws(Exception::class)
+    private fun launchCore(service: Service, vpnInterface: ParcelFileDescriptor?, isReload: Boolean = false) {
         val guid = MmkvManager.getSelectServer() ?: error("No server selected")
         val config = MmkvManager.decodeServerConfig(guid) ?: error("Failed to decode server config")
 
@@ -108,12 +130,6 @@ object CoreServiceManager {
             error(result.errorMessage.ifBlank { "Failed to get V2Ray config" })
         }
 
-        val mFilter = IntentFilter(AppConfig.BROADCAST_ACTION_SERVICE)
-        mFilter.addAction(Intent.ACTION_SCREEN_ON)
-        mFilter.addAction(Intent.ACTION_SCREEN_OFF)
-        mFilter.addAction(Intent.ACTION_USER_PRESENT)
-        ContextCompat.registerReceiver(service, mMsgReceive, mFilter, Utils.receiverFlags())
-
         currentConfig = config
         var tunFd = vpnInterface?.fd ?: 0
         val dialerMode = BrowserDialerMode.from(config.browserDialerMode)
@@ -154,7 +170,9 @@ object CoreServiceManager {
             else -> {}
         }
 
-        MessageHelper.sendMsg2UI(service, AppConfig.MSG_STATE_START_SUCCESS, "")
+        if (!isReload) {
+            MessageHelper.sendMsg2UI(service, AppConfig.MSG_STATE_START_SUCCESS, "")
+        }
         NotificationManager.startSpeedNotification()
         LogUtil.i(AppConfig.TAG, "StartCore-Manager: Core started successfully")
     }
@@ -167,6 +185,10 @@ object CoreServiceManager {
     fun stopCoreLoop(): Boolean {
         val service = getService() ?: return false
 
+        networkMonitor?.unregister()
+        networkMonitor = null
+        currentVpnInterface = null
+
         if (isRunning()) {
             CoroutineScope(Dispatchers.IO).launch {
                 try {
@@ -196,11 +218,83 @@ object CoreServiceManager {
         return true
     }
 
+    /**
+     * Subscribes to upstream network changes for whichever run mode is active.
+     * All three services share this manager, so the tunnel recovers from a handover in proxy only
+     * and root mode as well, not just behind the VPN interface.
+     */
+    private fun startNetworkMonitor(service: Service) {
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) return
+        if (networkMonitor != null) return
+
+        val connectivity = service.getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager ?: return
+        networkMonitor = NetworkMonitor(
+            connectivity = connectivity,
+            onUnderlyingNetworksChanged = { networks -> serviceControl?.get()?.setUnderlyingNetworks(networks) },
+            onHandover = { reloadCore() },
+        ).also { it.register() }
+    }
+
+    /**
+     * Restarts the core in place after the upstream network changed: the service, the notification
+     * and the VPN interface all stay up, so nothing of this is visible.
+     *
+     * The config is rebuilt on purpose, outbound server domains are resolved while building it and
+     * an address resolved on a network that is gone can be unusable on the new one.
+     *
+     * @return True if the core is running again.
+     */
+    private fun reloadCore(): Boolean {
+        if (isReloading) return false
+        val service = getService() ?: return false
+        if (!isRunning()) return false
+
+        return try {
+            val tunFd = tunFdForCore()
+
+            isReloading = true
+            LogUtil.i(AppConfig.TAG, "StartCore-Manager: Core reload start...")
+
+            coreController.stopLoop()
+            launchCore(service, tunFd, isReload = true)
+
+            LogUtil.i(AppConfig.TAG, "StartCore-Manager: Core reload finished")
+            true
+        } catch (e: Exception) {
+            val message = e.message?.takeUnless { it.isBlank() } ?: e.javaClass.simpleName
+            LogUtil.e(AppConfig.TAG, "StartCore-Manager: Failed to reload core: $message", e)
+            MessageHelper.sendMsg2UI(service, AppConfig.MSG_STATE_START_FAILURE, message)
+            false
+        } finally {
+            isReloading = false
+        }
+    }
+
+    /**
+     * Returns the tun descriptor to hand to the core on a reload.
+     *
+     * With hev-socks5-tunnel the core is started without a tun and never touches the descriptor.
+     * Otherwise it closes the one it was given when it stops, which would take the VPN interface
+     * down, so it gets a duplicate to close instead.
+     */
+    private fun tunFdForCore(): ParcelFileDescriptor? {
+        val vpnInterface = currentVpnInterface ?: return null
+        return try {
+            if (SettingsManager.isUsingHevTun()) vpnInterface else vpnInterface.dup()
+        } catch (e: Exception) {
+            LogUtil.e(AppConfig.TAG, "StartCore-Manager: Failed to duplicate VPN interface", e)
+            throw e
+        }
+    }
+
     /**
      * Queries and resets all outbound traffic counters in one core call.
      * Go side format: tag,direction,value;tag,direction,value;
      */
     fun queryAllOutboundTrafficStats(): List<OutboundTrafficStat> {
+        // The stats manager is gone once the core stops, querying it then reaches into freed state.
+        if (!isRunning()) return emptyList()
+
         val payload = coreController.queryAllOutboundTrafficStats()
 
         val result = ArrayList<OutboundTrafficStat>()

V2rayNG/app/src/main/java/com/v2ray/ang/service/CoreVpnService.kt

@@ -5,16 +5,12 @@ import android.app.Service
 import android.content.Context
 import android.content.Intent
 import android.content.pm.PackageManager
-import android.net.ConnectivityManager
 import android.net.Network
-import android.net.NetworkCapabilities
-import android.net.NetworkRequest
 import android.net.ProxyInfo
 import android.net.VpnService
 import android.os.Build
 import android.os.ParcelFileDescriptor
 import android.os.StrictMode
-import androidx.annotation.RequiresApi
 import com.v2ray.ang.AppConfig
 import com.v2ray.ang.AppConfig.LOOPBACK
 import com.v2ray.ang.BuildConfig
@@ -38,43 +34,6 @@ class CoreVpnService : VpnService(), ServiceControl {
     private var tun2SocksService: Tun2SocksControl? = null
     private val isStartingLock = AtomicBoolean(false)
 
-    /**destroy
-     * Unfortunately registerDefaultNetworkCallback is going to return our VPN interface: https://android.googlesource.com/platform/frameworks/base/+/dda156ab0c5d66ad82bdcf76cda07cbc0a9c8a2e
-     *
-     * This makes doing a requestNetwork with REQUEST necessary so that we don't get ALL possible networks that
-     * satisfies default network capabilities but only THE default network. Unfortunately we need to have
-     * android.permission.CHANGE_NETWORK_STATE to be able to call requestNetwork.
-     *
-     * Source: https://android.googlesource.com/platform/frameworks/base/+/2df4c7d/services/core/java/com/android/server/ConnectivityService.java#887
-     */
-    @delegate:RequiresApi(Build.VERSION_CODES.P)
-    private val defaultNetworkRequest by lazy {
-        NetworkRequest.Builder()
-            .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
-            .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
-            .build()
-    }
-
-    private val connectivity by lazy { getSystemService(CONNECTIVITY_SERVICE) as ConnectivityManager }
-
-    @delegate:RequiresApi(Build.VERSION_CODES.P)
-    private val defaultNetworkCallback by lazy {
-        object : ConnectivityManager.NetworkCallback() {
-            override fun onAvailable(network: Network) {
-                setUnderlyingNetworks(arrayOf(network))
-            }
-
-            override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) {
-                // it's a good idea to refresh capabilities
-                setUnderlyingNetworks(arrayOf(network))
-            }
-
-            override fun onLost(network: Network) {
-                setUnderlyingNetworks(null)
-            }
-        }
-    }
-
     override fun onCreate() {
         super.onCreate()
         LogUtil.i(AppConfig.TAG, "StartCore-VPN: Service created")
@@ -165,6 +124,10 @@ class CoreVpnService : VpnService(), ServiceControl {
         return protect(socket)
     }
 
+    override fun setUnderlyingNetworks(networks: Array<Network>?): Boolean {
+        return super<VpnService>.setUnderlyingNetworks(networks)
+    }
+
     override fun attachBaseContext(newBase: Context?) {
         val context = newBase?.let {
             MyContextWrapper.wrap(newBase, SettingsManager.getLocale())
@@ -283,15 +246,6 @@ class CoreVpnService : VpnService(), ServiceControl {
      * @param builder The VPN Builder to configure
      */
     private fun configurePlatformFeatures(builder: Builder) {
-        // Android P (API 28) and above: Configure network callbacks
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
-            try {
-                connectivity.requestNetwork(defaultNetworkRequest, defaultNetworkCallback)
-            } catch (e: Exception) {
-                LogUtil.e(AppConfig.TAG, "StartCore-VPN: Failed to request network", e)
-            }
-        }
-
         // Android Q (API 29) and above: Configure metering and HTTP proxy
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
             builder.setMetered(false)
@@ -372,13 +326,6 @@ class CoreVpnService : VpnService(), ServiceControl {
 //        saveVpnNetworkInfo(configName, info)
         unlockStart()
         isRunning = false
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
-            try {
-                connectivity.unregisterNetworkCallback(defaultNetworkCallback)
-            } catch (e: Exception) {
-                LogUtil.w(AppConfig.TAG, "StartCore-VPN: Failed to unregister callback", e)
-            }
-        }
 
         tun2SocksService?.stopTun2Socks()
         tun2SocksService = null

V2rayNG/app/src/main/java/com/v2ray/ang/service/NetworkMonitor.kt

@@ -0,0 +1,119 @@
+package com.v2ray.ang.service
+
+import android.net.ConnectivityManager
+import android.net.Network
+import android.net.NetworkCapabilities
+import android.net.NetworkRequest
+import com.v2ray.ang.AppConfig
+import com.v2ray.ang.util.LogUtil
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
+
+/**
+ * Watches the network that carries the tunnel and reports topology changes.
+ *
+ * Cellular -> Wi-Fi is a make-before-break handover: the new network is announced while the old one
+ * is still connected, so the socket to the server is never reset and the core keeps using a dead
+ * connection. Deciding that a handover happened is what this class is for, acting on it is not.
+ *
+ * Only used from Android P and above, see CoreServiceManager.startNetworkMonitor().
+ * [onHandover] is invoked on a background thread after the debounce window and may block.
+ */
+class NetworkMonitor(
+    private val connectivity: ConnectivityManager,
+    private val onUnderlyingNetworksChanged: (Array<Network>?) -> Unit,
+    private val onHandover: () -> Unit,
+) {
+    private companion object {
+        const val HANDOVER_DEBOUNCE_MS = 1000L
+    }
+
+    private var upstream: Network? = null
+    private var handoverJob: Job? = null
+    private var registered = false
+
+    /**
+     * Unfortunately registerDefaultNetworkCallback is going to return our VPN interface:
+     * https://android.googlesource.com/platform/frameworks/base/+/dda156ab0c5d66ad82bdcf76cda07cbc0a9c8a2e
+     *
+     * This makes doing a requestNetwork with REQUEST necessary so that we don't get ALL possible networks that
+     * satisfies default network capabilities but only THE default network. Unfortunately we need to have
+     * android.permission.CHANGE_NETWORK_STATE to be able to call requestNetwork.
+     *
+     * Source: https://android.googlesource.com/platform/frameworks/base/+/2df4c7d/services/core/java/com/android/server/ConnectivityService.java#887
+     */
+    private val request by lazy {
+        NetworkRequest.Builder()
+            .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+            .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
+            .build()
+    }
+
+    private val callback = object : ConnectivityManager.NetworkCallback() {
+        override fun onAvailable(network: Network) {
+            val previous = upstream
+            upstream = network
+            onUnderlyingNetworksChanged(arrayOf(network))
+            if (previous != null && previous != network) {
+                scheduleHandover(network)
+            }
+        }
+
+        override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) {
+            // it's a good idea to refresh capabilities
+            onUnderlyingNetworksChanged(arrayOf(network))
+        }
+
+        override fun onLost(network: Network) {
+            onUnderlyingNetworksChanged(null)
+        }
+    }
+
+    /**
+     * Starts watching. Safe to call more than once, only the first call registers.
+     */
+    fun register() {
+        if (registered) return
+        try {
+            connectivity.requestNetwork(request, callback)
+            registered = true
+        } catch (e: Exception) {
+            LogUtil.e(AppConfig.TAG, "NetworkMonitor: Failed to request network", e)
+        }
+    }
+
+    /**
+     * Stops watching and drops the tracked state. Safe to call more than once.
+     */
+    fun unregister() {
+        handoverJob?.cancel()
+        handoverJob = null
+        upstream = null
+        if (!registered) return
+        registered = false
+        try {
+            connectivity.unregisterNetworkCallback(callback)
+        } catch (e: Exception) {
+            LogUtil.w(AppConfig.TAG, "NetworkMonitor: Failed to unregister callback", e)
+        }
+    }
+
+    private fun scheduleHandover(network: Network) {
+        LogUtil.i(AppConfig.TAG, "NetworkMonitor: Upstream is now $network")
+        handoverJob?.cancel()
+        handoverJob = CoroutineScope(Dispatchers.IO).launch {
+            try {
+                delay(HANDOVER_DEBOUNCE_MS)
+                onHandover()
+            } catch (e: CancellationException) {
+                throw e
+            } catch (e: Exception) {
+                LogUtil.e(AppConfig.TAG, "NetworkMonitor: Failed to handle upstream change", e)
+            }
+        }
+    }
+}