From 85d497a4d2f9a212a93328a0c74005a2c76b7c20 Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Mon, 4 Nov 2024 17:21:54 +0000 Subject: [PATCH] Stop the sync loop after each background app refresh. (#3481) --- ElementX/Sources/Application/AppCoordinator.swift | 8 ++++++-- ElementX/Sources/Services/Client/ClientProxy.swift | 11 ++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index b46a3aad5d..eec3b77f94 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -988,7 +988,8 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg // This is important for the app to keep refreshing in the background scheduleBackgroundAppRefresh() - task.expirationHandler = { + task.expirationHandler = { [weak self] in + self?.stopSync() // Attempt to stop the sync loop cleanly. MXLog.info("Background app refresh task expired") task.setTaskCompleted(success: true) } @@ -1007,8 +1008,11 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg .collect(.byTimeOrCount(DispatchQueue.main, .seconds(10), 10)) .sink(receiveValue: { [weak self] _ in guard let self else { return } - MXLog.info("Background app refresh finished") + + // 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() backgroundRefreshSyncObserver?.cancel() task.setTaskCompleted(success: true) diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index 3db90f896b..5d6236a0be 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -306,15 +306,24 @@ class ClientProxy: ClientProxyProtocol { restartTask = nil } + guard let syncService else { + MXLog.warning("No sync service to stop.") + completion?() + return + } + // Capture the sync service strongly as this method is called on deinit and so the // existence of self when the Task executes is questionable and would sometimes crash. + // Note: This isn't strictly necessary now given the unwrap above, but leaving the code as + // documentation. SE-0371 will allow us to fix this by using an async deinit. Task { [syncService] in do { defer { completion?() } - try await syncService?.stop() + try await syncService.stop() + MXLog.info("Sync stopped") } catch { MXLog.error("Failed stopping the sync service with error: \(error)") }