mirror of
https://github.com/Novaloop-AG/element-x-ios.git
synced 2026-11-03 16:10:20 +00:00
Make ClientProxy.pauseServices async and drop its completion block
`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.
This commit is contained in:
@@ -821,7 +821,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
|
|||||||
|
|
||||||
showLoadingIndicator()
|
showLoadingIndicator()
|
||||||
|
|
||||||
pauseClientServices(isBackgroundTask: false)
|
Task { await pauseClientServices(isBackgroundTask: false) }
|
||||||
userSessionFlowCoordinator?.stop()
|
userSessionFlowCoordinator?.stop()
|
||||||
|
|
||||||
guard !isSoft else {
|
guard !isSoft else {
|
||||||
@@ -951,7 +951,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
|
|||||||
|
|
||||||
navigationRootCoordinator.setRootCoordinator(PlaceholderScreenCoordinator(hideBrandChrome: appSettings.hideBrandChrome))
|
navigationRootCoordinator.setRootCoordinator(PlaceholderScreenCoordinator(hideBrandChrome: appSettings.hideBrandChrome))
|
||||||
|
|
||||||
pauseClientServices(isBackgroundTask: false)
|
Task { await pauseClientServices(isBackgroundTask: false) }
|
||||||
userSessionFlowCoordinator?.stop()
|
userSessionFlowCoordinator?.stop()
|
||||||
|
|
||||||
// Allow for everything to deallocate properly
|
// Allow for everything to deallocate properly
|
||||||
@@ -1080,16 +1080,14 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
|
|||||||
|
|
||||||
// MARK: - Application State
|
// MARK: - Application State
|
||||||
|
|
||||||
private func pauseClientServices(isBackgroundTask: Bool, completion: (() -> Void)? = nil) {
|
private func pauseClientServices(isBackgroundTask: Bool) async {
|
||||||
if isBackgroundTask, UIApplication.shared.applicationState == .active {
|
if isBackgroundTask, UIApplication.shared.applicationState == .active {
|
||||||
// Attempt to pause the background task services cleanly, only if the app not already running
|
// Attempt to pause the background task services cleanly, only if the app not already running
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
MainActor.assumeIsolated {
|
await userSession?.clientProxy.pauseServices()
|
||||||
userSession?.clientProxy.pauseServices(completion: completion)
|
clientProxyObserver = nil
|
||||||
clientProxyObserver = nil
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private func resumeClientServices() async {
|
private func resumeClientServices() async {
|
||||||
@@ -1163,7 +1161,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
|
|||||||
@objc
|
@objc
|
||||||
private func applicationWillTerminate() {
|
private func applicationWillTerminate() {
|
||||||
MXLog.info("Application will terminate")
|
MXLog.info("Application will terminate")
|
||||||
pauseClientServices(isBackgroundTask: false)
|
Task { await pauseClientServices(isBackgroundTask: false) }
|
||||||
}
|
}
|
||||||
|
|
||||||
@objc
|
@objc
|
||||||
@@ -1190,7 +1188,8 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
|
|||||||
// We're intentionally strongly retaining self here to an EXC_BAD_ACCESS
|
// We're intentionally strongly retaining self here to an EXC_BAD_ACCESS
|
||||||
// `backgroundTask` will be eventually released in `endActiveBackgroundTask`
|
// `backgroundTask` will be eventually released in `endActiveBackgroundTask`
|
||||||
// https://sentry.tools.element.io/organizations/element/issues/4477794/events/9cfd04e4d045440f87498809cf718de5/
|
// 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()
|
self.endActiveBackgroundTask()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1258,20 +1257,18 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
|
|||||||
// This is important for the app to keep refreshing in the background
|
// This is important for the app to keep refreshing in the background
|
||||||
scheduleBackgroundAppRefresh()
|
scheduleBackgroundAppRefresh()
|
||||||
|
|
||||||
// We have a lot of crashes stemming here which we previously believed are caused by pauseServices not being async
|
// We have a lot of crashes stemming here which we previously believed were caused by pauseServices not being
|
||||||
// on the client proxy side (see the comment on that method). We have now realised that will likely not fix anything but
|
// async on the client proxy side. We have now realised that will likely not fix anything but we also noticed
|
||||||
// we also noticed this does not crash on the main thread, even though the whole AppCoordinator is on the Main actor.
|
// this does not crash on the main thread, even though the whole AppCoordinator is on the Main actor. As such,
|
||||||
// As such, we introduced a MainActor conformance on the expirationHandler but we are also assuming main actor
|
// we hop onto the main actor explicitly before pausing.
|
||||||
// isolated in the `pauseServices` method above.
|
|
||||||
// https://sentry.tools.element.io/organizations/element/issues/4477794/
|
// https://sentry.tools.element.io/organizations/element/issues/4477794/
|
||||||
task.expirationHandler = { @Sendable [weak self] in
|
task.expirationHandler = { @Sendable [weak self] in
|
||||||
MXLog.info("Background app refresh task is about to expire.")
|
MXLog.info("Background app refresh task is about to expire.")
|
||||||
|
|
||||||
Task { @MainActor in
|
Task { @MainActor in
|
||||||
self?.pauseClientServices(isBackgroundTask: true) {
|
await self?.pauseClientServices(isBackgroundTask: true)
|
||||||
MXLog.info("Marking Background app refresh task as complete.")
|
MXLog.info("Marking Background app refresh task as complete.")
|
||||||
task.setTaskCompleted(success: true)
|
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
|
// 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.
|
// 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.")
|
MXLog.info("Marking Background app refresh task as complete.")
|
||||||
task.setTaskCompleted(success: true)
|
task.setTaskCompleted(success: true)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2470,20 +2470,20 @@ class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable {
|
|||||||
}
|
}
|
||||||
//MARK: - pauseServices
|
//MARK: - pauseServices
|
||||||
|
|
||||||
private let pauseServicesCompletionCallsCountLock = NSLock()
|
private let pauseServicesCallsCountLock = NSLock()
|
||||||
private var pauseServicesCompletionUnderlyingCallsCount = 0
|
private var pauseServicesUnderlyingCallsCount = 0
|
||||||
var pauseServicesCompletionCallsCount: Int {
|
var pauseServicesCallsCount: Int {
|
||||||
get { pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount } }
|
get { pauseServicesCallsCountLock.withLock { pauseServicesUnderlyingCallsCount } }
|
||||||
set { pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount = newValue } }
|
set { pauseServicesCallsCountLock.withLock { pauseServicesUnderlyingCallsCount = newValue } }
|
||||||
}
|
}
|
||||||
var pauseServicesCompletionCalled: Bool {
|
var pauseServicesCalled: Bool {
|
||||||
return pauseServicesCompletionCallsCount > 0
|
return pauseServicesCallsCount > 0
|
||||||
}
|
}
|
||||||
var pauseServicesCompletionClosure: (((() -> Void)?) -> Void)?
|
var pauseServicesClosure: (() async -> Void)?
|
||||||
|
|
||||||
func pauseServices(completion: (() -> Void)?) {
|
func pauseServices() async {
|
||||||
pauseServicesCompletionCallsCountLock.withLock { pauseServicesCompletionUnderlyingCallsCount += 1 }
|
pauseServicesCallsCountLock.withLock { pauseServicesUnderlyingCallsCount += 1 }
|
||||||
pauseServicesCompletionClosure?(completion)
|
await pauseServicesClosure?()
|
||||||
}
|
}
|
||||||
//MARK: - expireSyncSessions
|
//MARK: - expireSyncSessions
|
||||||
|
|
||||||
|
|||||||
@@ -431,18 +431,14 @@ class ClientProxy: ClientProxyProtocol {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func pauseServices(completion: (() -> Void)?) {
|
func pauseServices() async {
|
||||||
MXLog.info("Pausing services")
|
MXLog.info("Pausing services")
|
||||||
|
|
||||||
if restartTask != nil {
|
if restartTask != nil {
|
||||||
restartTask = nil
|
restartTask = nil
|
||||||
}
|
}
|
||||||
|
|
||||||
let task = transitionServices(to: .suspended)
|
await transitionServices(to: .suspended).value
|
||||||
Task {
|
|
||||||
await task.value
|
|
||||||
completion?()
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func expireSyncSessions() async {
|
func expireSyncSessions() async {
|
||||||
|
|||||||
@@ -171,7 +171,7 @@ protocol ClientProxyProtocol: AnyObject {
|
|||||||
|
|
||||||
func resumeServices() async
|
func resumeServices() async
|
||||||
|
|
||||||
func pauseServices(completion: (() -> Void)?) // Hopefully this will become async once we get SE-0371.
|
func pauseServices() async
|
||||||
|
|
||||||
func expireSyncSessions() async
|
func expireSyncSessions() async
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user