From 7468ae2099b9d00bcd471dd1a4bb5f8c0885ffd8 Mon Sep 17 00:00:00 2001 From: Kyle Hickinson Date: Mon, 16 Oct 2023 10:29:47 -0400 Subject: [PATCH] Fix #8235: Fix potential crash around changing VPN regions (#8237) --- App/Client.xcodeproj/project.pbxproj | 2 +- .../BraveVPNRegionPickerViewController.swift | 65 +++++++++++-------- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/App/Client.xcodeproj/project.pbxproj b/App/Client.xcodeproj/project.pbxproj index d085aa16888..ec74ed2b7a1 100644 --- a/App/Client.xcodeproj/project.pbxproj +++ b/App/Client.xcodeproj/project.pbxproj @@ -1508,7 +1508,7 @@ CLANG_WARN_DOCUMENTATION_COMMENTS = YES; CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER = YES; CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE; - CODE_SIGN_ENTITLEMENTS = BrowserIntents/Entitlements/BrowserIntentsDebug.entitlements; + CODE_SIGN_ENTITLEMENTS = "BrowserIntents/Entitlements/BrowserIntentsRelease (AppStore).entitlements"; COPY_PHASE_STRIP = NO; DEBUG_INFORMATION_FORMAT = dwarf; ENABLE_TESTABILITY = YES; diff --git a/Sources/BraveVPN/BraveVPNRegionPickerViewController.swift b/Sources/BraveVPN/BraveVPNRegionPickerViewController.swift index 8ad2985564a..f1ccb810ef7 100644 --- a/Sources/BraveVPN/BraveVPNRegionPickerViewController.swift +++ b/Sources/BraveVPN/BraveVPNRegionPickerViewController.swift @@ -20,6 +20,7 @@ public class BraveVPNRegionPickerViewController: BraveVPNPickerViewController { /// This group monitors vpn connection status. private var dispatchGroup: DispatchGroup? + private var timeoutTimer: Timer? private var vpnRegionChangeSuccess = false public override init() { @@ -45,11 +46,16 @@ public class BraveVPNRegionPickerViewController: BraveVPNPickerViewController { guard let connection = notification.object as? NEVPNConnection else { return } if connection.status == .connected { + vpnRegionChangeSuccess = true + timeoutTimer?.invalidate() dispatchGroup?.leave() - self.vpnRegionChangeSuccess = true dispatchGroup = nil } } + + deinit { + timeoutTimer?.invalidate() + } } // MARK: - UITableView Data Source & Delegate @@ -116,40 +122,43 @@ extension BraveVPNRegionPickerViewController: UITableViewDelegate, UITableViewDa let newRegion = indexPath.section == Section.automatic.rawValue ? nil : region self.dispatchGroup = DispatchGroup() - + // Changing vpn server settings takes lot of time, + // and nothing we can do about it as it relies on Apple apis. + // Here we observe vpn status and we show success alert if it connected, + // otherwise an error alert is show if it did not manage to connect in 60 seconds. + self.dispatchGroup?.enter() + BraveVPN.changeVPNRegion(to: newRegion) { [weak self] success in guard let self = self else { return } if !success { - self.showErrorAlert(title: Strings.VPN.regionPickerErrorTitle, - message: Strings.VPN.regionPickerErrorMessage) - } - - // Changing vpn server settings takes lot of time, - // and nothing we can do about it as it relies on Apple apis. - // Here we observe vpn status and we show success alert if it connected, - // otherwise an error alert is show if it did not manage to connect in 60 seconds. - self.dispatchGroup?.enter() - - DispatchQueue.main.asyncAfter(deadline: .now() + 60) { - self.vpnRegionChangeSuccess = false - self.dispatchGroup?.leave() - self.dispatchGroup = nil + self.markRegionChangeFailed() + } else { + // Start a timeout timer + self.timeoutTimer = Timer.scheduledTimer(withTimeInterval: 60, repeats: false, block: { [weak self] _ in + guard let self = self else { return } + self.markRegionChangeFailed() + }) } - - self.dispatchGroup?.notify(queue: .main) { [weak self] in - guard let self = self else { return } - if self.vpnRegionChangeSuccess { - - self.dismiss(animated: true) { - self.showSuccessAlert(text: Strings.VPN.regionSwitchSuccessPopupText) - BraveVPN.fetchLastUsedRegionDetail() - } - } else { - self.showErrorAlert(title: Strings.VPN.regionPickerErrorTitle, - message: Strings.VPN.regionPickerErrorMessage) + } + + dispatchGroup?.notify(queue: .main) { [weak self] in + guard let self = self else { return } + if self.vpnRegionChangeSuccess { + self.dismiss(animated: true) { + self.showSuccessAlert(text: Strings.VPN.regionSwitchSuccessPopupText) + BraveVPN.fetchLastUsedRegionDetail() } + } else { + self.showErrorAlert(title: Strings.VPN.regionPickerErrorTitle, + message: Strings.VPN.regionPickerErrorMessage) } } } + + private func markRegionChangeFailed() { + vpnRegionChangeSuccess = false + dispatchGroup?.leave() + dispatchGroup = nil + } }