Switch the clientProxy's start/stop sync semantics to service pausing and resuming to better match underlying behaviors

ElementX/Sources/Application/AppCoordinator.swift

@@ -42,7 +42,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
                 configureNotificationManager()
                 observeUserSessionChanges()
                 Task {
-                    await startSync()
+                    await resumeServices()
                     await appHooks.configure(with: userSession)
                 }
             }
@@ -185,11 +185,11 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
                     self?.handleAppRoute(.call(roomID: roomID, isVoiceCall: isVoiceCall), windowType: nil)
                 case .receivedIncomingCallRequest:
                     // When reporting a VoIP call through the CXProvider's `reportNewIncomingVoIPPushPayload`
-                    // the UIApplication states don't change and syncing is neither started nor ran on
+                    // the UIApplication states don't change and services are neither started nor ran on
                     // a background task. Handle both manually here.
                     Task {
-                        await self?.startSync()
-                        self?.scheduleDelayedSyncStop()
+                        await self?.resumeServices()
+                        self?.scheduleDelayedPauseServices()
                     }
                 default:
                     break
@@ -813,7 +813,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
         
         showLoadingIndicator()
         
-        stopSync(isBackgroundTask: false)
+        pauseServices(isBackgroundTask: false)
         userSessionFlowCoordinator?.stop()
         
         guard !isSoft else {
@@ -943,7 +943,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
         
         navigationRootCoordinator.setRootCoordinator(PlaceholderScreenCoordinator(hideBrandChrome: appSettings.hideBrandChrome))
         
-        stopSync(isBackgroundTask: false)
+        pauseServices(isBackgroundTask: false)
         userSessionFlowCoordinator?.stop()
         
         // Allow for everything to deallocate properly
@@ -1064,24 +1064,24 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
     
     // MARK: - Application State
     
-    private func stopSync(isBackgroundTask: Bool, completion: (() -> Void)? = nil) {
+    private func pauseServices(isBackgroundTask: Bool, completion: (() -> Void)? = nil) {
         if isBackgroundTask, UIApplication.shared.applicationState == .active {
-            // Attempt to stop the background task sync loop cleanly, only if the app not already running
+            // Attempt to pause the background task services cleanly, only if the app not already running
             return
         }
         
         MainActor.assumeIsolated {
-            userSession?.clientProxy.stopSync(completion: completion)
+            userSession?.clientProxy.pauseServices(completion: completion)
             clientProxyObserver = nil
         }
     }
     
-    private func startSync() async {
+    private func resumeServices() async {
         guard let userSession else { return }
         
         analyticsService.signpost.startTransaction(.upToDateRoomList)
         
-        await userSession.clientProxy.startSync()
+        await userSession.clientProxy.resumeServices()
         
         guard clientProxyObserver == nil else {
             return
@@ -1147,14 +1147,14 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
     @objc
     private func applicationWillTerminate() {
         MXLog.info("Application will terminate")
-        stopSync(isBackgroundTask: false)
+        pauseServices(isBackgroundTask: false)
     }
     
     @objc
     private func applicationDidEnterBackground() {
         MXLog.info("Application did enter background")
         
-        scheduleDelayedSyncStop()
+        scheduleDelayedPauseServices()
         scheduleBackgroundAppRefresh()
     }
     
@@ -1163,7 +1163,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
         MXLog.info("Application will resign active")
     }
     
-    private func scheduleDelayedSyncStop() {
+    private func scheduleDelayedPauseServices() {
         guard backgroundTask == nil else {
             return
         }
@@ -1174,7 +1174,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
             // We're intentionally strongly retaining self here to an EXC_BAD_ACCESS
             // `backgroundTask` will be eventually released in `endActiveBackgroundTask`
             // https://sentry.tools.element.io/organizations/element/issues/4477794/events/9cfd04e4d045440f87498809cf718de5/
-            self.stopSync(isBackgroundTask: true) {
+            self.pauseServices(isBackgroundTask: true) {
                 self.endActiveBackgroundTask()
             }
         }
@@ -1185,7 +1185,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
         MXLog.info("Application will enter foreground")
         endActiveBackgroundTask()
         Task {
-            await startSync()
+            await resumeServices()
         }
     }
     
@@ -1242,17 +1242,17 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
         // This is important for the app to keep refreshing in the background
         scheduleBackgroundAppRefresh()
         
-        // We have a lot of crashes stemming here which we previously believed are caused by stopSync not being async
+        // We have a lot of crashes stemming here which we previously believed are caused by pauseServices not being async
         // on the client proxy side (see the comment on that method). We have now realised that will likely not fix anything but
         // we also noticed this does not crash on the main thread, even though the whole AppCoordinator is on the Main actor.
         // As such, we introduced a MainActor conformance on the expirationHandler but we are also assuming main actor
-        // isolated in the `stopSync` method above.
+        // isolated in the `pauseServices` method above.
         // https://sentry.tools.element.io/organizations/element/issues/4477794/
         task.expirationHandler = { @Sendable [weak self] in
             MXLog.info("Background app refresh task is about to expire.")
             
             Task { @MainActor in
-                self?.stopSync(isBackgroundTask: true) {
+                self?.pauseServices(isBackgroundTask: true) {
                     MXLog.info("Marking Background app refresh task as complete.")
                     task.setTaskCompleted(success: true)
                 }
@@ -1263,7 +1263,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
             return
         }
         
-        await startSync()
+        await resumeServices()
         
         // Be a good citizen, run for a max of 10 SS responses or 10 seconds
         // An SS request will time out after 30 seconds if no new data is available
@@ -1278,7 +1278,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
                 
                 // Make sure we stop the sync loop, otherwise the ongoing request is immediately
                 // handled the next time the app refreshes, which can trigger timeout failures.
-                stopSync(isBackgroundTask: true) {
+                pauseServices(isBackgroundTask: true) {
                     MXLog.info("Marking Background app refresh task as complete.")
                     task.setTaskCompleted(success: true)
                 }

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

@@ -2451,56 +2451,39 @@ class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable {
             return hasDevicesToVerifyAgainstReturnValue
         }
     }
-    //MARK: - startSync
+    //MARK: - resumeServices
 
-    private let startSyncCallsCountLock = NSLock()
-    private var startSyncUnderlyingCallsCount = 0
-    var startSyncCallsCount: Int {
-        get { startSyncCallsCountLock.withLock { startSyncUnderlyingCallsCount } }
-        set { startSyncCallsCountLock.withLock { startSyncUnderlyingCallsCount = newValue } }
+    private let resumeServicesCallsCountLock = NSLock()
+    private var resumeServicesUnderlyingCallsCount = 0
+    var resumeServicesCallsCount: Int {
+        get { resumeServicesCallsCountLock.withLock { resumeServicesUnderlyingCallsCount } }
+        set { resumeServicesCallsCountLock.withLock { resumeServicesUnderlyingCallsCount = newValue } }
     }
-    var startSyncCalled: Bool {
-        return startSyncCallsCount > 0
+    var resumeServicesCalled: Bool {
+        return resumeServicesCallsCount > 0
     }
-    var startSyncClosure: (() async -> Void)?
+    var resumeServicesClosure: (() async -> Void)?
 
-    func startSync() async {
-        startSyncCallsCountLock.withLock { startSyncUnderlyingCallsCount += 1 }
-        await startSyncClosure?()
+    func resumeServices() async {
+        resumeServicesCallsCountLock.withLock { resumeServicesUnderlyingCallsCount += 1 }
+        await resumeServicesClosure?()
     }
-    //MARK: - stopSync
+    //MARK: - pauseServices
 
-    private let stopSyncCallsCountLock = NSLock()
-    private var stopSyncUnderlyingCallsCount = 0
-    var stopSyncCallsCount: Int {
-        get { stopSyncCallsCountLock.withLock { stopSyncUnderlyingCallsCount } }
-        set { stopSyncCallsCountLock.withLock { stopSyncUnderlyingCallsCount = newValue } }
+    private let pauseServicesCompletionCallsCountLock = NSLock()
+    private var pauseServicesCompletionUnderlyingCallsCount = 0
+    var pauseServicesCompletionCallsCount: Int {
+        get { pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount } }
+        set { pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount = newValue } }
     }
-    var stopSyncCalled: Bool {
-        return stopSyncCallsCount > 0
+    var pauseServicesCompletionCalled: Bool {
+        return pauseServicesCompletionCallsCount > 0
     }
-    var stopSyncClosure: (() -> Void)?
+    var pauseServicesCompletionClosure: (((() -> Void)?) -> Void)?
 
-    func stopSync() {
-        stopSyncCallsCountLock.withLock { stopSyncUnderlyingCallsCount += 1 }
-        stopSyncClosure?()
-    }
-    //MARK: - stopSync
-
-    private let stopSyncCompletionCallsCountLock = NSLock()
-    private var stopSyncCompletionUnderlyingCallsCount = 0
-    var stopSyncCompletionCallsCount: Int {
-        get { stopSyncCompletionCallsCountLock.withLock { stopSyncCompletionUnderlyingCallsCount } }
-        set { stopSyncCompletionCallsCountLock.withLock { stopSyncCompletionUnderlyingCallsCount = newValue } }
-    }
-    var stopSyncCompletionCalled: Bool {
-        return stopSyncCompletionCallsCount > 0
-    }
-    var stopSyncCompletionClosure: (((() -> Void)?) -> Void)?
-
-    func stopSync(completion: (() -> Void)?) {
-        stopSyncCompletionCallsCountLock.withLock { stopSyncCompletionUnderlyingCallsCount += 1 }
-        stopSyncCompletionClosure?(completion)
+    func pauseServices(completion: (() -> Void)?) {
+        pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount += 1 }
+        pauseServicesCompletionClosure?(completion)
     }
     //MARK: - expireSyncSessions
 

ElementX/Sources/Services/Client/ClientProxy.swift

@@ -154,7 +154,7 @@ class ClientProxy: ClientProxyProtocol {
     private var hasEncounteredAuthError = false
     
     deinit {
-        stopSync { [delegateHandle] in
+        pauseServices { [delegateHandle] in
             // The delegate handle needs to be cancelled always after the sync stops
             delegateHandle?.cancel()
         }
@@ -389,7 +389,9 @@ class ClientProxy: ClientProxyProtocol {
         }
     }
     
-    func startSync() async {
+    func resumeServices() async {
+        MXLog.info("Resuming services")
+        
         guard !hasEncounteredAuthError else {
             MXLog.warning("Ignoring request, this client has an unknown token.")
             return
@@ -418,11 +420,11 @@ class ClientProxy: ClientProxyProtocol {
     }
     
     /// A stored task for restarting the sync after a failure. This is stored so that we can cancel
-    /// it when `stopSync` is called (e.g. when signing out) to prevent an otherwise infinite
+    /// it when `pauseServices` is called (e.g. when signing out) to prevent an otherwise infinite
     /// loop that was triggered by trying to sync a signed out session.
     @CancellableTask private var restartTask: Task<Void, Never>?
     
-    func restartSync() {
+    private func restartServices() {
         guard restartTask == nil else { return }
         
         restartTask = Task { [weak self] in
@@ -430,7 +432,7 @@ class ClientProxy: ClientProxyProtocol {
                 // Until the SDK can tell us the failure, we add a small
                 // delay to avoid generating multi-gigabyte log files.
                 try await Task.sleep(for: .milliseconds(250))
-                await self?.startSync()
+                await self?.resumeServices()
             } catch {
                 MXLog.error("Restart cancelled.")
             }
@@ -438,13 +440,10 @@ class ClientProxy: ClientProxyProtocol {
         }
     }
     
-    func stopSync() {
-        stopSync(completion: nil)
-    }
-    
-    func stopSync(completion: (() -> Void)?) {
+    func pauseServices(completion: (() -> Void)?) {
+        MXLog.info("Pausing services")
+        
         if restartTask != nil {
-            MXLog.warning("Removing the sync service restart task.")
             restartTask = nil
         }
         
@@ -1030,7 +1029,7 @@ class ClientProxy: ClientProxyProtocol {
             .sink { [weak self] reachability in
                 if reachability == .reachable {
                     Task {
-                        await self?.startSync()
+                        await self?.resumeServices()
                     }
                 }
             }
@@ -1147,7 +1146,7 @@ class ClientProxy: ClientProxyProtocol {
             case .offline:
                 homeserverReachabilitySubject.send(.unreachable)
             case .error:
-                restartSync()
+                restartServices()
             }
         })
     }

ElementX/Sources/Services/Client/ClientProxyProtocol.swift

@@ -169,11 +169,9 @@ protocol ClientProxyProtocol: AnyObject {
     
     func hasDevicesToVerifyAgainst() async -> Result<Bool, ClientProxyError>
     
-    func startSync() async
+    func resumeServices() async
     
-    func stopSync()
-    
-    func stopSync(completion: (() -> Void)?) // Hopefully this will become async once we get SE-0371.
+    func pauseServices(completion: (() -> Void)?) // Hopefully this will become async once we get SE-0371.
     
     func expireSyncSessions() async