mirror of
https://github.com/Novaloop-AG/element-x-ios.git
synced 2026-11-04 01:20:12 +00:00
Serialise client pausing and resuming to fix the foreground resume race
Pause and resume previously ran as independent, unsynchronised tasks: `pauseServices` stopped the sync and paused the client from a detached `Task`, while `resumeServices` resumed and started the sync directly. With no ordering between them, a late background pause could land after a foreground resume — leaving the client suspended and the sync stopped while the app was active, which broke everything until the next relaunch. Route both through a single serialised reconciler: each request records the desired state and chains onto the previous transition, and the reconcile always drives the SDK to the *latest* desired state. Transitions can no longer interleave and the most recent intent always wins. On deinit we no longer pause the client — it's being deallocated, so the SDK tears it down anyway; we just stop the sync and cancel the delegate.
This commit is contained in:
@@ -154,7 +154,12 @@ class ClientProxy: ClientProxyProtocol {
|
|||||||
private var hasEncounteredAuthError = false
|
private var hasEncounteredAuthError = false
|
||||||
|
|
||||||
deinit {
|
deinit {
|
||||||
pauseServices { [delegateHandle] in
|
// Stop the sync and cancel the delegate (in that order) before the client is released. No need to pause
|
||||||
|
// the client as it's being deallocated, so the SDK tears it down anyway. Capture the sync service strongly
|
||||||
|
// as `self` is being deallocated.
|
||||||
|
Task { [syncService, delegateHandle] in
|
||||||
|
await syncService.stop()
|
||||||
|
|
||||||
// The delegate handle needs to be cancelled always after the sync stops
|
// The delegate handle needs to be cancelled always after the sync stops
|
||||||
delegateHandle?.cancel()
|
delegateHandle?.cancel()
|
||||||
}
|
}
|
||||||
@@ -402,21 +407,7 @@ class ClientProxy: ClientProxyProtocol {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if appSettings.clientPausingAndResumingEnabled {
|
await transitionServices(to: .running).value
|
||||||
do {
|
|
||||||
MXLog.info("Resuming client")
|
|
||||||
try await client.resume()
|
|
||||||
} catch {
|
|
||||||
MXLog.error("Failed resuming client with error: \(error)")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
MXLog.info("Starting sync")
|
|
||||||
await syncService.start()
|
|
||||||
|
|
||||||
// If we are using OAuth we want to cache the account management URL in volatile memory on the SDK side.
|
|
||||||
// To avoid the cache being invalidated while the app is backgrounded, we cache at every sync start.
|
|
||||||
await cacheAccountURL()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A stored task for restarting the sync after a failure. This is stored so that we can cancel
|
/// A stored task for restarting the sync after a failure. This is stored so that we can cancel
|
||||||
@@ -447,28 +438,11 @@ class ClientProxy: ClientProxyProtocol {
|
|||||||
restartTask = nil
|
restartTask = nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Capture the sync service strongly as this method is called on deinit and so the
|
let task = transitionServices(to: .suspended)
|
||||||
// existence of self when the Task executes is questionable and would sometimes crash.
|
Task {
|
||||||
// Note: This isn't strictly necessary now given the unwrap above, but leaving the code as
|
await task.value
|
||||||
// documentation. SE-0371 will allow us to fix this by using an async deinit.
|
|
||||||
Task { [syncService] in
|
|
||||||
defer {
|
|
||||||
completion?()
|
completion?()
|
||||||
}
|
}
|
||||||
|
|
||||||
MXLog.info("Stopping sync")
|
|
||||||
await syncService.stop()
|
|
||||||
MXLog.info("Sync stopped")
|
|
||||||
|
|
||||||
if appSettings.clientPausingAndResumingEnabled {
|
|
||||||
do {
|
|
||||||
MXLog.info("Pausing client")
|
|
||||||
try await client.pause()
|
|
||||||
} catch {
|
|
||||||
MXLog.error("Failed pausing client with error: \(error)")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func expireSyncSessions() async {
|
func expireSyncSessions() async {
|
||||||
@@ -1084,6 +1058,67 @@ class ClientProxy: ClientProxyProtocol {
|
|||||||
.store(in: &cancellables)
|
.store(in: &cancellables)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// MARK: - Pausing and resuming
|
||||||
|
|
||||||
|
private enum ServiceState { case running, suspended }
|
||||||
|
|
||||||
|
/// The state the sync service and client *should* be in, updated by ``resumeServices()`` and ``pauseServices()``.
|
||||||
|
private var desiredServiceState: ServiceState = .suspended
|
||||||
|
|
||||||
|
/// Serialises service state transitions so that pause and resume never interleave. Each request chains onto
|
||||||
|
/// the previous one and the reconcile always drives the SDK to the *latest* desired state, so a late background
|
||||||
|
/// pause can no longer strand the client suspended after a foreground resume.
|
||||||
|
private var serviceStateTask: Task<Void, Never>?
|
||||||
|
|
||||||
|
@discardableResult
|
||||||
|
private func transitionServices(to state: ServiceState) -> Task<Void, Never> {
|
||||||
|
desiredServiceState = state
|
||||||
|
|
||||||
|
let previousTask = serviceStateTask
|
||||||
|
let task = Task { [weak self] in
|
||||||
|
await previousTask?.value
|
||||||
|
await self?.reconcileServiceState()
|
||||||
|
}
|
||||||
|
serviceStateTask = task
|
||||||
|
return task
|
||||||
|
}
|
||||||
|
|
||||||
|
private func reconcileServiceState() async {
|
||||||
|
switch desiredServiceState {
|
||||||
|
case .running:
|
||||||
|
if appSettings.clientPausingAndResumingEnabled {
|
||||||
|
do {
|
||||||
|
MXLog.info("Resuming client")
|
||||||
|
try await client.resume()
|
||||||
|
} catch {
|
||||||
|
MXLog.error("Failed resuming client with error: \(error)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
MXLog.info("Starting sync")
|
||||||
|
await syncService.start()
|
||||||
|
|
||||||
|
// If we are using OAuth we want to cache the account management URL in volatile memory on the SDK side.
|
||||||
|
// To avoid the cache being invalidated while the app is backgrounded, we cache at every sync start.
|
||||||
|
await cacheAccountURL()
|
||||||
|
case .suspended:
|
||||||
|
MXLog.info("Stopping sync")
|
||||||
|
await syncService.stop()
|
||||||
|
MXLog.info("Sync stopped")
|
||||||
|
|
||||||
|
if appSettings.clientPausingAndResumingEnabled {
|
||||||
|
do {
|
||||||
|
MXLog.info("Pausing client")
|
||||||
|
try await client.pause()
|
||||||
|
} catch {
|
||||||
|
MXLog.error("Failed pausing client with error: \(error)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// MARK: - Other
|
||||||
|
|
||||||
private func cacheAccountURL() async {
|
private func cacheAccountURL() async {
|
||||||
// Calling this function for the first time will cache the account URL in volatile memory for 24 hrs on the SDK.
|
// Calling this function for the first time will cache the account URL in volatile memory for 24 hrs on the SDK.
|
||||||
_ = try? await client.accountUrl(action: nil)
|
_ = try? await client.accountUrl(action: nil)
|
||||||
|
|||||||
Reference in New Issue
Block a user