From a22dc15249b593755c60f5e296d8ec58b573d72d Mon Sep 17 00:00:00 2001 From: Ryan Zhao Date: Mon, 4 Apr 2022 13:31:56 +1000 Subject: [PATCH 1/3] fix closed group poller unwrapping crash in background --- Session/Utilities/BackgroundPoller.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Session/Utilities/BackgroundPoller.swift b/Session/Utilities/BackgroundPoller.swift index ff31d0965..5d84d359c 100644 --- a/Session/Utilities/BackgroundPoller.swift +++ b/Session/Utilities/BackgroundPoller.swift @@ -3,7 +3,6 @@ import SessionSnodeKit @objc(LKBackgroundPoller) public final class BackgroundPoller : NSObject { - private static var closedGroupPoller: ClosedGroupPoller! private static var promises: [Promise] = [] private override init() { } From 43ca54c0a0b90020ea0632c6181b704de9a69c3c Mon Sep 17 00:00:00 2001 From: Ryan Zhao Date: Mon, 4 Apr 2022 14:40:24 +1000 Subject: [PATCH 2/3] refactor to use Atomic wrapper --- SessionMessagingKit/Jobs/AttachmentDownloadJob.swift | 2 +- SessionMessagingKit/Jobs/AttachmentUploadJob.swift | 2 +- SessionMessagingKit/Jobs/JobQueue.swift | 8 +++----- SessionMessagingKit/Jobs/MessageReceiveJob.swift | 2 +- SessionMessagingKit/Jobs/MessageSendJob.swift | 2 +- SessionMessagingKit/Jobs/NotifyPNServerJob.swift | 2 +- SessionMessagingKit/Open Groups/OpenGroupAPIV2.swift | 10 +++++----- .../Pollers/ClosedGroupPoller.swift | 9 ++++----- 8 files changed, 17 insertions(+), 20 deletions(-) diff --git a/SessionMessagingKit/Jobs/AttachmentDownloadJob.swift b/SessionMessagingKit/Jobs/AttachmentDownloadJob.swift index 164f422a9..f85c66e35 100644 --- a/SessionMessagingKit/Jobs/AttachmentDownloadJob.swift +++ b/SessionMessagingKit/Jobs/AttachmentDownloadJob.swift @@ -61,7 +61,7 @@ public final class AttachmentDownloadJob : NSObject, Job, NSCoding { // NSObject // MARK: Running public func execute() { if let id = id { - JobQueue.currentlyExecutingJobs.insert(id) + JobQueue.currentlyExecutingJobs.mutate{ $0.insert(id) } } guard !isDeferred else { return } if TSAttachment.fetch(uniqueId: attachmentID) is TSAttachmentStream { diff --git a/SessionMessagingKit/Jobs/AttachmentUploadJob.swift b/SessionMessagingKit/Jobs/AttachmentUploadJob.swift index 401824b69..5598eb9be 100644 --- a/SessionMessagingKit/Jobs/AttachmentUploadJob.swift +++ b/SessionMessagingKit/Jobs/AttachmentUploadJob.swift @@ -61,7 +61,7 @@ public final class AttachmentUploadJob : NSObject, Job, NSCoding { // NSObject/N // MARK: Running public func execute() { if let id = id { - JobQueue.currentlyExecutingJobs.insert(id) + JobQueue.currentlyExecutingJobs.mutate{ $0.insert(id) } } guard let stream = TSAttachment.fetch(uniqueId: attachmentID) as? TSAttachmentStream else { return handleFailure(error: Error.noAttachment) diff --git a/SessionMessagingKit/Jobs/JobQueue.swift b/SessionMessagingKit/Jobs/JobQueue.swift index 159641522..df8ebe02b 100644 --- a/SessionMessagingKit/Jobs/JobQueue.swift +++ b/SessionMessagingKit/Jobs/JobQueue.swift @@ -5,9 +5,7 @@ public final class JobQueue : NSObject, JobDelegate { private static var jobIDs: [UInt64:UInt64] = [:] - internal static var currentlyExecutingJobs: Set = [] - - private let internalQueue: DispatchQueue = DispatchQueue(label:"executingJobQueue") + internal static var currentlyExecutingJobs: Atomic> = Atomic([]) @objc public static let shared = JobQueue() @@ -38,7 +36,7 @@ public final class JobQueue : NSObject, JobDelegate { allJobTypes.forEach { type in let allPendingJobs = SNMessagingKitConfiguration.shared.storage.getAllPendingJobs(of: type) allPendingJobs.sorted(by: { $0.id! < $1.id! }).forEach { job in // Retry the oldest jobs first - guard !JobQueue.currentlyExecutingJobs.contains(job.id!) else { + guard !JobQueue.currentlyExecutingJobs.wrappedValue.contains(job.id!) else { return SNLog("Not resuming already executing job.") } SNLog("Resuming pending job of type: \(type).") @@ -95,7 +93,7 @@ public final class JobQueue : NSObject, JobDelegate { } private func removeExecutingJob(_ jobID: String) { - let _ = internalQueue.sync { JobQueue.currentlyExecutingJobs.remove(jobID) } + JobQueue.currentlyExecutingJobs.mutate { $0.remove(jobID) } } private func getRetryInterval(for job: Job) -> TimeInterval { diff --git a/SessionMessagingKit/Jobs/MessageReceiveJob.swift b/SessionMessagingKit/Jobs/MessageReceiveJob.swift index c9c690c2b..16827b0de 100644 --- a/SessionMessagingKit/Jobs/MessageReceiveJob.swift +++ b/SessionMessagingKit/Jobs/MessageReceiveJob.swift @@ -59,7 +59,7 @@ public final class MessageReceiveJob : NSObject, Job, NSCoding { // NSObject/NSC public func execute() -> Promise { if let id = id { // Can be nil (e.g. when background polling) - JobQueue.currentlyExecutingJobs.insert(id) + JobQueue.currentlyExecutingJobs.mutate { $0.insert(id) } } let (promise, seal) = Promise.pending() SNMessagingKitConfiguration.shared.storage.write(with: { transaction in // Intentionally capture self diff --git a/SessionMessagingKit/Jobs/MessageSendJob.swift b/SessionMessagingKit/Jobs/MessageSendJob.swift index e1cbad17d..1a302c174 100644 --- a/SessionMessagingKit/Jobs/MessageSendJob.swift +++ b/SessionMessagingKit/Jobs/MessageSendJob.swift @@ -71,7 +71,7 @@ public final class MessageSendJob : NSObject, Job, NSCoding { // NSObject/NSCodi // MARK: Running public func execute() { if let id = id { - JobQueue.currentlyExecutingJobs.insert(id) + JobQueue.currentlyExecutingJobs.mutate{ $0.insert(id) } } let storage = SNMessagingKitConfiguration.shared.storage if let message = message as? VisibleMessage { diff --git a/SessionMessagingKit/Jobs/NotifyPNServerJob.swift b/SessionMessagingKit/Jobs/NotifyPNServerJob.swift index 82c57b3f3..b6c9c1579 100644 --- a/SessionMessagingKit/Jobs/NotifyPNServerJob.swift +++ b/SessionMessagingKit/Jobs/NotifyPNServerJob.swift @@ -39,7 +39,7 @@ public final class NotifyPNServerJob : NSObject, Job, NSCoding { // NSObject/NSC public func execute() -> Promise { if let id = id { - JobQueue.currentlyExecutingJobs.insert(id) + JobQueue.currentlyExecutingJobs.mutate{ $0.insert(id) } } let server = PushNotificationAPI.server let parameters = [ "data" : message.data.description, "send_to" : message.recipient ] diff --git a/SessionMessagingKit/Open Groups/OpenGroupAPIV2.swift b/SessionMessagingKit/Open Groups/OpenGroupAPIV2.swift index d8206b604..97532799d 100644 --- a/SessionMessagingKit/Open Groups/OpenGroupAPIV2.swift +++ b/SessionMessagingKit/Open Groups/OpenGroupAPIV2.swift @@ -3,7 +3,7 @@ import SessionSnodeKit @objc(SNOpenGroupAPIV2) public final class OpenGroupAPIV2 : NSObject { - private static var authTokenPromises: [String:Promise] = [:] + private static var authTokenPromises: Atomic<[String:Promise]> = Atomic([:]) private static var hasPerformedInitialPoll: [String:Bool] = [:] private static var hasUpdatedLastOpenDate = false public static let workQueue = DispatchQueue(label: "OpenGroupAPIV2.workQueue", qos: .userInitiated) // It's important that this is a serial queue @@ -210,7 +210,7 @@ public final class OpenGroupAPIV2 : NSObject { if let authToken = storage.getAuthToken(for: room, on: server) { return Promise.value(authToken) } else { - if let authTokenPromise = authTokenPromises["\(server).\(room)"] { + if let authTokenPromise = authTokenPromises.wrappedValue["\(server).\(room)"] { return authTokenPromise } else { let promise = requestNewAuthToken(for: room, on: server) @@ -225,11 +225,11 @@ public final class OpenGroupAPIV2 : NSObject { return promise } promise.done(on: OpenGroupAPIV2.workQueue) { _ in - authTokenPromises["\(server).\(room)"] = nil + authTokenPromises.mutate{ $0["\(server).\(room)"] = nil } }.catch(on: OpenGroupAPIV2.workQueue) { _ in - authTokenPromises["\(server).\(room)"] = nil + authTokenPromises.mutate{ $0["\(server).\(room)"] = nil } } - authTokenPromises["\(server).\(room)"] = promise + authTokenPromises.mutate{ $0["\(server).\(room)"] = promise } return promise } } diff --git a/SessionMessagingKit/Sending & Receiving/Pollers/ClosedGroupPoller.swift b/SessionMessagingKit/Sending & Receiving/Pollers/ClosedGroupPoller.swift index 1644488e6..481a9b336 100644 --- a/SessionMessagingKit/Sending & Receiving/Pollers/ClosedGroupPoller.swift +++ b/SessionMessagingKit/Sending & Receiving/Pollers/ClosedGroupPoller.swift @@ -3,9 +3,8 @@ import PromiseKit @objc(LKClosedGroupPoller) public final class ClosedGroupPoller : NSObject { - private var isPolling: [String:Bool] = [:] + private var isPolling: Atomic<[String:Bool]> = Atomic([:]) private var timers: [String:Timer] = [:] - private let internalQueue: DispatchQueue = DispatchQueue(label:"isPollingQueue") // MARK: Settings private static let minPollInterval: Double = 2 @@ -44,7 +43,7 @@ public final class ClosedGroupPoller : NSObject { // Might be a race condition that the setUpPolling finishes too soon, // and the timer is not created, if we mark the group as is polling // after setUpPolling. So the poller may not work, thus misses messages. - internalQueue.sync{ isPolling[groupPublicKey] = true } + isPolling.mutate{ $0[groupPublicKey] = true } setUpPolling(for: groupPublicKey) } @@ -55,7 +54,7 @@ public final class ClosedGroupPoller : NSObject { } public func stopPolling(for groupPublicKey: String) { - internalQueue.sync{ isPolling[groupPublicKey] = false } + isPolling.mutate{ $0[groupPublicKey] = false } timers[groupPublicKey]?.invalidate() } @@ -139,6 +138,6 @@ public final class ClosedGroupPoller : NSObject { // MARK: Convenience private func isPolling(for groupPublicKey: String) -> Bool { - return internalQueue.sync{ isPolling[groupPublicKey] ?? false } + return isPolling.wrappedValue[groupPublicKey] ?? false } } From 77c00b6c37d0490e54ca60606cf4c5effdfe9a68 Mon Sep 17 00:00:00 2001 From: Ryan Zhao Date: Mon, 4 Apr 2022 16:10:28 +1000 Subject: [PATCH 3/3] fix #456 --- .../Media Viewing & Editing/ImagePickerController.swift | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/Session/Media Viewing & Editing/ImagePickerController.swift b/Session/Media Viewing & Editing/ImagePickerController.swift index 36f514d2a..3500fdc99 100644 --- a/Session/Media Viewing & Editing/ImagePickerController.swift +++ b/Session/Media Viewing & Editing/ImagePickerController.swift @@ -379,7 +379,7 @@ class ImagePickerGridController: UICollectionViewController, PhotoLibraryDelegat } collectionView.allowsMultipleSelection = delegate.isInBatchSelectMode - collectionView.reloadData() + reloadDataAndRestoreSelection() } func clearCollectionViewSelection() { @@ -551,12 +551,7 @@ class ImagePickerGridController: UICollectionViewController, PhotoLibraryDelegat let assetItem = photoCollectionContents.assetItem(at: indexPath.item, photoMediaSize: photoMediaSize) cell.configure(item: assetItem) - let isSelected = delegate.imagePicker(self, isAssetSelected: assetItem.asset) - if isSelected { - cell.isSelected = isSelected - } else { - cell.isSelected = isSelected - } + cell.isSelected = delegate.imagePicker(self, isAssetSelected: assetItem.asset) return cell }