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 }