Skip to content

Commit

Permalink
Improves the tunnel logic to better support on-demand (#505)
Browse files Browse the repository at this point in the history 
Task/Issue URL: https://app.asana.com/0/0/1205518995339759/f
iOS PR: duckduckgo/iOS#2016
macOS PR: duckduckgo/macos-browser#1631
What kind of version bump will this require?: Patch

## Description

Improves the tunnel logic to better support on-demand


* WIP

* WIP

* Removes some code that's not necessary

* Removes some code that's no longer needed

* Rolls back some unnecessary changes

* Removes some commented code

* Added some documentation comments

* Fixes the unit tests
  • Loading branch information
@diegoreymendez
diegoreymendez authored Sep 20, 2023
1 parent 781e68d commit af48c94
Show file tree
Hide file tree
Showing 7 changed files with 224 additions and 94 deletions.
114 changes: 67 additions & 47 deletions Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ final class NetworkProtectionConnectionTester {

enum TesterError: Error {
case couldNotFindInterface(named: String)
case connectionTestFailed
}

/// Provides a simple mechanism to synchronize an `isRunning` flag for the tester to know if it needs to interrupt its operation.
Expand Down Expand Up @@ -93,11 +94,11 @@ final class NetworkProtectionConnectionTester {
// MARK: - Test result handling

private var failureCount = 0
private let resultHandler: @MainActor (Result) -> Void
private let resultHandler: @MainActor (Result, Bool) -> Void

// MARK: - Init & deinit

init(timerQueue: DispatchQueue, log: OSLog, resultHandler: @escaping @MainActor (Result) -> Void) {
init(timerQueue: DispatchQueue, log: OSLog, resultHandler: @escaping @MainActor (Result, Bool) -> Void) {
self.timerQueue = timerQueue
self.log = log
self.resultHandler = resultHandler
Expand All @@ -113,40 +114,37 @@ final class NetworkProtectionConnectionTester {

// MARK: - Starting & Stopping the tester

func start(tunnelIfName: String) async throws {
func start(tunnelIfName: String, testImmediately: Bool) async throws {
guard await !timerRunCoordinator.isRunning else {
os_log("Will not start the connection tester as it's already running", log: log, type: .debug)
os_log("Will not start the connection tester as it's already running", log: log)
return
}

os_log("🟢 Starting connection tester", log: log, type: .debug)
os_log("🟢 Starting connection tester (testImmediately: %{public}@)", log: log, String(reflecting: testImmediately))
let tunnelInterface = try await networkInterface(forInterfaceNamed: tunnelIfName)
self.tunnelInterface = tunnelInterface

await scheduleTimer()
do {
try await scheduleTimer(testImmediately: testImmediately)
} catch {
os_log("🔴 Stopping connection tester early", log: log)
throw error
}
}

func stop() async {
os_log("🔴 Stopping connection tester", log: log, type: .debug)
os_log("🔴 Stopping connection tester", log: log)
await stopScheduledTimer()
}

/// Run the test right now and schedule the next one regularly.
///
func testImmediately() async {
await stopScheduledTimer()
testConnection()
await scheduleTimer()
}

// MARK: - Obtaining the interface

private func networkInterface(forInterfaceNamed interfaceName: String) async throws -> NWInterface {
try await withCheckedThrowingContinuation { continuation in
let monitor = NWPathMonitor()

monitor.pathUpdateHandler = { path in
os_log("All interfaces: %{public}@", log: self.log, type: .debug, String(describing: path.availableInterfaces))
os_log("All interfaces: %{public}@", log: self.log, String(describing: path.availableInterfaces))

guard let tunnelInterface = path.availableInterfaces.first(where: { $0.name == interfaceName }) else {
os_log("Could not find VPN interface %{public}@", log: self.log, type: .error, interfaceName)
Expand All @@ -169,23 +167,44 @@ final class NetworkProtectionConnectionTester {

// MARK: - Timer scheduling

private func scheduleTimer() async {
private func scheduleTimer(testImmediately: Bool) async throws {
await stopScheduledTimer()

await timerRunCoordinator.start()

if testImmediately {
do {
try await testConnection(isStartupTest: true)
} catch {
os_log("Rethrowing exception", log: log)
throw error
}
}

let timer = DispatchSource.makeTimerSource(queue: timerQueue)
self.timer = timer

timer.schedule(deadline: .now() + self.intervalBetweenTests, repeating: self.intervalBetweenTests)
timer.setEventHandler { [weak self] in
self?.testConnection()
guard let testConnection = self?.testConnection(isStartupTest:) else {
return
}

Task {
// During regular connection tests we don't care about the error thrown
// by this method, as it'll be handled through the result handler callback.
// The error we're ignoring here is only used when this class is initialized
// with an immediate test, to know whether the connection is up while the user
// still sees "Connecting..."
try? await testConnection(false)
}
}

timer.setCancelHandler { [weak self] in
self?.timer = nil
}

await timerRunCoordinator.start()
timer.resume()

self.timer = timer
}

private func stopScheduledTimer() async {
Expand All @@ -208,7 +227,7 @@ final class NetworkProtectionConnectionTester {

// MARK: - Testing the connection

func testConnection() {
func testConnection(isStartupTest: Bool) async throws {
guard let tunnelInterface = tunnelInterface else {
os_log("No interface to test!", log: log, type: .error)
return
Expand All @@ -222,29 +241,30 @@ final class NetworkProtectionConnectionTester {
let localParameters = NWParameters.tcp
localParameters.prohibitedInterfaces = [tunnelInterface]

Task {
// This is a bit ugly, but it's a quick way to run the tests in parallel without a task group.
async let vpnConnected = testConnection(name: "VPN", parameters: vpnParameters)
async let localConnected = testConnection(name: "Local", parameters: localParameters)
let vpnIsConnected = await vpnConnected
let localIsConnected = await localConnected
// This is a bit ugly, but it's a quick way to run the tests in parallel without a task group.
async let vpnConnected = testConnection(name: "VPN", parameters: vpnParameters)
async let localConnected = testConnection(name: "Local", parameters: localParameters)
let vpnIsConnected = await vpnConnected
let localIsConnected = await localConnected

let onlyVPNIsDown = !vpnIsConnected && localIsConnected
let onlyVPNIsDown = !vpnIsConnected && localIsConnected

// After completing the conection tests we check if the tester is still supposed to be running
// to avoid giving results when it should not be running.
guard await timerRunCoordinator.isRunning else {
os_log("Tester skipped returning results as it was stopped while running the tests", log: log, type: .info)
return
}
// After completing the conection tests we check if the tester is still supposed to be running
// to avoid giving results when it should not be running.
guard await timerRunCoordinator.isRunning else {
os_log("Tester skipped returning results as it was stopped while running the tests", log: log, type: .info)
return
}

if onlyVPNIsDown {
os_log("👎 VPN is DOWN", log: log, type: .debug)
await handleDisconnected()
} else {
os_log("👍 VPN: \(vpnIsConnected ? "UP" : "DOWN") local: \(localIsConnected ? "UP" : "DOWN")", log: log, type: .debug)
await handleConnected()
}
if onlyVPNIsDown {
os_log("👎 VPN is DOWN", log: log)
await handleDisconnected(isStartupTest: isStartupTest)
os_log("Throwing exception", log: log)
throw TesterError.connectionTestFailed
} else {
os_log("👍 VPN: \(vpnIsConnected ? "UP" : "DOWN") local: \(localIsConnected ? "UP" : "DOWN")", log: log)

await handleConnected(isStartupTest: isStartupTest)
}
}

Expand Down Expand Up @@ -276,19 +296,19 @@ final class NetworkProtectionConnectionTester {
// MARK: - Result handling

@MainActor
private func handleConnected() {
private func handleConnected(isStartupTest: Bool) {
if failureCount == 0 {
resultHandler(.connected)
resultHandler(.connected, isStartupTest)
} else if failureCount > 0 {
failureCount = 0

resultHandler(.reconnected)
resultHandler(.reconnected, isStartupTest)
}
}

@MainActor
private func handleDisconnected() {
private func handleDisconnected(isStartupTest: Bool) {
failureCount += 1
resultHandler(.disconnected(failureCount: failureCount))
resultHandler(.disconnected(failureCount: failureCount), isStartupTest)
}
}
Loading

0 comments on commit af48c94

Please sign in to comment.