From dd86966111a68711f3927f21f8eb4e29aaf95d1a Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Thu, 30 Apr 2020 08:51:19 +1000 Subject: [PATCH 1/4] Clean up docs & fix crash --- .../ConversationView/ConversationViewController.m | 8 +++++--- SignalServiceKit/src/Loki/API/LokiAPI.swift | 2 +- SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift | 1 + .../src/Loki/API/LokiFileServerAPI.swift | 3 +++ .../src/Loki/Protocol/Mentions/MentionsManager.swift | 2 +- .../Protocol/Message Types/LKSessionRestoreMessage.h | 2 +- .../Protocol/Message Types/LKUnlinkDeviceMessage.h | 2 +- .../Protocol/Multi Device/MultiDeviceProtocol.swift | 6 +++--- .../SessionManagementProtocol.swift | 9 ++++----- .../src/Loki/Protocol/SessionProtocol.swift | 12 +++++++----- .../Sync Messages/SyncMessagesProtocol.swift | 2 +- 11 files changed, 28 insertions(+), 21 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 2cd43c5b5..3f194b83e 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -1237,9 +1237,11 @@ typedef enum : NSUInteger { } - (void)restoreSession { - [OWSPrimaryStorage.sharedManager.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [LKSessionManagementProtocol sending_startSessionResetInThread:self.thread using:transaction]; - }]; + dispatch_async(dispatch_get_main_queue(), ^{ + [OWSPrimaryStorage.sharedManager.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [LKSessionManagementProtocol sending_startSessionResetInThread:self.thread using:transaction]; + }]; + }); } - (void)noLongerVerifiedBannerViewWasTapped:(UIGestureRecognizer *)sender diff --git a/SignalServiceKit/src/Loki/API/LokiAPI.swift b/SignalServiceKit/src/Loki/API/LokiAPI.swift index ccd3f277a..5a06afbe0 100644 --- a/SignalServiceKit/src/Loki/API/LokiAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiAPI.swift @@ -2,7 +2,7 @@ import PromiseKit // TODO: A lot of the API relies on things happening serially and state being maintained correctly (i.e. without // race conditions). To this end we should just have one high quality serial queue and do everything on there, except -// for things that explicitly *can* be done in parallel and don't modify state, any which should then happen +// for things that explicitly *can* be done in parallel and don't modify state, which should then happen // on a global queue. @objc(LKAPI) diff --git a/SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift b/SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift index 6d766357b..e0dc87178 100644 --- a/SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift @@ -40,6 +40,7 @@ public class LokiDotNetAPI : NSObject { } else { return requestNewAuthToken(for: server).then(on: LokiAPI.workQueue) { submitAuthToken($0, for: server) }.then(on: LokiAPI.workQueue) { token -> Promise in let (promise, seal) = Promise.pending() + // Dispatch async on the main queue to avoid nested write transactions DispatchQueue.main.async { storage.dbReadWriteConnection.readWrite { transaction in setAuthToken(for: server, to: token, in: transaction) diff --git a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift index 0f4409c14..617392700 100644 --- a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift @@ -86,6 +86,7 @@ public final class LokiFileServerAPI : LokiDotNetAPI { }) }.then(on: LokiAPI.workQueue) { deviceLinks -> Promise> in let (promise, seal) = Promise>.pending() + // Dispatch async on the main queue to avoid nested write transactions DispatchQueue.main.async { storage.dbReadWriteConnection.readWrite { transaction in storage.setDeviceLinks(deviceLinks, in: transaction) @@ -126,6 +127,7 @@ public final class LokiFileServerAPI : LokiDotNetAPI { deviceLinks.insert(deviceLink) return setDeviceLinks(deviceLinks).then(on: LokiAPI.workQueue) { _ -> Promise in let (promise, seal) = Promise.pending() + // Dispatch async on the main queue to avoid nested write transactions DispatchQueue.main.async { storage.dbReadWriteConnection.readWrite { transaction in storage.addDeviceLink(deviceLink, in: transaction) @@ -145,6 +147,7 @@ public final class LokiFileServerAPI : LokiDotNetAPI { deviceLinks.remove(deviceLink) return setDeviceLinks(deviceLinks).then(on: LokiAPI.workQueue) { _ -> Promise in let (promise, seal) = Promise.pending() + // Dispatch async on the main queue to avoid nested write transactions DispatchQueue.main.async { storage.dbReadWriteConnection.readWrite { transaction in storage.removeDeviceLink(deviceLink, in: transaction) diff --git a/SignalServiceKit/src/Loki/Protocol/Mentions/MentionsManager.swift b/SignalServiceKit/src/Loki/Protocol/Mentions/MentionsManager.swift index aabbc549c..b2d4ed278 100644 --- a/SignalServiceKit/src/Loki/Protocol/Mentions/MentionsManager.swift +++ b/SignalServiceKit/src/Loki/Protocol/Mentions/MentionsManager.swift @@ -9,7 +9,7 @@ public final class MentionsManager : NSObject { set { LokiAPI.stateQueue.sync { _userHexEncodedPublicKeyCache = newValue } } } - // TODO: I don't think this stateQueue stuff actually helps avoid race conditions + // TODO: I don't think stateQueue actually helps avoid race conditions internal static var storage: OWSPrimaryStorage { OWSPrimaryStorage.shared() } diff --git a/SignalServiceKit/src/Loki/Protocol/Message Types/LKSessionRestoreMessage.h b/SignalServiceKit/src/Loki/Protocol/Message Types/LKSessionRestoreMessage.h index 3c39cb9b1..a1a373c31 100644 --- a/SignalServiceKit/src/Loki/Protocol/Message Types/LKSessionRestoreMessage.h +++ b/SignalServiceKit/src/Loki/Protocol/Message Types/LKSessionRestoreMessage.h @@ -2,7 +2,7 @@ NS_ASSUME_NONNULL_BEGIN -/// TODO: This is just a friend request message with a flag set. Not sure if it needs to be its own type. +// TODO: This is just a friend request message with a flag set. Not sure if it needs to be its own type. NS_SWIFT_NAME(SessionRestoreMessage) @interface LKSessionRestoreMessage : LKFriendRequestMessage diff --git a/SignalServiceKit/src/Loki/Protocol/Message Types/LKUnlinkDeviceMessage.h b/SignalServiceKit/src/Loki/Protocol/Message Types/LKUnlinkDeviceMessage.h index 9365fd4f1..39d176ea3 100644 --- a/SignalServiceKit/src/Loki/Protocol/Message Types/LKUnlinkDeviceMessage.h +++ b/SignalServiceKit/src/Loki/Protocol/Message Types/LKUnlinkDeviceMessage.h @@ -2,7 +2,7 @@ NS_ASSUME_NONNULL_BEGIN -/// TODO: This is just an ephemeral message with a flag set. Not sure if it needs to be its own type. +// TODO: This is just an ephemeral message with a flag set. Not sure if it needs to be its own type. NS_SWIFT_NAME(UnlinkDeviceMessage) @interface LKUnlinkDeviceMessage : TSOutgoingMessage diff --git a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift index 45f682092..f2defa9d5 100644 --- a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift @@ -19,7 +19,7 @@ public final class MultiDeviceProtocol : NSObject { set { LokiAPI.stateQueue.sync { _lastDeviceLinkUpdate = newValue } } } - // TODO: I don't think this stateQueue stuff actually helps avoid race conditions + // TODO: I don't think stateQueue actually helps avoid race conditions internal static var storage: OWSPrimaryStorage { OWSPrimaryStorage.shared() } @@ -46,7 +46,7 @@ public final class MultiDeviceProtocol : NSObject { storage.dbReadConnection.read { transaction in recipient = SignalRecipient.getOrBuildUnsavedRecipient(forRecipientId: destination.hexEncodedPublicKey, transaction: transaction) } - // TODO: Apparently it's okay that the thread, sender certificate, etc. don't get changed? + // TODO: Why is it okay that the thread, sender certificate, etc. don't get changed? return OWSMessageSend(message: messageSend.message, thread: messageSend.thread, recipient: recipient, senderCertificate: messageSend.senderCertificate, udAccess: messageSend.udAccess, localNumber: messageSend.localNumber, success: { seal.fulfill(messageSend.message.thread as! TSContactThread) @@ -179,7 +179,7 @@ public final class MultiDeviceProtocol : NSObject { DispatchQueue.main.async { var recipientUDAccess: OWSUDAccess? if let senderCertificate = senderCertificate { - recipientUDAccess = udManager.udAccess(forRecipientId: hexEncodedPublicKey, requireSyncAccess: true) + recipientUDAccess = udManager.udAccess(forRecipientId: hexEncodedPublicKey, requireSyncAccess: true) // Starts a new write transaction internally } let messageSend = OWSMessageSend(message: message, thread: thread, recipient: recipient, senderCertificate: senderCertificate, udAccess: recipientUDAccess, localNumber: getUserHexEncodedPublicKey(), success: { diff --git a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift index 425aa5446..64231d678 100644 --- a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift @@ -80,7 +80,7 @@ public final class SessionManagementProtocol : NSObject { } // MARK: - Sending - // TODO: Confusing that we have this but also the receiving version + // TODO: Confusing that we have this but also a receiving version @objc(sending_startSessionResetInThread:using:) public static func sending_startSessionReset(in thread: TSThread, using transaction: YapDatabaseReadWriteTransaction) { guard let thread = thread as? TSContactThread else { @@ -155,14 +155,13 @@ public final class SessionManagementProtocol : NSObject { return dataMessage.flags & UInt32(sessionRestoreFlag.rawValue) != 0 } - // TODO: Is this only ever used for closed groups? @objc(isSessionRequestMessage:) public static func isSessionRequestMessage(_ dataMessage: SSKProtoDataMessage) -> Bool { let sessionRequestFlag = SSKProtoDataMessage.SSKProtoDataMessageFlags.sessionRequest return dataMessage.flags & UInt32(sessionRequestFlag.rawValue) != 0 } - // TODO: This seriously needs some explanation of when we expect pre key bundles to be attached + // TODO: This needs an explanation of when we expect pre key bundles to be attached @objc(handlePreKeyBundleMessageIfNeeded:wrappedIn:using:) public static func handlePreKeyBundleMessageIfNeeded(_ protoContent: SSKProtoContent, wrappedIn envelope: SSKProtoEnvelope, using transaction: YapDatabaseReadWriteTransaction) { // The envelope source is set during UD decryption @@ -177,7 +176,7 @@ public final class SessionManagementProtocol : NSObject { // If we received a friend request (i.e. also a new pre key bundle), but we were already friends with the other user, reset the session. // The envelope type is set during UD decryption. if envelope.type == .friendRequest, - let thread = TSContactThread.getWithContactId(hexEncodedPublicKey, transaction: transaction), // TODO: Maybe this should be getOrCreate? + let thread = TSContactThread.getWithContactId(hexEncodedPublicKey, transaction: transaction), // TODO: Should this be getOrCreate? thread.isContactFriend { receiving_startSessionReset(in: thread, using: transaction) // Notify our other devices that we've started a session reset @@ -186,7 +185,7 @@ public final class SessionManagementProtocol : NSObject { } } - // TODO: Confusing that we have this but also the sending version + // TODO: Confusing that we have this but also a sending version @objc(receiving_startSessionResetInThread:using:) public static func receiving_startSessionReset(in thread: TSContactThread, using transaction: YapDatabaseReadWriteTransaction) { let hexEncodedPublicKey = thread.contactIdentifier() diff --git a/SignalServiceKit/src/Loki/Protocol/SessionProtocol.swift b/SignalServiceKit/src/Loki/Protocol/SessionProtocol.swift index 8b391e99c..9f8e21836 100644 --- a/SignalServiceKit/src/Loki/Protocol/SessionProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/SessionProtocol.swift @@ -48,7 +48,7 @@ public final class SessionProtocol : NSObject { if let openGroup = LokiDatabaseUtilities.getPublicChat(for: thread.uniqueId!, in: transaction) { result = [ openGroup.server ] // Aim the message at the open group server } else { - // TODO: Handle + // TODO: Handle? } } } else { @@ -111,7 +111,8 @@ public final class SessionProtocol : NSObject { return !isMessageNoteToSelf(thread) && !thread.isGroupThread() } - // TODO: Not sure how these two relate. EDIT: I think the one below is used to block delivery receipts. That means that + // TODO: Not sure how these two relate + // EDIT: I think the one below is used to block delivery receipts. That means that // right now we do send delivery receipts in note to self, but not read receipts. Other than that their behavior should // be identical. Should we just not send any kind of receipt in note to self? @@ -139,8 +140,9 @@ public final class SessionProtocol : NSObject { // MARK: - Decryption @objc(shouldSkipMessageDecryptResult:) public static func shouldSkipMessageDecryptResult(_ result: OWSMessageDecryptResult) -> Bool { - // Called from OWSMessageReceiver to prevent messages from even being added to the processing queue for some reason - return result.source == getUserHexEncodedPublicKey() // NOTE: This doesn't take into account multi device + // Called from OWSMessageReceiver to prevent messages from even being added to the processing queue + // TODO: Why is this function needed at all? + return result.source == getUserHexEncodedPublicKey() // This intentionally doesn't take into account multi device } // MARK: Profile Updating @@ -167,7 +169,7 @@ public final class SessionProtocol : NSObject { return } let profileManager = SSKEnvironment.shared.profileManager - // This dispatches async on the main queue internally, where it starts a new write transaction. Apparently that's an okay thing to do in this case? + // This dispatches async on the main queue internally where it starts a new write transaction profileManager.setProfileKeyData(profileKey, forRecipientId: hexEncodedPublicKey, avatarURL: profilePictureURL) } diff --git a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift index 5a62f14bc..5340a6e03 100644 --- a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift @@ -44,7 +44,7 @@ public final class SyncMessagesProtocol : NSObject { guard thread.isContactFriend && thread.shouldThreadBeVisible && !thread.isForceHidden else { return } friends.append(SignalAccount(recipientId: hexEncodedPublicKey)) } - friends.append(SignalAccount(recipientId: getUserHexEncodedPublicKey())) // TODO: We sure about this? + friends.append(SignalAccount(recipientId: getUserHexEncodedPublicKey())) // TODO: Are we sure about this? let syncManager = SSKEnvironment.shared.syncManager let promises = friends.chunked(by: 3).map { friends -> Promise in // TODO: Does this always fit? return Promise(syncManager.syncContacts(for: friends)).map { _ in } From a2de00feb8d6603bfc30cbb90a4af87e22f92e78 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Thu, 30 Apr 2020 09:06:05 +1000 Subject: [PATCH 2/4] Avoid nested write transaction --- .../Multi Device/MultiDeviceProtocol.swift | 1 + .../SessionManagementProtocol.swift | 31 ++++++++++++------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift index f2defa9d5..133835a54 100644 --- a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift @@ -176,6 +176,7 @@ public final class MultiDeviceProtocol : NSObject { let udManager = SSKEnvironment.shared.udManager let senderCertificate = udManager.getSenderCertificate() let (promise, seal) = Promise.pending() + // Dispatch async on the main queue to avoid nested write transactions DispatchQueue.main.async { var recipientUDAccess: OWSUDAccess? if let senderCertificate = senderCertificate { diff --git a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift index 64231d678..4a722181d 100644 --- a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift @@ -91,8 +91,7 @@ public final class SessionManagementProtocol : NSObject { let devices = thread.sessionRestoreDevices // TODO: Rename this for device in devices { guard device.count != 0 else { continue } - let sessionResetMessageSend = getSessionResetMessageSend(for: device, in: transaction) - OWSDispatch.sendingQueue().async { + getSessionResetMessageSend(for: device, in: transaction).done(on: OWSDispatch.sendingQueue()) { sessionResetMessageSend in messageSender.sendMessage(sessionResetMessageSend) } } @@ -112,7 +111,11 @@ public final class SessionManagementProtocol : NSObject { } @objc(getSessionResetMessageSendForHexEncodedPublicKey:in:) - public static func getSessionResetMessageSend(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadWriteTransaction) -> OWSMessageSend { + public static func objc_getSessionResetMessageSend(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadWriteTransaction) -> AnyPromise { + return AnyPromise.from(getSessionResetMessageSend(for: hexEncodedPublicKey, in: transaction)) + } + + public static func getSessionResetMessageSend(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadWriteTransaction) -> Promise { let thread = TSContactThread.getOrCreateThread(withContactId: hexEncodedPublicKey, transaction: transaction) let masterHexEncodedPublicKey = storage.getMasterHexEncodedPublicKey(for: hexEncodedPublicKey, in: transaction) let isSlaveDeviceThread = masterHexEncodedPublicKey != hexEncodedPublicKey @@ -122,16 +125,22 @@ public final class SessionManagementProtocol : NSObject { let recipient = SignalRecipient.getOrBuildUnsavedRecipient(forRecipientId: hexEncodedPublicKey, transaction: transaction) let udManager = SSKEnvironment.shared.udManager let senderCertificate = udManager.getSenderCertificate() - var recipientUDAccess: OWSUDAccess? - if let senderCertificate = senderCertificate { - recipientUDAccess = udManager.udAccess(forRecipientId: hexEncodedPublicKey, requireSyncAccess: true) - } - return OWSMessageSend(message: message, thread: thread, recipient: recipient, senderCertificate: senderCertificate, - udAccess: recipientUDAccess, localNumber: getUserHexEncodedPublicKey(), success: { + let (promise, seal) = Promise.pending() + // Dispatch async on the main queue to avoid nested write transactions + DispatchQueue.main.async { + var recipientUDAccess: OWSUDAccess? + if let senderCertificate = senderCertificate { + recipientUDAccess = udManager.udAccess(forRecipientId: hexEncodedPublicKey, requireSyncAccess: true) // Starts a new write transaction internally + } + let messageSend = OWSMessageSend(message: message, thread: thread, recipient: recipient, senderCertificate: senderCertificate, + udAccess: recipientUDAccess, localNumber: getUserHexEncodedPublicKey(), success: { - }, failure: { error in + }, failure: { error in - }) + }) + seal.fulfill(messageSend) + } + return promise } // MARK: - Receiving From e374d1d0c884f8656c382a30b2dec484fe031154 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Thu, 30 Apr 2020 09:32:42 +1000 Subject: [PATCH 3/4] Fix multi device message status bar updating bug --- Signal/src/Loki/Components/ConversationTitleView.swift | 10 +++++++++- .../ConversationView/ConversationViewController.m | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Signal/src/Loki/Components/ConversationTitleView.swift b/Signal/src/Loki/Components/ConversationTitleView.swift index ad041ffab..6f63668a1 100644 --- a/Signal/src/Loki/Components/ConversationTitleView.swift +++ b/Signal/src/Loki/Components/ConversationTitleView.swift @@ -128,7 +128,15 @@ final class ConversationTitleView : UIView { guard interaction.timestamp == timestamp.uint64Value else { return } uncheckedTargetInteraction = interaction } - guard let targetInteraction = uncheckedTargetInteraction, targetInteraction.interactionType() == .outgoingMessage, status.rawValue > (currentStatus?.rawValue ?? 0) else { return } + guard let targetInteraction = uncheckedTargetInteraction, targetInteraction.interactionType() == .outgoingMessage, + status.rawValue > (currentStatus?.rawValue ?? 0), let hexEncodedPublicKey = targetInteraction.thread.contactIdentifier() else { return } + var masterHexEncodedPublicKey: String! + let storage = OWSPrimaryStorage.shared() + storage.dbReadConnection.read { transaction in + masterHexEncodedPublicKey = storage.getMasterHexEncodedPublicKey(for: hexEncodedPublicKey, in: transaction) ?? hexEncodedPublicKey + } + let isSlaveDevice = masterHexEncodedPublicKey != hexEncodedPublicKey + guard !isSlaveDevice else { return } currentStatus = status } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 3f194b83e..a798d972b 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -5416,6 +5416,14 @@ typedef enum : NSUInteger { } }]; if (targetInteraction == nil || targetInteraction.interactionType != OWSInteractionType_OutgoingMessage) { return; } + NSString *hexEncodedPublicKey = targetInteraction.thread.contactIdentifier; + if (hexEncodedPublicKey == nil) { return; } + __block NSString *masterHexEncodedPublicKey; + [OWSPrimaryStorage.sharedManager.dbReadConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + masterHexEncodedPublicKey = [LKDatabaseUtilities getMasterHexEncodedPublicKeyFor:hexEncodedPublicKey in:transaction] ?: hexEncodedPublicKey; + }]; + BOOL isSlaveDevice = ![masterHexEncodedPublicKey isEqual:hexEncodedPublicKey]; + if (isSlaveDevice) { return; } dispatch_async(dispatch_get_main_queue(), ^{ if (progress <= self.progressIndicatorView.progress) { return; } self.progressIndicatorView.alpha = 1; From a49e686ca0df33aaef2780acd732493d4276b04a Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Thu, 30 Apr 2020 10:04:14 +1000 Subject: [PATCH 4/4] Fix race condition --- Signal/src/Loki/Components/ConversationTitleView.swift | 3 +++ .../ConversationView/ConversationViewController.m | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/Signal/src/Loki/Components/ConversationTitleView.swift b/Signal/src/Loki/Components/ConversationTitleView.swift index 6f63668a1..aaad7022c 100644 --- a/Signal/src/Loki/Components/ConversationTitleView.swift +++ b/Signal/src/Loki/Components/ConversationTitleView.swift @@ -3,6 +3,7 @@ final class ConversationTitleView : UIView { private let thread: TSThread private var currentStatus: Status? { didSet { updateSubtitleForCurrentStatus() } } + private var handledMessageTimestamps: Set = [] // MARK: Types private enum Status : Int { @@ -112,6 +113,7 @@ final class ConversationTitleView : UIView { @objc private func handleMessageSentNotification(_ notification: Notification) { guard let timestamp = notification.object as? NSNumber else { return } setStatusIfNeeded(to: .messageSent, forMessageWithTimestamp: timestamp) + handledMessageTimestamps.insert(timestamp) DispatchQueue.main.asyncAfter(deadline: .now() + 1) { self.clearStatusIfNeededForMessageWithTimestamp(timestamp) } @@ -123,6 +125,7 @@ final class ConversationTitleView : UIView { } private func setStatusIfNeeded(to status: Status, forMessageWithTimestamp timestamp: NSNumber) { + guard !handledMessageTimestamps.contains(timestamp) else { return } var uncheckedTargetInteraction: TSInteraction? = nil thread.enumerateInteractions { interaction in guard interaction.timestamp == timestamp.uint64Value else { return } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index a798d972b..b1e812c87 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -220,6 +220,10 @@ typedef enum : NSUInteger { @property (nonatomic) NSMutableArray *mentions; @property (nonatomic) NSString *oldText; +// Status bar updating +/// Used to avoid duplicate status bar updates. +@property (nonatomic) NSMutableSet *handledMessageTimestamps; + @end #pragma mark - @@ -5396,6 +5400,7 @@ typedef enum : NSUInteger { { NSNumber *timestamp = (NSNumber *)notification.object; [self setProgressIfNeededTo:1.0f forMessageWithTimestamp:timestamp]; + [self.handledMessageTimestamps addObject:timestamp]; dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^(void) { [self hideProgressIndicatorViewForMessageWithTimestamp:timestamp]; }); @@ -5409,6 +5414,11 @@ typedef enum : NSUInteger { - (void)setProgressIfNeededTo:(float)progress forMessageWithTimestamp:(NSNumber *)timestamp { + if ([self.handledMessageTimestamps contains:^BOOL(NSNumber *t) { + return [t isEqual:timestamp]; + }]) { + return; + } __block TSInteraction *targetInteraction; [self.thread enumerateInteractionsUsingBlock:^(TSInteraction *interaction) { if (interaction.timestamp == timestamp.unsignedLongLongValue) {