Merge pull request #658 from OneBusAway/ualch9/PR-635

Fixes some concurrency related crashes
OneBusAway · Mar 30, 2023 · 1be686b · 1be686b
2 parents d0c459d + f5dfd28
commit 1be686b
Show file tree

Hide file tree

Showing 10 changed files with 74 additions and 47 deletions.
diff --git a/OBAKit/Controls/AlertPresenter.swift b/OBAKit/Controls/AlertPresenter.swift
@@ -16,22 +16,25 @@ class AlertPresenter: NSObject {
     /// Displays an error message to the user
     /// - Parameter error: The error to show to the user.
     /// - Parameter presentingController: The view controller that will act as the host for the presented error alert UI.
-    public class func show(error: Error, presentingController: UIViewController) {
-        show(errorMessage: error.localizedDescription, presentingController: presentingController)
+    @MainActor
+    public class func show(error: Error, presentingController: UIViewController) async {
+        await show(errorMessage: error.localizedDescription, presentingController: presentingController)
     }
 
     /// Displays an error message to the user.
     /// - Parameter errorMessage: The error message that will be shown.
     /// - Parameter presentingController: The view controller that will act as the host for the presented error alert UI.
-    public class func show(errorMessage: String, presentingController: UIViewController) {
-        showDismissableAlert(title: Strings.error, message: errorMessage, presentingController: presentingController)
+    @MainActor
+    public class func show(errorMessage: String, presentingController: UIViewController) async {
+        await showDismissableAlert(title: Strings.error, message: errorMessage, presentingController: presentingController)
     }
 
     /// Displays an alert with a Dismiss button, presented from `presentingController`.
     /// - Parameter title: Optional alert title.
     /// - Parameter message: Optional alert message.
     /// - Parameter presentingController: The presenting view controller.
-    public class func showDismissableAlert(title: String?, message: String?, presentingController: UIViewController) {
+    @MainActor
+    public class func showDismissableAlert(title: String?, message: String?, presentingController: UIViewController) async {
         let alert = UIAlertController(title: title, message: message, preferredStyle: .alert)
         alert.addAction(UIAlertAction(title: Strings.dismiss, style: .default, handler: nil))
         presentingController.present(alert, animated: true, completion: nil)

diff --git a/OBAKit/Mapping/MapRegionManager.swift b/OBAKit/Mapping/MapRegionManager.swift
@@ -203,7 +203,9 @@ public class MapRegionManager: NSObject,
 
         mapView.delegate = self
 
-        renderRegionsOnMap()
+        Task { @MainActor [weak self] in
+            await self?.renderRegionsOnMap()
+        }
     }
 
     deinit {
@@ -632,14 +634,17 @@ public class MapRegionManager: NSObject,
     // MARK: - Regions
 
     public func regionsService(_ service: RegionsService, updatedRegionsList regions: [Region]) {
-        renderRegionsOnMap()
+        Task { @MainActor [weak self] in
+            await self?.renderRegionsOnMap()
+        }
     }
 
     public func regionsService(_ service: RegionsService, updatedRegion region: Region) {
         mapView.setVisibleMapRect(region.serviceRect, animated: true)
     }
 
-    private func renderRegionsOnMap() {
+    @MainActor
+    private func renderRegionsOnMap() async {
         mapView.updateAnnotations(with: application.regionsService.regions)
     }
 }

diff --git a/OBAKit/Mapping/MapViewController.swift b/OBAKit/Mapping/MapViewController.swift
@@ -520,40 +520,49 @@ class MapViewController: UIViewController,
     }
 
     public func mapRegionManager(_ manager: MapRegionManager, noSearchResults response: SearchResponse) {
-        AlertPresenter.show(errorMessage: OBALoc("map_controller.no_search_results_found", value: "No search results were found.", comment: "A generic message shown when the user's search query produces no search results."), presentingController: self)
+        Task { @MainActor [weak self] in
+            guard let self else { return }
+            await AlertPresenter.show(errorMessage: OBALoc("map_controller.no_search_results_found", value: "No search results were found.", comment: "A generic message shown when the user's search query produces no search results."), presentingController: self)
+        }
     }
 
     public func mapRegionManager(_ manager: MapRegionManager, disambiguateSearch response: SearchResponse) {
-        let searchResults = SearchResultsController(searchResponse: response, application: application, delegate: self)
-        let nav = UINavigationController(rootViewController: searchResults)
-        application.viewRouter.present(nav, from: self, isModal: true)
+        Task { @MainActor [weak self] in
+            guard let self else { return }
+            let searchResults = SearchResultsController(searchResponse: response, application: application, delegate: self)
+            let nav = UINavigationController(rootViewController: searchResults)
+            application.viewRouter.present(nav, from: self, isModal: true)
+        }
     }
 
+    @MainActor
     public func mapRegionManager(_ manager: MapRegionManager, showSearchResult response: SearchResponse) {
-        guard let result = response.results.first else { return }
-
-        statusOverlay.isHidden = true
-
-        switch result {
-        case let result as MKMapItem:
-            let mapItemController = MapItemViewController(application: application, mapItem: result, delegate: self)
-            showSemiModalPanel(childController: mapItemController)
-        case let result as StopsForRoute:
-            let routeStopController = RouteStopsViewController(application: application, stopsForRoute: result, delegate: self)
-            showSemiModalPanel(childController: routeStopController)
-        case let result as Stop:
-            show(stop: result)
-        case let result as VehicleStatus:
-            if let convertible = TripConvertible(vehicleStatus: result) {
-                let tripController = TripViewController(application: application, tripConvertible: convertible)
-                application.viewRouter.navigate(to: tripController, from: self)
-            }
-            else {
-                let msg = OBALoc("map_controller.vehicle_not_on_trip_error", value: "The vehicle you chose doesn't appear to be on a trip right now, which means we don't know how to show it to you.", comment: "This message appears when a searched-for vehicle doesn't have an assigned trip.")
-                AlertPresenter.show(errorMessage: msg, presentingController: self)
+        Task { @MainActor [weak self] in
+            guard let self, let result = response.results.first else { return }
+
+            statusOverlay.isHidden = true
+
+            switch result {
+            case let result as MKMapItem:
+                let mapItemController = MapItemViewController(application: application, mapItem: result, delegate: self)
+                showSemiModalPanel(childController: mapItemController)
+            case let result as StopsForRoute:
+                let routeStopController = RouteStopsViewController(application: application, stopsForRoute: result, delegate: self)
+                showSemiModalPanel(childController: routeStopController)
+            case let result as Stop:
+                show(stop: result)
+            case let result as VehicleStatus:
+                if let convertible = TripConvertible(vehicleStatus: result) {
+                    let tripController = TripViewController(application: application, tripConvertible: convertible)
+                    application.viewRouter.navigate(to: tripController, from: self)
+                }
+                else {
+                    let msg = OBALoc("map_controller.vehicle_not_on_trip_error", value: "The vehicle you chose doesn't appear to be on a trip right now, which means we don't know how to show it to you.", comment: "This message appears when a searched-for vehicle doesn't have an assigned trip.")
+                    await AlertPresenter.show(errorMessage: msg, presentingController: self)
+                }
+            default:
+                fatalError()
             }
-        default:
-            fatalError()
         }
     }
 

diff --git a/OBAKit/PushNotifications/PushService.swift b/OBAKit/PushNotifications/PushService.swift
@@ -92,8 +92,10 @@ public class PushService: NSObject {
     }
 
     private func displayMessage(_ message: String) {
-        if let presentingController = delegate?.pushServicePresentingController(self) {
-            AlertPresenter.showDismissableAlert(title: message, message: nil, presentingController: presentingController)
+        Task { @MainActor in
+            if let presentingController = delegate?.pushServicePresentingController(self) {
+                await AlertPresenter.showDismissableAlert(title: message, message: nil, presentingController: presentingController)
+            }
         }
     }
 

diff --git a/OBAKit/Reporting/StopProblemViewController.swift b/OBAKit/Reporting/StopProblemViewController.swift
@@ -142,8 +142,8 @@ class StopProblemViewController: FormViewController {
         } catch {
             await MainActor.run {
                 ProgressHUD.dismiss()
-                AlertPresenter.show(error: error, presentingController: self)
             }
+            await AlertPresenter.show(error: error, presentingController: self)
         }
     }
 }
diff --git a/OBAKit/Reporting/VehicleProblemViewController.swift b/OBAKit/Reporting/VehicleProblemViewController.swift
@@ -180,8 +180,8 @@ class VehicleProblemViewController: FormViewController {
         } catch {
             await MainActor.run {
                 ProgressHUD.dismiss()
-                AlertPresenter.show(error: error, presentingController: self)
             }
+            await AlertPresenter.show(error: error, presentingController: self)
         }
     }
 }
diff --git a/OBAKit/Settings/MoreViewController.swift b/OBAKit/Settings/MoreViewController.swift
@@ -264,7 +264,10 @@ public class MoreViewController: UIViewController,
         controller.dismiss(animated: true, completion: nil)
 
         if let error = error {
-            AlertPresenter.show(error: error, presentingController: self)
+            Task { @MainActor [weak self] in
+                guard let self else { return }
+                await AlertPresenter.show(error: error, presentingController: self)
+            }
         }
     }
 
@@ -305,7 +308,10 @@ public class MoreViewController: UIViewController,
     }
 
     public func farePayments(_ farePayments: FarePayments, present error: Error) {
-        AlertPresenter.show(error: error, presentingController: self)
+        Task { @MainActor [weak self] in
+            guard let self else { return }
+            await AlertPresenter.show(error: error, presentingController: self)
+        }
     }
 
     // MARK: - Regions Service Delegate

diff --git a/OBAKit/Settings/SettingsViewController.swift b/OBAKit/Settings/SettingsViewController.swift
@@ -234,7 +234,10 @@ class SettingsViewController: FormViewController {
                     let activity = UIActivityViewController(activityItems: [xmlPath], applicationActivities: nil)
                     self.present(activity, animated: true, completion: nil)
                 } catch let ex {
-                    AlertPresenter.show(error: ex, presentingController: self)
+                    Task { @MainActor [weak self] in
+                        guard let self else { return }
+                        await AlertPresenter.show(error: ex, presentingController: self)
+                    }
                 }
             }
         }

diff --git a/OBAKit/Stops/StopViewController.swift b/OBAKit/Stops/StopViewController.swift
@@ -966,7 +966,9 @@ public class StopViewController: UIViewController,
 
     func alarmBuilder(_ alarmBuilder: AlarmBuilder, error: Error) {
         ProgressHUD.dismiss()
-        AlertPresenter.show(error: error, presentingController: self)
+        Task { @MainActor in
+            await AlertPresenter.show(error: error, presentingController: self)
+        }
     }
 
     // MARK: - Bookmarks

diff --git a/OBAKitCore/Location/Regions/RegionsService.swift b/OBAKitCore/Location/Regions/RegionsService.swift
@@ -345,10 +345,7 @@ public class RegionsService: NSObject, LocationServiceDelegate {
             return
         }
 
-        // FIXME: Audit which delegates are doing stuff on background thread, when they should be on Main.
-        await MainActor.run {
-            self.regions = regions
-        }
+        self.regions = regions
     }
 
     /// Fetches the current list of `Region`s from the network.