Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #8235: Fix potential crash around changing VPN regions
Browse files Browse the repository at this point in the history
- Ensures the DispatchGroup enter's early so it is always balanced
- Fixes the region picker controller staying alive for the full 60s timeout regardless of success
  • Loading branch information
kylehickinson committed Oct 12, 2023
1 parent cd528a7 commit 6dad136
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 29 deletions.
2 changes: 1 addition & 1 deletion App/Client.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
65 changes: 37 additions & 28 deletions Sources/BraveVPN/BraveVPNRegionPickerViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -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
}
}

0 comments on commit 6dad136

Please sign in to comment.