From 02b114d40ba60542fc3c2bf8ca6c8d82c03127a2 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 15 Jun 2026 12:49:46 +0300 Subject: [PATCH] Make ClientProxy.pauseServices async and drop its completion block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pauseServices` took a completion block only because it was historically called from `deinit`, which can't await (the SE-0371 note). `deinit` now stops the sync itself, so the sole remaining caller is the AppCoordinator, which can await instead. Make `pauseServices` async and update the AppCoordinator background-task and lifecycle call sites to await it, calling the OS-facing completions (endBackgroundTask / setTaskCompleted) afterwards. This also removes the `MainActor.assumeIsolated` workaround — awaiting a @MainActor method hops onto the main actor cleanly — and fixes a latent bug where a nil `self` in the background app refresh expiration handler would skip setTaskCompleted. --- .../Sources/Application/AppCoordinator.swift | 36 +++++++++---------- .../Mocks/Generated/GeneratedMocks.swift | 22 ++++++------ .../Sources/Services/Client/ClientProxy.swift | 8 ++--- .../Services/Client/ClientProxyProtocol.swift | 2 +- 4 files changed, 31 insertions(+), 37 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index 7363dbeca..2c47a25a1 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -821,7 +821,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg showLoadingIndicator() - pauseClientServices(isBackgroundTask: false) + Task { await pauseClientServices(isBackgroundTask: false) } userSessionFlowCoordinator?.stop() guard !isSoft else { @@ -951,7 +951,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg navigationRootCoordinator.setRootCoordinator(PlaceholderScreenCoordinator(hideBrandChrome: appSettings.hideBrandChrome)) - pauseClientServices(isBackgroundTask: false) + Task { await pauseClientServices(isBackgroundTask: false) } userSessionFlowCoordinator?.stop() // Allow for everything to deallocate properly @@ -1080,16 +1080,14 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg // MARK: - Application State - private func pauseClientServices(isBackgroundTask: Bool, completion: (() -> Void)? = nil) { + private func pauseClientServices(isBackgroundTask: Bool) async { if isBackgroundTask, UIApplication.shared.applicationState == .active { // Attempt to pause the background task services cleanly, only if the app not already running return } - MainActor.assumeIsolated { - userSession?.clientProxy.pauseServices(completion: completion) - clientProxyObserver = nil - } + await userSession?.clientProxy.pauseServices() + clientProxyObserver = nil } private func resumeClientServices() async { @@ -1163,7 +1161,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg @objc private func applicationWillTerminate() { MXLog.info("Application will terminate") - pauseClientServices(isBackgroundTask: false) + Task { await pauseClientServices(isBackgroundTask: false) } } @objc @@ -1190,7 +1188,8 @@ 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.pauseClientServices(isBackgroundTask: true) { + Task { @MainActor in + await self.pauseClientServices(isBackgroundTask: true) self.endActiveBackgroundTask() } } @@ -1258,20 +1257,18 @@ 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 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 `pauseServices` method above. + // We have a lot of crashes stemming here which we previously believed were caused by pauseServices not being + // async on the client proxy side. 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 hop onto the main actor explicitly before pausing. // 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?.pauseClientServices(isBackgroundTask: true) { - MXLog.info("Marking Background app refresh task as complete.") - task.setTaskCompleted(success: true) - } + await self?.pauseClientServices(isBackgroundTask: true) + MXLog.info("Marking Background app refresh task as complete.") + task.setTaskCompleted(success: true) } } @@ -1294,7 +1291,8 @@ 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. - pauseClientServices(isBackgroundTask: true) { + Task { + await self.pauseClientServices(isBackgroundTask: true) MXLog.info("Marking Background app refresh task as complete.") task.setTaskCompleted(success: true) } diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index 810678f00..53464f47b 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -2470,20 +2470,20 @@ class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable { } //MARK: - pauseServices - private let pauseServicesCompletionCallsCountLock = NSLock() - private var pauseServicesCompletionUnderlyingCallsCount = 0 - var pauseServicesCompletionCallsCount: Int { - get { pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount } } - set { pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount = newValue } } + private let pauseServicesCallsCountLock = NSLock() + private var pauseServicesUnderlyingCallsCount = 0 + var pauseServicesCallsCount: Int { + get { pauseServicesCallsCountLock.withLock { pauseServicesUnderlyingCallsCount } } + set { pauseServicesCallsCountLock.withLock { pauseServicesUnderlyingCallsCount = newValue } } } - var pauseServicesCompletionCalled: Bool { - return pauseServicesCompletionCallsCount > 0 + var pauseServicesCalled: Bool { + return pauseServicesCallsCount > 0 } - var pauseServicesCompletionClosure: (((() -> Void)?) -> Void)? + var pauseServicesClosure: (() async -> Void)? - func pauseServices(completion: (() -> Void)?) { - pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount += 1 } - pauseServicesCompletionClosure?(completion) + func pauseServices() async { + pauseServicesCallsCountLock.withLock { pauseServicesUnderlyingCallsCount += 1 } + await pauseServicesClosure?() } //MARK: - expireSyncSessions diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index 97232f49a..e578055c9 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -431,18 +431,14 @@ class ClientProxy: ClientProxyProtocol { } } - func pauseServices(completion: (() -> Void)?) { + func pauseServices() async { MXLog.info("Pausing services") if restartTask != nil { restartTask = nil } - let task = transitionServices(to: .suspended) - Task { - await task.value - completion?() - } + await transitionServices(to: .suspended).value } func expireSyncSessions() async { diff --git a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift index e852e02d8..fcfd5b337 100644 --- a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift +++ b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift @@ -171,7 +171,7 @@ protocol ClientProxyProtocol: AnyObject { func resumeServices() async - func pauseServices(completion: (() -> Void)?) // Hopefully this will become async once we get SE-0371. + func pauseServices() async func expireSyncSessions() async