Merge branch 'refactor-packet-tunnel-to-use-only-a-single-network-pat…

…h-ios-401'
mullvad · Nov 29, 2023 · 5c8d672 · 5c8d672
2 parents c3ea6f4 + 18e21fa
commit 5c8d672
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 76 deletions.
diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift
@@ -61,7 +61,6 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
             eventQueue: internalQueue,
             pinger: Pinger(replyQueue: internalQueue),
             tunnelDeviceInfo: adapter,
-            defaultPathObserver: PacketTunnelPathObserver(packetTunnelProvider: self, eventQueue: internalQueue),
             timings: TunnelMonitorTimings()
         )
 

diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift
@@ -15,6 +15,8 @@ extension PacketTunnelActor {
      - Parameter notifyObserverWithCurrentPath: immediately notifies path observer with the current path when set to `true`.
      */
     func startDefaultPathObserver(notifyObserverWithCurrentPath: Bool = false) {
+        logger.trace("Start default path observer.")
+
         defaultPathObserver.start { [weak self] networkPath in
             self?.commandChannel.send(.networkReachability(networkPath))
         }
@@ -26,6 +28,8 @@ extension PacketTunnelActor {
 
     /// Stop observing changes to default path.
     func stopDefaultPathObserver() {
+        logger.trace("Stop default path observer.")
+
         defaultPathObserver.stop()
     }
 
@@ -35,6 +39,8 @@ extension PacketTunnelActor {
      - Parameter networkPath: new default path
      */
     func handleDefaultPathChange(_ networkPath: NetworkPath) {
+        tunnelMonitor.handleNetworkPathUpdate(networkPath)
+
         let newReachability = networkPath.networkReachability
 
         func mutateConnectionState(_ connState: inout ConnectionState) -> Bool {

diff --git a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift b/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift
@@ -10,6 +10,7 @@ import Foundation
 import MullvadLogging
 import MullvadTypes
 import Network
+import NetworkExtension
 
 /// Tunnel monitor.
 public final class TunnelMonitor: TunnelMonitorProtocol {
@@ -203,7 +204,6 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
     private let timings: TunnelMonitorTimings
 
     private var pinger: PingerProtocol
-    private var defaultPathObserver: DefaultPathObserverProtocol
     private var isObservingDefaultPath = false
     private var timer: DispatchSourceTimer?
 
@@ -230,12 +230,10 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
         eventQueue: DispatchQueue,
         pinger: PingerProtocol,
         tunnelDeviceInfo: TunnelDeviceInfoProtocol,
-        defaultPathObserver: DefaultPathObserverProtocol,
         timings: TunnelMonitorTimings
     ) {
         self.eventQueue = eventQueue
         self.tunnelDeviceInfo = tunnelDeviceInfo
-        self.defaultPathObserver = defaultPathObserver
 
         self.timings = timings
         state = State(timings: timings)
@@ -272,7 +270,7 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
         self.probeAddress = probeAddress
         state.connectionState = .pendingStart
 
-        addDefaultPathObserver()
+        startMonitoring()
     }
 
     public func stop() {
@@ -291,12 +289,8 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
         switch state.connectionState {
         case .connecting, .connected:
             startConnectivityCheckTimer()
-            addDefaultPathObserver()
 
-        case .waitingConnectivity, .pendingStart:
-            addDefaultPathObserver()
-
-        case .stopped, .recovering:
+        case .waitingConnectivity, .pendingStart, .stopped, .recovering:
             break
         }
     }
@@ -308,7 +302,40 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
         logger.trace("Prepare to sleep.")
 
         stopConnectivityCheckTimer()
-        removeDefaultPathObserver()
+    }
+
+    public func handleNetworkPathUpdate(_ networkPath: NetworkPath) {
+        nslock.withLock {
+            let pathStatus = networkPath.status
+            let isReachable = pathStatus == .satisfiable || pathStatus == .satisfied
+
+            switch state.connectionState {
+            case .pendingStart:
+                if isReachable {
+                    logger.debug("Start monitoring connection.")
+                    startMonitoring()
+                } else {
+                    logger.debug("Wait for network to become reachable before starting monitoring.")
+                    state.connectionState = .waitingConnectivity
+                }
+
+            case .waitingConnectivity:
+                guard isReachable else { return }
+
+                logger.debug("Network is reachable. Resume monitoring.")
+                startMonitoring()
+
+            case .connecting, .connected:
+                guard !isReachable else { return }
+
+                logger.debug("Network is unreachable. Pause monitoring.")
+                state.connectionState = .waitingConnectivity
+                stopMonitoring(resetRetryAttempt: true)
+
+            case .stopped, .recovering:
+                break
+            }
+        }
     }
 
     // MARK: - Private
@@ -324,42 +351,11 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
 
         probeAddress = nil
 
-        removeDefaultPathObserver()
         stopMonitoring(resetRetryAttempt: !forRestart)
 
         state.connectionState = .stopped
     }
 
-    private func addDefaultPathObserver() {
-        defaultPathObserver.stop()
-
-        logger.trace("Add default path observer.")
-
-        isObservingDefaultPath = true
-
-        defaultPathObserver.start { [weak self] nwPath in
-            guard let self else { return }
-
-            nslock.withLock {
-                self.handleNetworkPathUpdate(nwPath)
-            }
-        }
-
-        if let currentPath = defaultPathObserver.defaultPath {
-            handleNetworkPathUpdate(currentPath)
-        }
-    }
-
-    private func removeDefaultPathObserver() {
-        guard isObservingDefaultPath else { return }
-
-        logger.trace("Remove default path observer.")
-
-        defaultPathObserver.stop()
-
-        isObservingDefaultPath = false
-    }
-
     private func checkConnectivity() {
         nslock.lock()
         defer { nslock.unlock() }
@@ -429,7 +425,6 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
     #endif
 
     private func startConnectionRecovery() {
-        removeDefaultPathObserver()
         stopMonitoring(resetRetryAttempt: false)
 
         state.retryAttempt = state.retryAttempt.saturatingAddition(1)
@@ -450,38 +445,6 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
         }
     }
 
-    private func handleNetworkPathUpdate(_ networkPath: NetworkPath) {
-        let pathStatus = networkPath.status
-        let isReachable = pathStatus == .satisfiable || pathStatus == .satisfied
-
-        switch state.connectionState {
-        case .pendingStart:
-            if isReachable {
-                logger.debug("Start monitoring connection.")
-                startMonitoring()
-            } else {
-                logger.debug("Wait for network to become reachable before starting monitoring.")
-                state.connectionState = .waitingConnectivity
-            }
-
-        case .waitingConnectivity:
-            guard isReachable else { return }
-
-            logger.debug("Network is reachable. Resume monitoring.")
-            startMonitoring()
-
-        case .connecting, .connected:
-            guard !isReachable else { return }
-
-            logger.debug("Network is unreachable. Pause monitoring.")
-            state.connectionState = .waitingConnectivity
-            stopMonitoring(resetRetryAttempt: true)
-
-        case .stopped, .recovering:
-            break
-        }
-    }
-
     private func didReceivePing(from sender: IPAddress, sequenceNumber: UInt16) {
         nslock.lock()
         defer { nslock.unlock() }

diff --git a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitorProtocol.swift b/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitorProtocol.swift
@@ -38,4 +38,7 @@ public protocol TunnelMonitorProtocol: AnyObject {
     /// Cancels internal timers and time dependent data in preparation for device sleep.
     /// Call this method when packet tunnel provider receives a sleep event.
     func onSleep()
+
+    /// Handle changes in network path, eg. update connection state and monitoring.
+    func handleNetworkPathUpdate(_ networkPath: NetworkPath)
 }
diff --git a/ios/PacketTunnelCoreTests/Mocks/TunnelMonitorStub.swift b/ios/PacketTunnelCoreTests/Mocks/TunnelMonitorStub.swift
@@ -64,6 +64,8 @@ class TunnelMonitorStub: TunnelMonitorProtocol {
 
     func onSleep() {}
 
+    func handleNetworkPathUpdate(_ networkPath: NetworkPath) {}
+
     func dispatch(_ event: TunnelMonitorEvent, after delay: DispatchTimeInterval = .never) {
         if case .never = delay {
             onEvent?(event)

diff --git a/ios/PacketTunnelCoreTests/TunnelMonitorTests.swift b/ios/PacketTunnelCoreTests/TunnelMonitorTests.swift
@@ -117,7 +117,6 @@ extension TunnelMonitorTests {
             eventQueue: .main,
             pinger: pinger,
             tunnelDeviceInfo: TunnelDeviceInfoStub(networkStatsProviding: networkCounters),
-            defaultPathObserver: DefaultPathObserverFake(),
             timings: timings
         )
     }