From 4435240d2bbe1104739d67efa300c679f0c665b4 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 7 Mar 2025 12:00:45 +1100 Subject: [PATCH] Fixed a number of background processing and polling issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Tweaked some background poller logs • Refactored the `BackgroundFetch` handling to use a `DispatchSourceTimer` instead of `NSTimer` and use a specific `DispatchQueue` to avoid race conditions • Refactored the BackgroundTaskManager to use `DispatchQueue` and `DispatchSourceTimer` and removed unused code (was seeing background tasks incorrectly running too long so wanted to clean it up) • Fixed an issue where pollers would incorrectly be released during background polling • Fixed an issue where the background poller wouldn't update the app notification badge count • Fixed an issue where the community pollers 'timeSinceLastPoll' was incorrectly being given both an epoch timestamp as well as a duration since the last poll (resulting in always just refetching recent messages) • Fixed an issue where the community poller wasn't updating the last poll timestamp (also renamed some functions to make them clearer) • Fixed an issue where pollers could incorrectly be started in the background (eg. when receiving a PN) --- Session/Home/HomeVC.swift | 2 +- Session/Meta/AppDelegate.swift | 71 +++-- .../PushRegistrationManager.swift | 2 +- Session/Utilities/BackgroundPoller.swift | 63 ++-- .../Open Groups/OpenGroupManager.swift | 25 +- .../Pollers/CommunityPoller.swift | 14 +- .../Pollers/PollerType.swift | 11 +- .../Open Groups/OpenGroupManagerSpec.swift | 43 ++- .../Pollers/CommunityPollerSpec.swift | 11 +- .../_TestUtilities/MockOGMCache.swift | 8 +- .../Types/BackgroundTaskManager.swift | 296 ++++++++---------- 11 files changed, 293 insertions(+), 253 deletions(-) diff --git a/Session/Home/HomeVC.swift b/Session/Home/HomeVC.swift index 5419046d5..f4eb61731 100644 --- a/Session/Home/HomeVC.swift +++ b/Session/Home/HomeVC.swift @@ -353,7 +353,7 @@ public final class HomeVC: BaseVC, LibSessionRespondingViewController, UITableVi if Identity.userExists(using: viewModel.dependencies), let appDelegate: AppDelegate = UIApplication.shared.delegate as? AppDelegate, - !viewModel.dependencies[singleton: .appContext].isNotInForeground + viewModel.dependencies[singleton: .appContext].isMainAppAndActive { appDelegate.startPollersIfNeeded() } diff --git a/Session/Meta/AppDelegate.swift b/Session/Meta/AppDelegate.swift index e39048605..c2df5b9bb 100644 --- a/Session/Meta/AppDelegate.swift +++ b/Session/Meta/AppDelegate.swift @@ -318,26 +318,35 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD // MARK: - Background Fetching func application(_ application: UIApplication, performFetchWithCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void) { + /// It seems like it's possible for this function to be called with an invalid `backgroundTimeRemaining` value + /// (`TimeInterval.greatestFiniteMagnitude`) in which case we just want to mark it as a failure + /// + /// Additionally we want to ensure that our timeout timer has enough time to run so make sure we have at least `5 seconds` + /// of background execution (if we don't then the process could incorrectly run longer than it should) + guard + application.backgroundTimeRemaining < TimeInterval.greatestFiniteMagnitude && + application.backgroundTimeRemaining > 5 + else { return completionHandler(.failed) } + Log.appResumedExecution() Log.info(.backgroundPoller, "Starting background fetch.") dependencies[singleton: .storage].resumeDatabaseAccess() dependencies.mutate(cache: .libSessionNetwork) { $0.resumeNetworkAccess() } - let queue: DispatchQueue = .global(qos: .userInitiated) + let queue: DispatchQueue = DispatchQueue(label: "com.session.backgroundPoll") let poller: BackgroundPoller = BackgroundPoller() var cancellable: AnyCancellable? - // Background tasks only last for a certain amount of time (which can result in a crash and a - // prompt appearing for the user), we want to avoid this and need to make sure to suspend the - // database again before the background task ends so we start a timer that expires 1 second - // before the background task is due to expire in order to do so - let cancelTimer: Timer = Timer.scheduledTimerOnMainThread( - withTimeInterval: (application.backgroundTimeRemaining - 5), - repeats: false, - using: dependencies - ) { [poller, dependencies] timer in - timer.invalidate() - + /// Background tasks only last for a certain amount of time (which can result in a crash and a prompt appearing for the user), + /// we want to avoid this and need to make sure to suspend the database again before the background task ends so we start + /// a timer that expires before the background task is due to expire in order to do so + /// + /// **Note:** We **MUST** capture both `poller` and `cancellable` strongly in the event handler to ensure neither + /// go out of scope until we want them to (we essentually want a retain cycle in this case) + let durationRemainingMs: Int = max(1, Int((application.backgroundTimeRemaining - 5) * 1000)) + let timer: DispatchSourceTimer = DispatchSource.makeTimerSource(queue: queue) + timer.schedule(deadline: .now() + .milliseconds(durationRemainingMs)) + timer.setEventHandler { [poller, dependencies] in guard cancellable != nil else { return } Log.info(.backgroundPoller, "Background poll failed due to manual timeout.") @@ -352,32 +361,49 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD _ = poller // Capture poller to ensure it doesn't go out of scope completionHandler(.failed) } + timer.resume() dependencies[singleton: .appReadiness].runNowOrWhenAppDidBecomeReady { [dependencies, poller] in - // If the 'AppReadiness' process takes too long then it's possible for the user to open - // the app after this closure is registered but before it's actually triggered - this can - // result in the `BackgroundPoller` incorrectly getting called in the foreground, this check - // is here to prevent that + /// If the 'AppReadiness' process takes too long then it's possible for the user to open the app after this closure is registered + /// but before it's actually triggered - this can result in the `BackgroundPoller` incorrectly getting called in the foreground, + /// this check is here to prevent that guard dependencies[singleton: .appContext].isInBackground else { return } + /// Kick off the `BackgroundPoller` + /// + /// **Note:** We **MUST** capture both `poller` and `timer` strongly in the completion handler to ensure neither + /// go out of scope until we want them to (we essentually want a retain cycle in this case) cancellable = poller .poll(using: dependencies) .subscribe(on: queue, using: dependencies) - .receive(on: DispatchQueue.main, using: dependencies) + .receive(on: queue, using: dependencies) .sink( - receiveCompletion: { [poller] result in + receiveCompletion: { [timer, poller] result in // Ensure we haven't timed out yet - guard cancelTimer.isValid else { return } + guard timer.isCancelled == false else { return } + + // Immediately cancel the timer to prevent the timeout being triggered + timer.cancel() + // Update the unread count badge + let unreadCount: Int = dependencies[singleton: .storage] + .read { db in try Interaction.fetchAppBadgeUnreadCount(db, using: dependencies) } + .defaulting(to: 0) + + DispatchQueue.main.async(using: dependencies) { + UIApplication.shared.applicationIconBadgeNumber = unreadCount + } + + // If we are still running in the background then suspend the network & database if dependencies[singleton: .appContext].isInBackground { dependencies.mutate(cache: .libSessionNetwork) { $0.suspendNetworkAccess() } dependencies[singleton: .storage].suspendDatabaseAccess() Log.flush() } - cancelTimer.invalidate() _ = poller // Capture poller to ensure it doesn't go out of scope + // Complete the background task switch result { case .failure: completionHandler(.failed) case .finished: completionHandler(.newData) @@ -850,16 +876,13 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD // MARK: - Polling - public func startPollersIfNeeded(shouldStartGroupPollers: Bool = true) { + public func startPollersIfNeeded() { guard dependencies[cache: .onboarding].state == .completed else { return } /// Start the pollers on a background thread so that any database queries they need to run don't /// block the main thread DispatchQueue.global(qos: .background).async { [dependencies] in dependencies[singleton: .currentUserPoller].startIfNeeded() - - guard shouldStartGroupPollers else { return } - dependencies.mutate(cache: .groupPollers) { $0.startAllPollers() } dependencies.mutate(cache: .communityPollers) { $0.startAllPollers() } } diff --git a/Session/Notifications/PushRegistrationManager.swift b/Session/Notifications/PushRegistrationManager.swift index a8bcbd240..96f5d850b 100644 --- a/Session/Notifications/PushRegistrationManager.swift +++ b/Session/Notifications/PushRegistrationManager.swift @@ -315,7 +315,7 @@ public class PushRegistrationManager: NSObject, PKPushRegistryDelegate { dependencies[singleton: .jobRunner].appDidBecomeActive() // NOTE: Just start 1-1 poller so that it won't wait for polling group messages - (UIApplication.shared.delegate as? AppDelegate)?.startPollersIfNeeded(shouldStartGroupPollers: false) + dependencies[singleton: .currentUserPoller].startIfNeeded(forceStartInBackground: true) call.reportIncomingCallIfNeeded { error in if let error = error { diff --git a/Session/Utilities/BackgroundPoller.swift b/Session/Utilities/BackgroundPoller.swift index 9def82740..bb30cb259 100644 --- a/Session/Utilities/BackgroundPoller.swift +++ b/Session/Utilities/BackgroundPoller.swift @@ -18,11 +18,17 @@ public extension Log.Category { // MARK: - BackgroundPoller public final class BackgroundPoller { + typealias Pollers = ( + currentUser: CurrentUserPoller, + groups: [GroupPoller], + communities: [CommunityPoller] + ) + public func poll(using dependencies: Dependencies) -> AnyPublisher { let pollStart: TimeInterval = dependencies.dateNow.timeIntervalSince1970 return dependencies[singleton: .storage] - .readPublisher { db -> (Set, Set) in + .readPublisher { db -> (Set, Set, [String]) in ( try ClosedGroup .select(.threadId) @@ -46,16 +52,36 @@ public final class BackgroundPoller { ) .distinct() .asRequest(of: String.self) - .fetchSet(db) + .fetchSet(db), + try OpenGroup + .select(.roomToken) + .filter( + OpenGroup.Columns.roomToken != "" && + OpenGroup.Columns.isActive && + OpenGroup.Columns.pollFailureCount < CommunityPoller.maxRoomFailureCountForBackgroundPoll + ) + .distinct() + .asRequest(of: String.self) + .fetchAll(db) ) } - .catch { _ in Just(([], [])).eraseToAnyPublisher() } + .catch { _ in Just(([], [], [])).eraseToAnyPublisher() } .handleEvents( - receiveOutput: { groupIds, servers in - Log.info(.backgroundPoller, "Fetching Users: 1, Groups: \(groupIds.count), Communities: \(servers.count).") + receiveOutput: { groupIds, servers, rooms in + Log.info(.backgroundPoller, "Fetching Users: 1, Groups: \(groupIds.count), Communities: \(servers.count) (\(rooms.count) room(s)).") } ) - .map { groupIds, servers -> ([GroupPoller], [CommunityPoller]) in + .map { groupIds, servers, _ -> Pollers in + let currentUserPoller: CurrentUserPoller = CurrentUserPoller( + pollerName: "Background Main Poller", + pollerQueue: DispatchQueue.main, + pollerDestination: .swarm(dependencies[cache: .general].sessionId.hexString), + pollerDrainBehaviour: .limitedReuse(count: 6), + namespaces: CurrentUserPoller.namespaces, + shouldStoreMessages: true, + logStartAndStopCalls: false, + using: dependencies + ) let groupPollers: [GroupPoller] = groupIds.map { groupId in GroupPoller( pollerName: "Background Group poller for: \(groupId)", // stringlint:ignore @@ -80,16 +106,18 @@ public final class BackgroundPoller { ) } - return (groupPollers, communityPollers) + return (currentUserPoller, groupPollers, communityPollers) } - .flatMap { groupPollers, communityPollers in + .flatMap { currentUserPoller, groupPollers, communityPollers in + /// Need to map back to the pollers to ensure they don't get released until after the polling finishes Publishers.MergeMany( - [BackgroundPoller.pollUserMessages(using: dependencies)] + [BackgroundPoller.pollUserMessages(poller: currentUserPoller, using: dependencies)] .appending(contentsOf: BackgroundPoller.poll(pollers: groupPollers, using: dependencies)) .appending(contentsOf: BackgroundPoller.poll(pollerInfo: communityPollers, using: dependencies)) ) + .collect() + .map { _ in (currentUserPoller, groupPollers, communityPollers) } } - .collect() .map { _ in () } .handleEvents( receiveOutput: { _ in @@ -102,18 +130,9 @@ public final class BackgroundPoller { } private static func pollUserMessages( + poller: CurrentUserPoller, using dependencies: Dependencies ) -> AnyPublisher { - let poller: CurrentUserPoller = CurrentUserPoller( - pollerName: "Background Main Poller", - pollerQueue: DispatchQueue.main, - pollerDestination: .swarm(dependencies[cache: .general].sessionId.hexString), - pollerDrainBehaviour: .limitedReuse(count: 6), - namespaces: CurrentUserPoller.namespaces, - shouldStoreMessages: true, - logStartAndStopCalls: false, - using: dependencies - ) let pollStart: TimeInterval = dependencies.dateNow.timeIntervalSince1970 return poller @@ -182,10 +201,10 @@ public final class BackgroundPoller { return poller .pollFromBackground() .handleEvents( - receiveOutput: { [pollerName = poller.pollerName] _ in + receiveOutput: { [pollerName = poller.pollerName] _, _, rawMessageCount, _ in let endTime: TimeInterval = dependencies.dateNow.timeIntervalSince1970 let duration: TimeUnit = .seconds(endTime - pollStart) - Log.info(.backgroundPoller, "\(pollerName) succeeded after \(duration, unit: .s).") + Log.info(.backgroundPoller, "\(pollerName) received \(rawMessageCount) message(s) succeeded after \(duration, unit: .s).") }, receiveCompletion: { [pollerName = poller.pollerName] result in switch result { diff --git a/SessionMessagingKit/Open Groups/OpenGroupManager.swift b/SessionMessagingKit/Open Groups/OpenGroupManager.swift index 2a3486a62..41f2a4b2c 100644 --- a/SessionMessagingKit/Open Groups/OpenGroupManager.swift +++ b/SessionMessagingKit/Open Groups/OpenGroupManager.swift @@ -935,7 +935,7 @@ public extension OpenGroupManager { class Cache: OGMCacheType { private let dependencies: Dependencies private let defaultRoomsSubject: CurrentValueSubject<[DefaultRoomInfo], Error> = CurrentValueSubject([]) - private var _timeSinceLastOpen: TimeInterval? + private var _lastSuccessfulCommunityPollTimestamp: TimeInterval? public var pendingChanges: [OpenGroupAPI.PendingChange] = [] public var defaultRoomsPublisher: AnyPublisher<[DefaultRoomInfo], Error> { @@ -961,18 +961,22 @@ public extension OpenGroupManager { // MARK: - Functions - public func getTimeSinceLastOpen(using dependencies: Dependencies) -> TimeInterval { - if let storedTimeSinceLastOpen: TimeInterval = _timeSinceLastOpen { - return storedTimeSinceLastOpen + public func getLastSuccessfulCommunityPollTimestamp() -> TimeInterval { + if let storedTime: TimeInterval = _lastSuccessfulCommunityPollTimestamp { + return storedTime } - guard let lastOpen: Date = dependencies[defaults: .standard, key: .lastOpen] else { - _timeSinceLastOpen = .greatestFiniteMagnitude - return .greatestFiniteMagnitude + guard let lastPoll: Date = dependencies[defaults: .standard, key: .lastOpen] else { + return 0 } - _timeSinceLastOpen = dependencies.dateNow.timeIntervalSince(lastOpen) - return dependencies.dateNow.timeIntervalSince(lastOpen) + _lastSuccessfulCommunityPollTimestamp = lastPoll.timeIntervalSince1970 + return lastPoll.timeIntervalSince1970 + } + + public func setLastSuccessfulCommunityPollTimestamp(_ timestamp: TimeInterval) { + dependencies[defaults: .standard, key: .lastOpen] = Date(timeIntervalSince1970: timestamp) + _lastSuccessfulCommunityPollTimestamp = timestamp } public func setDefaultRoomInfo(_ info: [DefaultRoomInfo]) { @@ -995,6 +999,7 @@ public protocol OGMCacheType: OGMImmutableCacheType, MutableCacheType { var pendingChanges: [OpenGroupAPI.PendingChange] { get set } - func getTimeSinceLastOpen(using dependencies: Dependencies) -> TimeInterval + func getLastSuccessfulCommunityPollTimestamp() -> TimeInterval + func setLastSuccessfulCommunityPollTimestamp(_ timestamp: TimeInterval) func setDefaultRoomInfo(_ info: [OpenGroupManager.DefaultRoomInfo]) } diff --git a/SessionMessagingKit/Sending & Receiving/Pollers/CommunityPoller.swift b/SessionMessagingKit/Sending & Receiving/Pollers/CommunityPoller.swift index b78d19f38..ccfbad27a 100644 --- a/SessionMessagingKit/Sending & Receiving/Pollers/CommunityPoller.swift +++ b/SessionMessagingKit/Sending & Receiving/Pollers/CommunityPoller.swift @@ -247,10 +247,10 @@ public final class CommunityPoller: CommunityPollerType & PollerType { /// **Note:** The returned messages will have already been processed by the `Poller`, they are only returned /// for cases where we need explicit/custom behaviours to occur (eg. Onboarding) public func poll(forceSynchronousProcessing: Bool = false) -> AnyPublisher { - let timeSinceLastPoll: TimeInterval = (self.lastPollStart > 0 ? + let lastSuccessfulPollTimestamp: TimeInterval = (self.lastPollStart > 0 ? lastPollStart : dependencies.mutate(cache: .openGroupManager) { cache in - cache.getTimeSinceLastOpen(using: dependencies) + cache.getLastSuccessfulCommunityPollTimestamp() } ) @@ -260,7 +260,7 @@ public final class CommunityPoller: CommunityPollerType & PollerType { db, server: pollerDestination.target, hasPerformedInitialPoll: (pollCount > 0), - timeSinceLastPoll: timeSinceLastPoll, + timeSinceLastPoll: (dependencies.dateNow.timeIntervalSince1970 - lastSuccessfulPollTimestamp), using: dependencies ) } @@ -274,8 +274,14 @@ public final class CommunityPoller: CommunityPollerType & PollerType { ) } .handleEvents( - receiveOutput: { [weak self] _ in + receiveOutput: { [weak self, dependencies] _ in self?.pollCount += 1 + + dependencies.mutate(cache: .openGroupManager) { cache in + cache.setLastSuccessfulCommunityPollTimestamp( + dependencies.dateNow.timeIntervalSince1970 + ) + } } ) .eraseToAnyPublisher() diff --git a/SessionMessagingKit/Sending & Receiving/Pollers/PollerType.swift b/SessionMessagingKit/Sending & Receiving/Pollers/PollerType.swift index 12f8ea1a9..581f03fab 100644 --- a/SessionMessagingKit/Sending & Receiving/Pollers/PollerType.swift +++ b/SessionMessagingKit/Sending & Receiving/Pollers/PollerType.swift @@ -72,7 +72,7 @@ public protocol PollerType: AnyObject { using dependencies: Dependencies ) - func startIfNeeded() + func startIfNeeded(forceStartInBackground: Bool) func stop() func pollerDidStart() @@ -84,7 +84,14 @@ public protocol PollerType: AnyObject { // MARK: - Default Implementations public extension PollerType { - func startIfNeeded() { + func startIfNeeded() { startIfNeeded(forceStartInBackground: false) } + + func startIfNeeded(forceStartInBackground: Bool) { + guard + forceStartInBackground || + dependencies[singleton: .appContext].isMainAppAndActive + else { return Log.info(.poller, "Ignoring call to start \(pollerName) due to not being active.") } + pollerQueue.async(using: dependencies) { [weak self, pollerName] in guard self?.isPolling != true else { return } diff --git a/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift b/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift index 6e8349a17..9edde7635 100644 --- a/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift +++ b/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift @@ -183,6 +183,7 @@ class OpenGroupManagerSpec: QuickSpec { @TestState(defaults: .standard, in: dependencies) var mockUserDefaults: MockUserDefaults! = MockUserDefaults( initialSetup: { defaults in defaults.when { $0.integer(forKey: .any) }.thenReturn(0) + defaults.when { $0.set(.any, forKey: .any) }.thenReturn(()) } ) @TestState(defaults: .appGroup, in: dependencies) var mockAppGroupDefaults: MockUserDefaults! = MockUserDefaults( @@ -199,7 +200,7 @@ class OpenGroupManagerSpec: QuickSpec { initialSetup: { cache in cache.when { $0.pendingChanges }.thenReturn([]) cache.when { $0.pendingChanges = .any }.thenReturn(()) - cache.when { $0.getTimeSinceLastOpen(using: .any) }.thenReturn(0) + cache.when { $0.getLastSuccessfulCommunityPollTimestamp() }.thenReturn(0) cache.when { $0.setDefaultRoomInfo(.any) }.thenReturn(()) } ) @@ -237,20 +238,19 @@ class OpenGroupManagerSpec: QuickSpec { // MARK: -- cache data context("cache data") { - // MARK: ---- defaults the time since last open to greatestFiniteMagnitude - it("defaults the time since last open to greatestFiniteMagnitude") { + // MARK: ---- defaults the time since last open to zero + it("defaults the time since last open to zero") { mockUserDefaults .when { (defaults: inout any UserDefaultsType) -> Any? in defaults.object(forKey: UserDefaults.DateKey.lastOpen.rawValue) } .thenReturn(nil) - expect(cache.getTimeSinceLastOpen(using: dependencies)) - .to(beCloseTo(.greatestFiniteMagnitude)) + expect(cache.getLastSuccessfulCommunityPollTimestamp()).to(equal(0)) } - // MARK: ---- returns the time since the last open - it("returns the time since the last open") { + // MARK: ---- returns the time since the last poll + it("returns the time since the last poll") { mockUserDefaults .when { (defaults: inout any UserDefaultsType) -> Any? in defaults.object(forKey: UserDefaults.DateKey.lastOpen.rawValue) @@ -258,12 +258,12 @@ class OpenGroupManagerSpec: QuickSpec { .thenReturn(Date(timeIntervalSince1970: 1234567880)) dependencies.dateNow = Date(timeIntervalSince1970: 1234567890) - expect(cache.getTimeSinceLastOpen(using: dependencies)) - .to(beCloseTo(10)) + expect(cache.getLastSuccessfulCommunityPollTimestamp()) + .to(equal(1234567880)) } - // MARK: ---- caches the time since the last open - it("caches the time since the last open") { + // MARK: ---- caches the time since the last poll in memory + it("caches the time since the last poll in memory") { mockUserDefaults .when { (defaults: inout any UserDefaultsType) -> Any? in defaults.object(forKey: UserDefaults.DateKey.lastOpen.rawValue) @@ -271,8 +271,8 @@ class OpenGroupManagerSpec: QuickSpec { .thenReturn(Date(timeIntervalSince1970: 1234567770)) dependencies.dateNow = Date(timeIntervalSince1970: 1234567780) - expect(cache.getTimeSinceLastOpen(using: dependencies)) - .to(beCloseTo(10)) + expect(cache.getLastSuccessfulCommunityPollTimestamp()) + .to(equal(1234567770)) mockUserDefaults .when { (defaults: inout any UserDefaultsType) -> Any? in @@ -281,8 +281,21 @@ class OpenGroupManagerSpec: QuickSpec { .thenReturn(Date(timeIntervalSince1970: 1234567890)) // Cached value shouldn't have been updated - expect(cache.getTimeSinceLastOpen(using: dependencies)) - .to(beCloseTo(10)) + expect(cache.getLastSuccessfulCommunityPollTimestamp()) + .to(equal(1234567770)) + } + + // MARK: ---- updates the time since the last poll in user defaults + it("updates the time since the last poll in user defaults") { + cache.setLastSuccessfulCommunityPollTimestamp(12345) + + expect(mockUserDefaults) + .to(call(matchingParameters: .all) { + $0.set( + Date(timeIntervalSince1970: 12345), + forKey: UserDefaults.DateKey.lastOpen.rawValue + ) + }) } } diff --git a/SessionMessagingKitTests/Sending & Receiving/Pollers/CommunityPollerSpec.swift b/SessionMessagingKitTests/Sending & Receiving/Pollers/CommunityPollerSpec.swift index d66a07ad8..ebbbaae29 100644 --- a/SessionMessagingKitTests/Sending & Receiving/Pollers/CommunityPollerSpec.swift +++ b/SessionMessagingKitTests/Sending & Receiving/Pollers/CommunityPollerSpec.swift @@ -69,6 +69,11 @@ class CommunityPollerSpec: QuickSpec { ) } ) + @TestState(singleton: .appContext, in: dependencies) var mockAppContext: MockAppContext! = MockAppContext( + initialSetup: { context in + context.when { $0.isMainAppAndActive }.thenReturn(false) + } + ) @TestState(defaults: .standard, in: dependencies) var mockUserDefaults: MockUserDefaults! = MockUserDefaults() @TestState(cache: .general, in: dependencies) var mockGeneralCache: MockGeneralCache! = MockGeneralCache( initialSetup: { cache in @@ -78,7 +83,7 @@ class CommunityPollerSpec: QuickSpec { @TestState(cache: .openGroupManager, in: dependencies) var mockOGMCache: MockOGMCache! = MockOGMCache( initialSetup: { cache in cache.when { $0.pendingChanges }.thenReturn([]) - cache.when { $0.getTimeSinceLastOpen(using: .any) }.thenReturn(0) + cache.when { $0.getLastSuccessfulCommunityPollTimestamp() }.thenReturn(0) } ) @TestState var cache: CommunityPoller.Cache! = CommunityPoller.Cache(using: dependencies) @@ -87,6 +92,10 @@ class CommunityPollerSpec: QuickSpec { describe("a CommunityPollerCache") { // MARK: -- when starting polling context("when starting polling") { + beforeEach { + mockAppContext.when { $0.isMainAppAndActive }.thenReturn(true) + } + // MARK: ---- creates pollers for all of the communities it("creates pollers for all of the communities") { cache.startAllPollers() diff --git a/SessionMessagingKitTests/_TestUtilities/MockOGMCache.swift b/SessionMessagingKitTests/_TestUtilities/MockOGMCache.swift index 103b644ad..a7db4da5d 100644 --- a/SessionMessagingKitTests/_TestUtilities/MockOGMCache.swift +++ b/SessionMessagingKitTests/_TestUtilities/MockOGMCache.swift @@ -16,8 +16,12 @@ class MockOGMCache: Mock, OGMCacheType { set { mockNoReturn(args: [newValue]) } } - func getTimeSinceLastOpen(using dependencies: Dependencies) -> TimeInterval { - return mock(args: [dependencies]) + func getLastSuccessfulCommunityPollTimestamp() -> TimeInterval { + return mock() + } + + func setLastSuccessfulCommunityPollTimestamp(_ timestamp: TimeInterval) { + mockNoReturn(args: [timestamp]) } func setDefaultRoomInfo(_ info: [OpenGroupManager.DefaultRoomInfo]) { diff --git a/SessionUtilitiesKit/Types/BackgroundTaskManager.swift b/SessionUtilitiesKit/Types/BackgroundTaskManager.swift index 43252b6fa..bf099a6b7 100644 --- a/SessionUtilitiesKit/Types/BackgroundTaskManager.swift +++ b/SessionUtilitiesKit/Types/BackgroundTaskManager.swift @@ -23,7 +23,11 @@ public enum SessionBackgroundTaskState { // MARK: - SessionBackgroundTaskManager public class SessionBackgroundTaskManager { + /// Maximum duration to extend background tasks + private static let maxBackgroundTime: TimeInterval = 180 + private let dependencies: Dependencies + private let queue = DispatchQueue(label: "com.session.backgroundTaskManager") /// This property should only be accessed while synchronized on this instance. private var backgroundTaskId: UIBackgroundTaskIdentifier = .invalid @@ -43,11 +47,11 @@ public class SessionBackgroundTaskManager { /// begins, we use a single uninterrupted background that spans their lifetimes. /// /// This property should only be accessed while synchronized on this instance. - private var continuityTimer: Timer? + private var continuityTimer: DispatchSourceTimer? /// In order to ensure we have sufficient time to clean up before background tasks expire (without having to kick off additional tasks) /// we track the remaining background execution time and end tasks 5 seconds early (same as the AppDelegate background fetch) - private var expirationTimeObserver: Timer? + private var expirationTimeObserver: DispatchSourceTimer? private var hasGottenValidBackgroundTimeRemaining: Bool = false fileprivate init(using dependencies: Dependencies) { @@ -61,13 +65,6 @@ public class SessionBackgroundTaskManager { // MARK: - Functions - @discardableResult private static func synced(_ lock: Any, closure: () -> T) -> T { - objc_sync_enter(lock) - let result: T = closure() - objc_sync_exit(lock) - return result - } - public func startObservingNotifications() { guard dependencies[singleton: .appContext].isMainApp else { return } @@ -86,14 +83,14 @@ public class SessionBackgroundTaskManager { } @objc private func applicationDidBecomeActive() { - SessionBackgroundTaskManager.synced(self) { [weak self] in + queue.sync { [weak self] in self?.isAppActive = true self?.ensureBackgroundTaskState() } } @objc private func applicationWillResignActive() { - SessionBackgroundTaskManager.synced(self) { [weak self] in + queue.sync { [weak self] in self?.isAppActive = false self?.ensureBackgroundTaskState() } @@ -109,7 +106,7 @@ public class SessionBackgroundTaskManager { // background task, but the background task couldn't be begun. // In that case expirationBlock will not be called. fileprivate func addTask(expiration: @escaping () -> ()) -> UInt64? { - return SessionBackgroundTaskManager.synced(self) { [weak self, dependencies] in + return queue.sync { [weak self] () -> UInt64? in let taskId: UInt64 = ((self?.idCounter ?? 0) + 1) self?.idCounter = taskId self?.expirationMap[taskId] = expiration @@ -118,18 +115,15 @@ public class SessionBackgroundTaskManager { self?.expirationMap.removeValue(forKey: taskId) } - self?.continuityTimer?.invalidate() - self?.continuityTimer = nil + if self?.continuityTimer != nil { + self?.continuityTimer?.cancel() + self?.continuityTimer = nil + } // Start observing the background time remaining - if self?.expirationTimeObserver?.isValid != true { + if self?.expirationTimeObserver?.isCancelled == true { self?.hasGottenValidBackgroundTimeRemaining = false - self?.expirationTimeObserver = Timer.scheduledTimerOnMainThread( - withTimeInterval: 1, - repeats: true, - using: dependencies, - block: { _ in self?.expirationTimerDidFire() } - ) + self?.checkExpirationTime(in: .seconds(1)) // Don't know the remaining time so check soon } return taskId @@ -139,7 +133,7 @@ public class SessionBackgroundTaskManager { fileprivate func removeTask(taskId: UInt64?) { guard let taskId: UInt64 = taskId else { return } - SessionBackgroundTaskManager.synced(self) { [weak self, dependencies] in + queue.sync { [weak self, queue] in self?.expirationMap.removeValue(forKey: taskId) // This timer will ensure that we keep the background task active (if necessary) @@ -148,87 +142,93 @@ public class SessionBackgroundTaskManager { // should be able to ensure background tasks by "narrowly" wrapping // their core logic with a SessionBackgroundTask and not worrying about "hand off" // between SessionBackgroundTasks. - self?.continuityTimer?.invalidate() - self?.continuityTimer = Timer.scheduledTimerOnMainThread( - withTimeInterval: 0.25, - using: dependencies, - block: { _ in self?.continuityTimerDidFire() } - ) + if self?.continuityTimer != nil { + self?.continuityTimer?.cancel() + self?.continuityTimer = nil + } + + self?.continuityTimer = DispatchSource.makeTimerSource(queue: queue) + self?.continuityTimer?.schedule(deadline: .now() + .milliseconds(250)) + self?.continuityTimer?.setEventHandler { self?.continuityTimerDidFire() } + self?.continuityTimer?.resume() self?.ensureBackgroundTaskState() } } - /// Begins or end a background task if necessary. + /// Begins or end a background task if necessary + /// + /// **Note:** Should only be called internally within `queue.sync` for thread safety @discardableResult private func ensureBackgroundTaskState() -> Bool { // We can't create background tasks in the SAE, but pretend that we succeeded. guard dependencies[singleton: .appContext].isMainApp else { return true } - return SessionBackgroundTaskManager.synced(self) { [weak self, dependencies] in - // We only want to have a background task if we are: - // a) "not active" AND - // b1) there is one or more active instance of SessionBackgroundTask OR... - // b2) ...there _was_ an active instance recently. - let shouldHaveBackgroundTask: Bool = ( - self?.isAppActive == false && ( - (self?.expirationMap.count ?? 0) > 0 || - self?.continuityTimer != nil - ) + // We only want to have a background task if we are: + // a) "not active" AND + // b1) there is one or more active instance of SessionBackgroundTask OR... + // b2) ...there _was_ an active instance recently. + let shouldHaveBackgroundTask: Bool = ( + self.isAppActive == false && ( + self.expirationMap.count > 0 || + self.continuityTimer != nil ) - let hasBackgroundTask: Bool = (self?.backgroundTaskId != .invalid) - - guard shouldHaveBackgroundTask != hasBackgroundTask else { - // Current state is correct - return true - } - guard !shouldHaveBackgroundTask else { - return (self?.startBackgroundTask() == true) - } - - // Need to end background task. - let maybeBackgroundTaskId: UIBackgroundTaskIdentifier? = self?.backgroundTaskId - self?.backgroundTaskId = .invalid - self?.expirationTimeObserver?.invalidate() - self?.expirationTimeObserver = nil - - if let backgroundTaskId: UIBackgroundTaskIdentifier = maybeBackgroundTaskId, backgroundTaskId != .invalid { - dependencies[singleton: .appContext].endBackgroundTask(backgroundTaskId) - } - + ) + let hasBackgroundTask: Bool = (self.backgroundTaskId != .invalid) + + guard shouldHaveBackgroundTask != hasBackgroundTask else { + // Current state is correct return true } + guard !shouldHaveBackgroundTask else { + return (self.startOverarchingBackgroundTask() == true) + } + + // Need to end background task. + let maybeBackgroundTaskId: UIBackgroundTaskIdentifier? = self.backgroundTaskId + self.backgroundTaskId = .invalid + + if self.expirationTimeObserver != nil { + self.expirationTimeObserver?.cancel() + self.expirationTimeObserver = nil + } + + if let backgroundTaskId: UIBackgroundTaskIdentifier = maybeBackgroundTaskId, backgroundTaskId != .invalid { + dependencies[singleton: .appContext].endBackgroundTask(backgroundTaskId) + } + + return true } - /// Returns `false` if the background task cannot be begun. - private func startBackgroundTask() -> Bool { + /// Returns `false` if the background task cannot be begun + /// + /// **Note:** Should only be called internally within `queue.sync` for thread safety + private func startOverarchingBackgroundTask() -> Bool { guard dependencies[singleton: .appContext].isMainApp else { return false } - return SessionBackgroundTaskManager.synced(self) { [weak self, dependencies] in - self?.backgroundTaskId = dependencies[singleton: .appContext].beginBackgroundTask { - /// Supposedly `[UIApplication beginBackgroundTaskWithExpirationHandler]`'s handler - /// will always be called on the main thread, but in practice we've observed otherwise. - /// - /// See: - /// https://developer.apple.com/documentation/uikit/uiapplication/1623031-beginbackgroundtaskwithexpiratio) + self.backgroundTaskId = dependencies[singleton: .appContext].beginBackgroundTask { [weak self] in + /// Supposedly `[UIApplication beginBackgroundTaskWithExpirationHandler]`'s handler + /// will always be called on the main thread, but in practice we've observed otherwise. + /// + /// See: + /// https://developer.apple.com/documentation/uikit/uiapplication/1623031-beginbackgroundtaskwithexpiratio) + self?.queue.sync { self?.backgroundTaskExpired() } - - // If the background task could not begin, return false to indicate that - return (self?.backgroundTaskId != .invalid) } + + // If the background task could not begin, return false to indicate that + return (self.backgroundTaskId != .invalid) } + /// **Note:** Should only be called internally within `queue.sync` for thread safety private func backgroundTaskExpired() { - var backgroundTaskId: UIBackgroundTaskIdentifier = .invalid - var expirationMap: [UInt64: () -> ()] = [:] + let backgroundTaskId: UIBackgroundTaskIdentifier = self.backgroundTaskId + let expirationMap: [UInt64: () -> ()] = self.expirationMap + self.backgroundTaskId = .invalid + self.expirationMap.removeAll() - SessionBackgroundTaskManager.synced(self) { [weak self] in - backgroundTaskId = (self?.backgroundTaskId ?? .invalid) - self?.backgroundTaskId = .invalid - self?.expirationTimeObserver?.invalidate() - self?.expirationTimeObserver = nil - - expirationMap = (self?.expirationMap ?? [:]) - self?.expirationMap.removeAll() + if self.expirationTimeObserver != nil { + self.expirationTimeObserver?.cancel() + self.expirationTimeObserver = nil } /// Supposedly `[UIApplication beginBackgroundTaskWithExpirationHandler]`'s handler @@ -247,33 +247,44 @@ public class SessionBackgroundTaskManager { } } + private func checkExpirationTime(in interval: DispatchTimeInterval) { + expirationTimeObserver = DispatchSource.makeTimerSource(queue: queue) + expirationTimeObserver?.schedule(deadline: .now() + interval) + expirationTimeObserver?.setEventHandler { [weak self] in self?.expirationTimerDidFire() } + expirationTimeObserver?.resume() + } + + /// Timer will always fire on the `queue` so no need to `queue.sync` private func continuityTimerDidFire() { - SessionBackgroundTaskManager.synced(self) { [weak self] in - self?.continuityTimer?.invalidate() - self?.continuityTimer = nil - self?.ensureBackgroundTaskState() - } + continuityTimer = nil + ensureBackgroundTaskState() } + /// Timer will always fire on the `queue` so no need to `queue.sync` private func expirationTimerDidFire() { + expirationTimeObserver = nil + guard dependencies[singleton: .appContext].isMainApp else { return } let backgroundTimeRemaining: TimeInterval = dependencies[singleton: .appContext].backgroundTimeRemaining - SessionBackgroundTaskManager.synced(self) { [weak self] in - // It takes the OS a little while to update the 'backgroundTimeRemaining' value so if it hasn't been updated - // yet then don't do anything - guard self?.hasGottenValidBackgroundTimeRemaining == true || backgroundTimeRemaining != .greatestFiniteMagnitude else { - return - } - - self?.hasGottenValidBackgroundTimeRemaining = true - - // If there is more than 5 seconds remaining then no need to do anything yet (plenty of time to continue running) - guard backgroundTimeRemaining <= 5 else { return } - - // There isn't a lot of time remaining so trigger the expiration - self?.backgroundTaskExpired() + /// It takes the OS a little while to update the 'backgroundTimeRemaining' value so if it hasn't been updated yet then don't do anything + guard self.hasGottenValidBackgroundTimeRemaining == true || backgroundTimeRemaining != .greatestFiniteMagnitude else { + self.checkExpirationTime(in: .seconds(1)) + return + } + + self.hasGottenValidBackgroundTimeRemaining = true + + switch backgroundTimeRemaining { + /// There is more than 10 seconds remaining so no need to do anything yet (plenty of time to continue running) + case 10...: self.checkExpirationTime(in: .seconds(5)) + + /// There is between 5 and 10 seconds so poll more frequently just in case + case 5..<10: self.checkExpirationTime(in: .milliseconds(2500)) + + /// There isn't a lot of time remaining so trigger the expiration + default: self.backgroundTaskExpired() } } } @@ -282,8 +293,6 @@ public class SessionBackgroundTaskManager { public class SessionBackgroundTask { private let dependencies: Dependencies - - /// This property should only be accessed while synchronized on this instance private var taskId: UInt64? private let label: String private var completion: ((SessionBackgroundTaskState) -> ())? @@ -308,86 +317,31 @@ public class SessionBackgroundTask { // MARK: - Functions - @discardableResult private static func synced(_ lock: Any, closure: () -> T) -> T { - objc_sync_enter(lock) - let result: T = closure() - objc_sync_exit(lock) - return result - } - private func startBackgroundTask() { - // Make a local copy of completion to ensure that it is called exactly once - var completion: ((SessionBackgroundTaskState) -> ())? - - self.taskId = dependencies[singleton: .backgroundTaskManager].addTask { [weak self] in - Threading.dispatchMainThreadSafe { - guard let strongSelf = self else { return } - - SessionBackgroundTask.synced(strongSelf) { - self?.taskId = nil - completion = self?.completion - self?.completion = nil - } - - completion?(.expired) - } - } - - // If we didn't get a taskId then the background task could not be started so - // we should call the completion block with a 'couldNotStart' error - guard taskId == nil else { return } - - SessionBackgroundTask.synced(self) { [weak self] in - completion = self?.completion - self?.completion = nil + taskId = dependencies[singleton: .backgroundTaskManager].addTask { [weak self] in + self?.taskExpired() } - if completion != nil { - Threading.dispatchMainThreadSafe { - completion?(.couldNotStart) - } + if taskId == nil { + completion?(.couldNotStart) + completion = nil } } public func cancel() { guard taskId != nil else { return } - // Make a local copy of completion to ensure that it is called exactly once - var completion: ((SessionBackgroundTaskState) -> ())? - - SessionBackgroundTask.synced(self) { [weak self, dependencies] in - dependencies[singleton: .backgroundTaskManager].removeTask(taskId: self?.taskId) - completion = self?.completion - self?.taskId = nil - self?.completion = nil - } - - // endBackgroundTask must be called on the main thread. - if completion != nil { - Threading.dispatchMainThreadSafe { - completion?(.cancelled) - } - } + dependencies[singleton: .backgroundTaskManager].removeTask(taskId: taskId) + completion?(.cancelled) + completion = nil } private func endBackgroundTask() { - guard taskId != nil else { return } - - // Make a local copy of completion since this method is called by `dealloc` - var completion: ((SessionBackgroundTaskState) -> ())? - - SessionBackgroundTask.synced(self) { [weak self, dependencies] in - dependencies[singleton: .backgroundTaskManager].removeTask(taskId: self?.taskId) - completion = self?.completion - self?.taskId = nil - self?.completion = nil - } - - // endBackgroundTask must be called on the main thread. - if completion != nil { - Threading.dispatchMainThreadSafe { - completion?(.cancelled) - } - } + cancel() + } + + private func taskExpired() { + completion?(.expired) + completion = nil } }