From 18e21fa49287f17fadc0b77093db4452659043f7 Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Tue, 28 Nov 2023 16:16:16 +0100 Subject: [PATCH] Refactor packet tunnel to use only a single network path observer --- .../PacketTunnelProvider.swift | 1 - ...acketTunnelActor+NetworkReachability.swift | 6 + .../TunnelMonitor/TunnelMonitor.swift | 111 ++++++------------ .../TunnelMonitor/TunnelMonitorProtocol.swift | 3 + .../Mocks/TunnelMonitorStub.swift | 2 + .../TunnelMonitorTests.swift | 1 - 6 files changed, 48 insertions(+), 76 deletions(-) diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift index 96d657ce48af..76e7eede9d23 100644 --- 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 index 34a4e02ed154..2d3b600239ae 100644 --- 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 index 16ab1d06989b..55facd9048c7 100644 --- 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 index bae8250d27a3..0a47d01fb716 100644 --- 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 index 4857b03ae2b7..60609fee36e7 100644 --- 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 index b742ea117f6c..dfe55f86d7c5 100644 --- 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 ) }