Fix crash in dispatch queue assertion with background tasks

buggmagnet · buggmagnet · commit 14eefef6d8f1 · 2025-01-23T16:09:08.000+01:00
diff --git a/ios/MullvadVPN/AddressCacheTracker/AddressCacheTracker.swift b/ios/MullvadVPN/AddressCacheTracker/AddressCacheTracker.swift
@@ -84,7 +84,7 @@ final class AddressCacheTracker: @unchecked Sendable {
         timer = nil
     }
 
-    func updateEndpoints(completionHandler: (@Sendable (Result<Bool, Error>) -> Void)? = nil) -> Cancellable {
+    func updateEndpoints(completionHandler: ((sending Result<Bool, Error>) -> Void)? = nil) -> Cancellable {
         let operation = ResultBlockOperation<Bool> { finish -> Cancellable in
             guard self.nextScheduleDate() <= Date() else {
                 finish(.success(false))
diff --git a/ios/MullvadVPN/AppDelegate.swift b/ios/MullvadVPN/AppDelegate.swift
@@ -277,15 +277,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
     private func registerAppRefreshTask() {
         let isRegistered = BGTaskScheduler.shared.register(
             forTaskWithIdentifier: BackgroundTask.appRefresh.identifier,
-            using: nil
+            using: .main
         ) { [self] task in
-            nonisolated(unsafe) let nonisolatedTask = task
 
             let handle = relayCacheTracker.updateRelays { result in
-                nonisolatedTask.setTaskCompleted(success: result.isSuccess)
+                task.setTaskCompleted(success: result.isSuccess)
             }
 
-            nonisolatedTask.expirationHandler = {
+            task.expirationHandler = {
                 handle.cancel()
             }
 
@@ -302,18 +301,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
     private func registerKeyRotationTask() {
         let isRegistered = BGTaskScheduler.shared.register(
             forTaskWithIdentifier: BackgroundTask.privateKeyRotation.identifier,
-            using: nil
+            using: .main
         ) { [self] task in
-            nonisolated(unsafe) let nonisolatedTask = task
             let handle = tunnelManager.rotatePrivateKey { [self] error in
-                Task { @MainActor in
-                    scheduleKeyRotationTask()
-
-                    nonisolatedTask.setTaskCompleted(success: error == nil)
-                }
+                scheduleKeyRotationTask()
+                task.setTaskCompleted(success: error == nil)
             }
 
-            nonisolatedTask.expirationHandler = {
+            task.expirationHandler = {
                 handle.cancel()
             }
         }
@@ -328,18 +323,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
     private func registerAddressCacheUpdateTask() {
         let isRegistered = BGTaskScheduler.shared.register(
             forTaskWithIdentifier: BackgroundTask.addressCacheUpdate.identifier,
-            using: nil
+            using: .main
         ) { [self] task in
-            nonisolated(unsafe) let nonisolatedTask = task
-
             let handle = addressCacheTracker.updateEndpoints { [self] result in
-                Task { @MainActor in
-                    scheduleAddressCacheUpdateTask()
-                    nonisolatedTask.setTaskCompleted(success: result.isSuccess)
-                }
+                scheduleAddressCacheUpdateTask()
+                task.setTaskCompleted(success: result.isSuccess)
             }
 
-            nonisolatedTask.expirationHandler = {
+            task.expirationHandler = {
                 handle.cancel()
             }
         }
diff --git a/ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift b/ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift
@@ -16,7 +16,7 @@ import UIKit
 protocol RelayCacheTrackerProtocol: Sendable {
     func startPeriodicUpdates()
     func stopPeriodicUpdates()
-    func updateRelays(completionHandler: (@Sendable (Result<RelaysFetchResult, Error>) -> Void)?) -> Cancellable
+    func updateRelays(completionHandler: ((sending Result<RelaysFetchResult, Error>) -> Void)?) -> Cancellable
     func getCachedRelays() throws -> CachedRelays
     func getNextUpdateDate() -> Date
     func addObserver(_ observer: RelayCacheTrackerObserver)
@@ -164,7 +164,7 @@ final class RelayCacheTracker: RelayCacheTrackerProtocol, @unchecked Sendable {
         timerSource = nil
     }
 
-    func updateRelays(completionHandler: (@Sendable (Result<RelaysFetchResult, Error>) -> Void)? = nil)
+    func updateRelays(completionHandler: ((sending Result<RelaysFetchResult, Error>) -> Void)? = nil)
         -> Cancellable {
         let operation = ResultBlockOperation<RelaysFetchResult> { finish in
             let cachedRelays = try? self.getCachedRelays()
diff --git a/ios/MullvadVPN/TunnelManager/BackgroundTaskProvider.swift b/ios/MullvadVPN/TunnelManager/BackgroundTaskProvider.swift
@@ -14,18 +14,10 @@ import UIKit
 @available(iOSApplicationExtension, unavailable)
 public protocol BackgroundTaskProviding: Sendable {
     var backgroundTimeRemaining: TimeInterval { get }
-    #if compiler(>=6)
-    nonisolated
-    func beginBackgroundTask(
+    nonisolated func beginBackgroundTask(
         withName taskName: String?,
         expirationHandler handler: (@MainActor @Sendable () -> Void)?
     ) -> UIBackgroundTaskIdentifier
-    #else
-    func beginBackgroundTask(
-        withName taskName: String?,
-        expirationHandler handler: (() -> Void)?
-    ) -> UIBackgroundTaskIdentifier
-    #endif
 
     func endBackgroundTask(_ identifier: UIBackgroundTaskIdentifier)
 }
@@ -40,22 +32,12 @@ public final class BackgroundTaskProvider: BackgroundTaskProviding {
         self.application = application
     }
 
-    #if compiler(>=6)
-    nonisolated
-    public func beginBackgroundTask(
-        withName taskName: String?,
-        expirationHandler handler: (@MainActor @Sendable () -> Void)?
-    ) -> UIBackgroundTaskIdentifier {
-        application.beginBackgroundTask(withName: taskName, expirationHandler: handler)
-    }
-    #else
-    public func beginBackgroundTask(
+    nonisolated public func beginBackgroundTask(
         withName taskName: String?,
-        expirationHandler handler: (() -> Void)?
+        expirationHandler handler: (@MainActor @Sendable () -> Void)? = nil
     ) -> UIBackgroundTaskIdentifier {
         application.beginBackgroundTask(withName: taskName, expirationHandler: handler)
     }
-    #endif
 
     public func endBackgroundTask(_ identifier: UIBackgroundTaskIdentifier) {
         application.endBackgroundTask(identifier)
diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
@@ -218,29 +218,25 @@ final class TunnelManager: StorePaymentObserver, @unchecked Sendable {
     }
 
     func startTunnel(completionHandler: ((Error?) -> Void)? = nil) {
-        nonisolated(unsafe) let nonisolatedCompletionHandler = completionHandler
-
         let operation = StartTunnelOperation(
             dispatchQueue: internalQueue,
             interactor: TunnelInteractorProxy(self),
             completionHandler: { [weak self] result in
                 guard let self else { return }
-                DispatchQueue.main.async {
-                    if let error = result.error {
-                        self.logger.error(
-                            error: error,
-                            message: "Failed to start the tunnel."
-                        )
-
-                        let tunnelError = StartTunnelError(underlyingError: error)
-
-                        self.observerList.notify { observer in
-                            observer.tunnelManager(self, didFailWithError: tunnelError)
-                        }
-                    }
+                if let error = result.error {
+                    self.logger.error(
+                        error: error,
+                        message: "Failed to start the tunnel."
+                    )
+
+                    let tunnelError = StartTunnelError(underlyingError: error)
 
-                    nonisolatedCompletionHandler?(result.error)
+                    self.observerList.notify { observer in
+                        observer.tunnelManager(self, didFailWithError: tunnelError)
+                    }
                 }
+
+                completionHandler?(result.error)
             }
         )
 
@@ -255,29 +251,26 @@ final class TunnelManager: StorePaymentObserver, @unchecked Sendable {
     }
 
     func stopTunnel(completionHandler: ((Error?) -> Void)? = nil) {
-        nonisolated(unsafe) let nonisolatedCompletionHandler = completionHandler
         let operation = StopTunnelOperation(
             dispatchQueue: internalQueue,
             interactor: TunnelInteractorProxy(self)
         ) { [weak self] result in
             guard let self else { return }
 
-            DispatchQueue.main.async {
-                if let error = result.error {
-                    self.logger.error(
-                        error: error,
-                        message: "Failed to stop the tunnel."
-                    )
+            if let error = result.error {
+                self.logger.error(
+                    error: error,
+                    message: "Failed to stop the tunnel."
+                )
 
-                    let tunnelError = StopTunnelError(underlyingError: error)
+                let tunnelError = StopTunnelError(underlyingError: error)
 
-                    self.observerList.notify { observer in
-                        observer.tunnelManager(self, didFailWithError: tunnelError)
-                    }
+                self.observerList.notify { observer in
+                    observer.tunnelManager(self, didFailWithError: tunnelError)
                 }
-
-                nonisolatedCompletionHandler?(result.error)
             }
+
+            completionHandler?(result.error)
         }
 
         operation.addObserver(BackgroundObserver(
@@ -482,7 +475,7 @@ final class TunnelManager: StorePaymentObserver, @unchecked Sendable {
         operationQueue.addOperation(operation)
     }
 
-    func rotatePrivateKey(completionHandler: @escaping @Sendable (Error?) -> Void) -> Cancellable {
+    func rotatePrivateKey(completionHandler: @MainActor @escaping @Sendable (Error?) -> Void) -> Cancellable {
         let operation = RotateKeyOperation(
             dispatchQueue: internalQueue,
             interactor: TunnelInteractorProxy(self),
@@ -499,8 +492,6 @@ final class TunnelManager: StorePaymentObserver, @unchecked Sendable {
             if let error {
                 handleRestError(error)
             }
-
-            completionHandler(error)
         }
 
         operation.addObserver(
diff --git a/ios/MullvadVPNTests/MullvadVPN/RelayCacheTracker/RelayCacheTracker+Stubs.swift b/ios/MullvadVPNTests/MullvadVPN/RelayCacheTracker/RelayCacheTracker+Stubs.swift
@@ -15,7 +15,7 @@ struct RelayCacheTrackerStub: RelayCacheTrackerProtocol {
 
     func stopPeriodicUpdates() {}
 
-    func updateRelays(completionHandler: ((Result<RelaysFetchResult, Error>) -> Void)?) -> Cancellable {
+    func updateRelays(completionHandler: sending ((Result<RelaysFetchResult, Error>) -> Void)?) -> Cancellable {
         AnyCancellable()
     }
 
diff --git a/ios/Operations/ResultOperation.swift b/ios/Operations/ResultOperation.swift
@@ -10,7 +10,7 @@ import Foundation
 
 /// Base class for operations producing result.
 open class ResultOperation<Success: Sendable>: AsyncOperation, OutputOperation, @unchecked Sendable {
-    public typealias CompletionHandler = @Sendable (Result<Success, Error>) -> Void
+    public typealias CompletionHandler = (sending Result<Success, Error>) -> Void
 
     private let nslock = NSLock()
     private var _output: Success?
@@ -118,7 +118,7 @@ open class ResultOperation<Success: Sendable>: AsyncOperation, OutputOperation,
         let completionQueue = _completionQueue
         nslock.unlock()
 
-        let block: @Sendable () -> Void = {
+        let block: () -> Void = {
             // Call completion handler.
             completionHandler?(result)
 

Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ final class AddressCacheTracker: @unchecked Sendable {`
`84`	`84`	`timer = nil`
`85`	`85`	`}`
`86`	`86`
`87`		`- func updateEndpoints(completionHandler: (@Sendable (Result<Bool, Error>) -> Void)? = nil) -> Cancellable {`
	`87`	`+ func updateEndpoints(completionHandler: ((sending Result<Bool, Error>) -> Void)? = nil) -> Cancellable {`
`88`	`88`	`let operation = ResultBlockOperation<Bool> { finish -> Cancellable in`
`89`	`89`	`guard self.nextScheduleDate() <= Date() else {`
`90`	`90`	`finish(.success(false))`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ struct RelayCacheTrackerStub: RelayCacheTrackerProtocol {`
`15`	`15`
`16`	`16`	`func stopPeriodicUpdates() {}`
`17`	`17`
`18`		`- func updateRelays(completionHandler: ((Result<RelaysFetchResult, Error>) -> Void)?) -> Cancellable {`
	`18`	`+ func updateRelays(completionHandler: sending ((Result<RelaysFetchResult, Error>) -> Void)?) -> Cancellable {`
`19`	`19`	`AnyCancellable()`
`20`	`20`	`}`
`21`	`21`