passepartoutvpn/passepartout-apple
1
0
Fork 0
mirror of https://github.com/passepartoutvpn/passepartout-apple.git synced 2025-01-31 04:52:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address UI race conditions (#229)

Browse Source 
* Make some managers concurrency-safe

- IntentsManager: @MainActor, non-shared, continuation

- SSIDReader: @MainActor, continuation

- Reviewer: main queue, non-shared

* Review wrong use of Concurrency framework

There were background thread calls e.g. in VPNToggle, because
ProfileManager was used inside a VPNManager async call.

Annotate @MainActor wherever a Task involves UI.

* Make main managers MainActor

* Apply MainActor to Mac menus

* [ci skip] Update CHANGELOG

* Set MainActor consistently on Mac menu view models
This commit is contained in:
Davide De Rosa 2022-10-13 08:53:50 +02:00 committed by GitHub
parent 54dc8a2556
commit 5627e6c4a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
43 changed files with 138 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -18,7 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Oeck provider is available again to free users.
- Randomic crashes on profile updates.
- Randomic crashes on profile updates. [#229](https://github.com/passepartoutvpn/passepartout-apple/pull/229)
## 2.0.0 (2022-10-02)

8
Passepartout/App/Context/AppContext+Shared.swift
View File

@ -32,11 +32,3 @@ extension AppContext {
extension ProductManager {
static let shared = AppContext.shared.productManager
}
extension IntentsManager {
static let shared = AppContext.shared.intentsManager
}
extension Reviewer {
static let shared = AppContext.shared.reviewer
}

24
Passepartout/App/Context/AppContext.swift
View File

@ -27,14 +27,13 @@ import Foundation
import Combine
import PassepartoutLibrary
@MainActor
class AppContext {
private let logManager: LogManager
let productManager: ProductManager
private let reviewer: Reviewer
let intentsManager: IntentsManager
let reviewer: Reviewer
let productManager: ProductManager
private var cancellables: Set<AnyCancellable> = []
@ -45,32 +44,33 @@ class AppContext {
logManager.configureLogging()
pp_log.info("Logging to: \(logManager.logFile!)")
reviewer = Reviewer()
reviewer.eventCountBeforeRating = Constants.Rating.eventCount
productManager = ProductManager(
appType: Constants.InApp.appType,
buildProducts: Constants.InApp.buildProducts
)
intentsManager = IntentsManager()
reviewer = Reviewer()
reviewer.eventCountBeforeRating = Constants.Rating.eventCount
// post
configureObjects(coreContext: coreContext)
}
private func configureObjects(coreContext: CoreContext) {
coreContext.vpnManager.isOnDemandRulesSupported = {
self.isEligibleForOnDemandRules()
}
coreContext.vpnManager.currentState.$vpnStatus
.removeDuplicates()
.receive(on: DispatchQueue.main)
.sink {
if $0 == .connected {
pp_log.info("VPN successful connection, report to Reviewer")
self.reviewer.reportEvent()
}
}.store(in: &cancellables)
coreContext.vpnManager.isOnDemandRulesSupported = {
self.isEligibleForOnDemandRules()
}
}
// eligibility: ignore network settings if ineligible

33
Passepartout/App/Intents/IntentsManager.swift
View File

@ -28,6 +28,7 @@ import Intents
import IntentsUI
import Combine
@MainActor
class IntentsManager: NSObject, ObservableObject {
@Published private(set) var isReloadingShortcuts = false
@ -35,28 +36,26 @@ class IntentsManager: NSObject, ObservableObject {
let shouldDismissIntentView = PassthroughSubject<Void, Never>()
private var continuation: CheckedContinuation<[INVoiceShortcut], Never>?
override init() {
super.init()
reloadShortcuts()
Task {
await reloadShortcuts()
}
}
func reloadShortcuts() {
func reloadShortcuts() async {
isReloadingShortcuts = true
INVoiceShortcutCenter.shared.getAllVoiceShortcuts { vs, error in
if let error = error {
assertionFailure("Unable to fetch existing shortcuts: \(error)")
DispatchQueue.main.async {
self.isReloadingShortcuts = false
}
return
}
let shortcuts = (vs ?? []).reduce(into: [UUID: Shortcut]()) {
do {
let vs = try await INVoiceShortcutCenter.shared.allVoiceShortcuts()
shortcuts = vs.reduce(into: [UUID: Shortcut]()) {
$0[$1.identifier] = Shortcut($1)
}
DispatchQueue.main.async {
self.shortcuts = shortcuts
self.isReloadingShortcuts = false
}
isReloadingShortcuts = false
} catch {
assertionFailure("Unable to fetch existing shortcuts: \(error)")
isReloadingShortcuts = false
}
}
}
@ -93,9 +92,7 @@ extension IntentsManager: INUIEditVoiceShortcutViewControllerDelegate {
// so damn it, reload manually after a delay
Task {
await Task.maybeWait(forMilliseconds: Constants.Delays.xxxReloadEditedShortcut)
await MainActor.run {
reloadShortcuts()
}
await reloadShortcuts()
}
}

1
Passepartout/App/Mac/MacBundle.swift
View File

@ -32,6 +32,7 @@ class MacBundle {
private lazy var bridgeDelegate = MacBundleDelegate(bundle: self)
@MainActor
func configure() {
guard let bundleURL = Bundle.main.builtInPlugInsURL?.appendingPathComponent(Constants.Plugins.macBridgeName) else {
fatalError("Unable to find Mac bundle in plugins")

3
Passepartout/App/Mac/MacBundleDelegate.swift
View File

@ -28,14 +28,17 @@ import Foundation
class MacBundleDelegate: MacMenuDelegate {
private weak var bundle: MacBundle?
@MainActor
var profileManager: LightProfileManager {
DefaultLightProfileManager()
}
@MainActor
var providerManager: LightProviderManager {
DefaultLightProviderManager()
}
@MainActor
var vpnManager: LightVPNManager {
DefaultLightVPNManager()
}

1
Passepartout/App/Mac/Models/DefaultLightProviderManager.swift
View File

@ -103,7 +103,6 @@ class DefaultLightProviderManager: LightProviderManager {
.map(DefaultLightProviderCategory.init)
}
@MainActor
func downloadIfNeeded(_ name: String, vpnProtocol: String) {
guard let vpnProtocolType = VPNProtocolType(rawValue: vpnProtocol) else {
fatalError("Unrecognized VPN protocol: \(vpnProtocol)")

5
Passepartout/App/Mac/Models/DefaultLightVPNManager.swift
View File

@ -64,28 +64,24 @@ class DefaultLightVPNManager: LightVPNManager {
}.store(in: &subscriptions)
}
@MainActor
func connect(with profileId: UUID) {
Task {
try? await vpnManager.connect(with: profileId)
}
}
@MainActor
func connect(with profileId: UUID, to serverId: String) {
Task {
try? await vpnManager.connect(with: profileId, toServer: serverId)
}
}
@MainActor
func disconnect() {
Task {
await vpnManager.disable()
}
}
@MainActor
func toggle() {
Task {
if !isEnabled {
@ -96,7 +92,6 @@ class DefaultLightVPNManager: LightVPNManager {
}
}
@MainActor
func reconnect() {
Task {
await vpnManager.reconnect()

2
Passepartout/App/Reusable/GenericCreditsView.swift
View File

@ -159,7 +159,7 @@ extension GenericCreditsView {
guard content == nil else {
return
}
Task {
Task { @MainActor in
withAnimation {
do {
content = try String(contentsOf: url)

2
Passepartout/App/Views/AddHostViewModel.swift
View File

@ -52,6 +52,7 @@ extension AddHostView {
profileName = url.normalizedFilename
}
@MainActor
mutating func processURL(
_ url: URL,
with profileManager: ProfileManager,
@ -96,6 +97,7 @@ extension AddHostView {
}
}
@MainActor
mutating func addProcessedProfile(to profileManager: ProfileManager) -> Bool {
guard !processedProfile.isPlaceholder else {
assertionFailure("Saving profile without processing first?")

6
Passepartout/App/Views/AddProfileMenu.swift
View File

@ -58,9 +58,7 @@ struct AddProfileMenu: View {
} label: {
Label(L10n.Global.Strings.provider, systemImage: themeProviderImage)
}
Button {
presentHostFileImporter()
} label: {
Button(action: presentHostFileImporter) {
Label(L10n.Menu.Contextual.AddProfile.fromFiles, systemImage: themeHostFilesImage)
}
// Button {
@ -146,7 +144,7 @@ extension AddProfileMenu {
//
// https://stackoverflow.com/questions/66965471/swiftui-fileimporter-modifier-not-updating-binding-when-dismissed-by-tapping
isHostFileImporterPresented = false
Task {
Task { @MainActor in
await Task.maybeWait(forMilliseconds: Constants.Delays.xxxPresentFileImporter)
isHostFileImporterPresented = true
}

13
Passepartout/App/Views/AddProviderViewModel.swift
View File

@ -27,8 +27,6 @@ import Foundation
import PassepartoutLibrary
extension AddProviderView {
@MainActor
class ViewModel: ObservableObject {
enum PendingOperation {
case index
@ -75,18 +73,16 @@ extension AddProviderView {
metadata.name,
vpnProtocol: selectedVPNProtocol
) else {
Task {
await selectProviderAfterFetchingInfrastructure(metadata, providerManager)
}
selectProviderAfterFetchingInfrastructure(metadata, providerManager)
return
}
doSelectProvider(metadata, server)
}
private func selectProviderAfterFetchingInfrastructure(_ metadata: ProviderMetadata, _ providerManager: ProviderManager) async {
private func selectProviderAfterFetchingInfrastructure(_ metadata: ProviderMetadata, _ providerManager: ProviderManager) {
errorMessage = nil
pendingOperation = .provider(metadata.name)
Task {
Task { @MainActor in
do {
try await providerManager.fetchProviderPublisher(
withName: metadata.name,
@ -117,7 +113,7 @@ extension AddProviderView {
func updateIndex(_ providerManager: ProviderManager) {
errorMessage = nil
pendingOperation = .index
Task {
Task { @MainActor in
do {
try await providerManager.fetchProvidersIndexPublisher(
priority: .remoteThenBundle
@ -153,6 +149,7 @@ extension AddProviderView.NameView {
profileName = metadata.fullName
}
@MainActor
mutating func addProfile(
_ profile: Profile,
to profileManager: ProfileManager,

15
Passepartout/App/Views/OnDemandView+SSID.swift
View File

@ -34,11 +34,7 @@ extension OnDemandView {
var body: some View {
EditableTextList(elements: allSSIDs, allowsDuplicates: false, mapping: mapElements) { text in
reader.requestCurrentSSID {
if !withSSIDs.keys.contains($0) {
text.wrappedValue = $0
}
}
requestSSID(text)
} textField: {
ssidRow(callback: $0)
} addLabel: {
@ -74,6 +70,15 @@ extension OnDemandView {
onCommit: callback.onCommit
).themeValidSSID(callback.text.wrappedValue)
}
private func requestSSID(_ text: Binding<String>) {
Task { @MainActor in
let ssid = try await reader.requestCurrentSSID()
if !withSSIDs.keys.contains(ssid) {
text.wrappedValue = ssid
}
}
}
}
}

7
Passepartout/App/Views/OrganizerView+Profiles.swift
View File

@ -41,9 +41,8 @@ extension OrganizerView {
if !profileManager.hasProfiles {
emptyView
}
}.onAppear {
performMigrationsIfNeeded()
}.onReceive(profileManager.didCreateProfile) {
}.onAppear(perform: performMigrationsIfNeeded)
.onReceive(profileManager.didCreateProfile) {
profileManager.currentProfileId = $0.id
}
}
@ -141,7 +140,7 @@ extension OrganizerView {
}
private func performMigrationsIfNeeded() {
Task {
Task { @MainActor in
UpgradeManager.shared.doMigrations(profileManager)
}
}

2
Passepartout/App/Views/OrganizerView+Scene.swift
View File

@ -87,7 +87,7 @@ extension OrganizerView {
switch phase {
case .active:
if productManager.hasRefunded() {
Task {
Task { @MainActor in
await vpnManager.uninstall()
}
}

2
Passepartout/App/Views/OrganizerView.swift
View File

@ -102,7 +102,7 @@ extension OrganizerView {
assertionFailure("Empty URLs from file importer?")
return
}
Task {
Task { @MainActor in
await Task.maybeWait(forMilliseconds: Constants.Delays.xxxPresentFileImporter)
addProfileModalType = .addHost(url, false)
}

2
Passepartout/App/Views/ProfileView+MainMenu.swift
View File

@ -138,7 +138,7 @@ extension ProfileView {
}
private func uninstallVPN() {
Task {
Task { @MainActor in
await vpnManager.uninstall()
}
}

2
Passepartout/App/Views/ProfileView+Provider.swift
View File

@ -127,7 +127,7 @@ extension ProfileView {
private func refreshInfrastructure() {
isRefreshingInfrastructure = true
Task {
Task { @MainActor in
try await providerManager.fetchRemoteProviderPublisher(forProfile: profile).async()
isRefreshingInfrastructure = false
}

9
Passepartout/App/Views/ShortcutsView.swift
View File

@ -43,7 +43,7 @@ struct ShortcutsView: View {
}
}
@ObservedObject private var intentsManager: IntentsManager
@StateObject private var intentsManager = IntentsManager()
@Environment(\.presentationMode) private var presentationMode
@ -56,7 +56,6 @@ struct ShortcutsView: View {
@State private var pendingShortcut: INShortcut?
init(target: Profile) {
intentsManager = .shared
self.target = target
}
@ -69,11 +68,7 @@ struct ShortcutsView: View {
}.toolbar {
themeCloseItem(presentationMode: presentationMode)
}.sheet(item: $modalType, content: presentedModal)
// reloading
.onAppear {
intentsManager.reloadShortcuts()
}.themeAnimation(on: intentsManager.isReloadingShortcuts)
.themeAnimation(on: intentsManager.isReloadingShortcuts)
// IntentsUI
.onReceive(intentsManager.shouldDismissIntentView) { _ in

9
Passepartout/App/Views/VPNToggle.swift
View File

@ -77,11 +77,10 @@ struct VPNToggle: View {
}
private func enableVPN() {
Task {
Task { @MainActor in
canToggle = false
DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(rateLimit)) {
canToggle = true
}
await Task.maybeWait(forMilliseconds: rateLimit)
canToggle = true
do {
let profile = try await vpnManager.connect(with: profileId)
donateIntents(withProfile: profile)
@ -93,7 +92,7 @@ struct VPNToggle: View {
}
private func disableVPN() {
Task {
Task { @MainActor in
canToggle = false
await vpnManager.disable()
canToggle = true

4
Passepartout/AppShared/Context/CoreContext+Shared.swift
View File

@ -27,8 +27,6 @@ import Foundation
import PassepartoutLibrary
extension CoreContext {
@MainActor
static let shared = CoreContext(store: UserDefaultsStore(defaults: .standard))
}
@ -49,5 +47,7 @@ extension VPNManager {
}
extension ObservableVPNState {
@MainActor
static let shared = CoreContext.shared.vpnManager.currentState
}

2
Passepartout/AppShared/Context/CoreContext.swift
View File

@ -27,6 +27,7 @@ import Foundation
import Combine
import PassepartoutLibrary
@MainActor
class CoreContext {
let store: KeyValueStore
@ -52,7 +53,6 @@ class CoreContext {
private var cancellables: Set<AnyCancellable> = []
@MainActor
init(store: KeyValueStore) {
self.store = store

1
Passepartout/AppShared/Mac/MacMenu.swift
View File

@ -25,6 +25,7 @@
import Foundation
@MainActor
@objc
public protocol MacMenu {
var delegate: MacMenuDelegate? { get set }

1
Passepartout/AppShared/Mac/Models/LightProfileManager.swift
View File

@ -46,6 +46,7 @@ extension LightProfile {
}
}
@MainActor
@objc
public protocol LightProfileManager {
var hasProfiles: Bool { get }

1
Passepartout/AppShared/Mac/Models/LightProviderManager.swift
View File

@ -56,6 +56,7 @@ public protocol LightProviderServer {
var serverId: String { get }
}
@MainActor
@objc
public protocol LightProviderManager {
var delegate: LightProviderManagerDelegate? { get set }

1
Passepartout/AppShared/Mac/Models/LightVPNManager.swift
View File

@ -36,6 +36,7 @@ public enum LightVPNStatus: Int {
case disconnected
}
@MainActor
@objc
public protocol LightVPNManager {
var isEnabled: Bool { get }

6
Passepartout/Mac/Menu/HostProfileItem+ViewModel.swift
View File

@ -26,6 +26,8 @@
import Foundation
extension HostProfileItem {
@MainActor
class ViewModel {
let profile: LightProfile
@ -41,7 +43,9 @@ extension HostProfileItem {
}
deinit {
vpnManager.removeDelegate(withIdentifier: profile.id.uuidString)
Task { @MainActor in
vpnManager.removeDelegate(withIdentifier: profile.id.uuidString)
}
}
@objc func connectTo() {

2
Passepartout/Mac/Menu/LaunchOnLoginItem+ViewModel.swift
View File

@ -28,6 +28,8 @@ import Combine
import ServiceManagement
extension LaunchOnLoginItem {
@MainActor
class ViewModel: ObservableObject {
let title: String

6
Passepartout/Mac/Menu/PassepartoutMenu+StatusButton.swift
View File

@ -27,6 +27,8 @@ import Foundation
import AppKit
extension PassepartoutMenu {
@MainActor
class StatusButton {
private lazy var statusItem = NSStatusBar.system.statusItem(withLength: NSStatusItem.variableLength)
@ -50,7 +52,9 @@ extension PassepartoutMenu {
}
deinit {
vpnManager.removeDelegate(withIdentifier: "PassepartoutMenu")
Task { @MainActor in
vpnManager.removeDelegate(withIdentifier: "PassepartoutMenu")
}
}
func install(systemMenu: SystemMenu) {

1
Passepartout/Mac/Menu/PassepartoutMenu.swift
View File

@ -26,6 +26,7 @@
import Foundation
import AppKit
@MainActor
class PassepartoutMenu {
private let macMenuDelegate: MacMenuDelegate

2
Passepartout/Mac/Menu/ProviderLocationItem+ViewModel.swift
View File

@ -26,6 +26,8 @@
import Foundation
extension ProviderLocationItem {
@MainActor
class ViewModel {
private let profile: LightProfile

6
Passepartout/Mac/Menu/ProviderProfileItem+ViewModel.swift
View File

@ -26,6 +26,8 @@
import Foundation
extension ProviderProfileItem {
@MainActor
class ViewModel {
let profile: LightProfile
@ -44,7 +46,9 @@ extension ProviderProfileItem {
}
deinit {
vpnManager.removeDelegate(withIdentifier: profile.id.uuidString)
Task { @MainActor in
vpnManager.removeDelegate(withIdentifier: profile.id.uuidString)
}
}
private var providerName: String {

2
Passepartout/Mac/Menu/ProviderServerItem+ViewModel.swift
View File

@ -26,6 +26,8 @@
import Foundation
extension ProviderServerItem {
@MainActor
class ViewModel {
private let profile: LightProfile

6
Passepartout/Mac/Menu/VPNItemGroup+ViewModel.swift
View File

@ -27,6 +27,8 @@ import Foundation
import Combine
extension VPNItemGroup {
@MainActor
class ViewModel {
private let vpnManager: LightVPNManager
@ -51,7 +53,9 @@ extension VPNItemGroup {
}
deinit {
vpnManager.removeDelegate(withIdentifier: "VPNItemGroup")
Task { @MainActor in
vpnManager.removeDelegate(withIdentifier: "VPNItemGroup")
}
}
var toggleTitle: String {

2
Passepartout/Mac/Menu/VisibilityItem+ViewModel.swift
View File

@ -27,6 +27,8 @@ import Foundation
import AppKit
extension VisibilityItem {
@MainActor
class ViewModel {
private let transformer: ObservableProcessTransformer

1
Passepartout/Mac/PassepartoutMac.swift
View File

@ -32,5 +32,6 @@ class PassepartoutMac: NSObject, MacBridge {
let utils: MacUtils = DefaultMacUtils()
@MainActor
let menu: MacMenu = DefaultMacMenu()
}

1
Passepartout/Mac/Reusable/ItemGroup.swift
View File

@ -26,6 +26,7 @@
import Foundation
import AppKit
@MainActor
protocol ItemGroup {
func asMenuItems(withParent parent: NSMenu) -> [NSMenuItem]
}

1
Passepartout/Mac/Reusable/SystemMenu.swift
View File

@ -26,6 +26,7 @@
import Foundation
import AppKit
@MainActor
protocol SystemMenu {
var asMenu: NSMenu { get }
}

1
PassepartoutLibrary/Sources/PassepartoutLibrary/Managers/UpgradeManager.swift
View File

@ -29,6 +29,7 @@ import SwiftyBeaver
import PassepartoutCore
import PassepartoutUtils
@MainActor
public final class UpgradeManager: ObservableObject {
// MARK: Initialization

7
PassepartoutLibrary/Sources/PassepartoutProfiles/Managers/ProfileManager.swift
View File

@ -30,6 +30,7 @@ import PassepartoutCore
import PassepartoutUtils
import PassepartoutProviders
@MainActor
public final class ProfileManager: ObservableObject {
public typealias ProfileEx = (profile: Profile, isReady: Bool)
@ -292,10 +293,8 @@ extension ProfileManager {
currentProfile.isLoading = true
Task {
try await makeProfileReady(profile)
await MainActor.run {
currentProfile.value = profile
currentProfile.isLoading = false
}
currentProfile.value = profile
currentProfile.isLoading = false
}
}
}

56
PassepartoutLibrary/Sources/PassepartoutUtils/Reusable/SSIDReader.swift
View File

@ -25,48 +25,50 @@
import Foundation
import CoreLocation
import Combine
public class SSIDReader: NSObject, ObservableObject, CLLocationManagerDelegate {
@MainActor
public class SSIDReader: NSObject, ObservableObject {
private let manager = CLLocationManager()
private let publisher = PassthroughSubject<String, Never>()
private var cancellables: Set<AnyCancellable> = []
public func requestCurrentSSID(onSSID: @escaping (String) -> Void) {
publisher
.sink(receiveValue: onSSID)
.store(in: &cancellables)
private var continuation: CheckedContinuation<String, Error>?
private func currentSSID() async -> String {
await Utils.currentWifiSSID() ?? ""
}
public func requestCurrentSSID() async throws -> String {
switch manager.authorizationStatus {
case .authorizedAlways, .authorizedWhenInUse, .denied:
notifyCurrentSSID()
return
return await currentSSID()
default:
manager.delegate = self
manager.requestWhenInUseAuthorization()
}
}
private func notifyCurrentSSID() {
Task {
let currentSSID = await Utils.currentWifiSSID() ?? ""
await MainActor.run {
publisher.send(currentSSID)
cancellables.removeAll()
return try await withCheckedThrowingContinuation { continuation in
self.continuation = continuation
manager.delegate = self
manager.requestWhenInUseAuthorization()
}
}
}
}
extension SSIDReader: CLLocationManagerDelegate {
public func locationManagerDidChangeAuthorization(_ manager: CLLocationManager) {
switch manager.authorizationStatus {
case .authorizedWhenInUse, .authorizedAlways, .denied:
notifyCurrentSSID()
Task {
continuation?.resume(returning: await currentSSID())
continuation = nil
}
default:
cancellables.removeAll()
continuation?.resume(with: .success(""))
continuation = nil
}
}
public func locationManager(_ manager: CLLocationManager, didFailWithError error: Error) {
continuation?.resume(throwing: error)
continuation = nil
}
}

1
PassepartoutLibrary/Sources/PassepartoutVPN/Managers/TunnelKitVPNManagerStrategy.swift
View File

@ -70,7 +70,6 @@ public class TunnelKitVPNManagerStrategy<VPNType: VPN>: VPNManagerStrategy where
private var currentBundleIdentifier: String?
@MainActor
public init(
appGroup: String,
tunnelBundleIdentifier: @escaping (VPNProtocolType) -> String,

1
PassepartoutLibrary/Sources/PassepartoutVPN/Managers/VPNManager.swift
View File

@ -32,6 +32,7 @@ import PassepartoutProfiles
import PassepartoutProviders
import PassepartoutUtils
@MainActor
public final class VPNManager: ObservableObject {
// MARK: Initialization