Fix message processing edge cases.

// FREEBIE

Signal/src/ViewControllers/DebugUI/DebugUIMisc.m

@@ -61,6 +61,13 @@ NS_ASSUME_NONNULL_BEGIN
                                      actionBlock:^{
                                          [DebugUIMisc clearHasDismissedOffers];
                                      }]];
+    if ([thread isKindOfClass:[TSGroupThread class]]) {
+        TSGroupThread *groupThread = (TSGroupThread *)thread;
+        [items addObject:[OWSTableItem itemWithTitle:@"Hallucinate twin group"
+                                         actionBlock:^{
+                                             [DebugUIMisc hallucinateTwinGroup:groupThread];
+                                         }]];
+    }
     return [OWSTableSection sectionWithTitle:self.name items:items];
 }
 
@@ -115,6 +122,28 @@ NS_ASSUME_NONNULL_BEGIN
         }];
 }
 
+// Creates a new group (by cloning the current group) without informing the,
+// other members. This can be used to test "group info requests", etc.
++ (void)hallucinateTwinGroup:(TSGroupThread *)groupThread
+{
+    __block TSGroupThread *thread;
+    [[TSStorageManager sharedManager].dbReadWriteConnection
+        readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
+            TSGroupModel *groupModel =
+                [[TSGroupModel alloc] initWithTitle:[groupThread.groupModel.groupName stringByAppendingString:@" Copy"]
+                                          memberIds:[groupThread.groupModel.groupMemberIds mutableCopy]
+                                              image:groupThread.groupModel.groupImage
+                                            groupId:[SecurityUtils generateRandomBytes:16]];
+            thread = [TSGroupThread getOrCreateThreadWithGroupModel:groupModel transaction:transaction];
+        }];
+    OWSAssert(thread);
+
+    dispatch_async(dispatch_get_main_queue(), ^{
+        [Environment presentConversationForThread:thread];
+
+    });
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

Signal/src/environment/NotificationsManager.m

@@ -206,6 +206,7 @@ NSString *const kNotificationsManagerNewMesssageSoundName = @"NewMessage.aifc";
 #pragma mark - Signal Messages
 
 - (void)notifyUserForErrorMessage:(TSErrorMessage *)message inThread:(TSThread *)thread {
+    OWSAssert([NSThread isMainThread]);
     OWSAssert(message);
     OWSAssert(thread);
 

SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m

@@ -102,7 +102,7 @@ NS_ASSUME_NONNULL_BEGIN
 
     // If there is an attachment + text, render the text here, as Signal-iOS renders two messages.
     if (attachmentsProcessor.hasSupportedAttachments && transcript.body && ![transcript.body isEqualToString:@""]) {
-        // render text *after* the attachment
+        // We want the text to appear after the attachment.
         uint64_t textMessageTimestamp = transcript.timestamp + 1;
         TSOutgoingMessage *textMessage = [[TSOutgoingMessage alloc] initWithTimestamp:textMessageTimestamp
                                                                              inThread:thread

SignalServiceKit/src/Messages/Interactions/TSInteraction.m

@@ -14,6 +14,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 + (instancetype)interactionForTimestamp:(uint64_t)timestamp
                         withTransaction:(YapDatabaseReadWriteTransaction *)transaction {
+    OWSAssert(timestamp > 0);
+
     __block int counter = 0;
     __block TSInteraction *interaction;
 

SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m

@@ -30,7 +30,8 @@ NSString *TSInvalidRecipientKey = @"TSInvalidRecipientKey";
                            forRecipient:(NSString *)recipientId
                            preKeyBundle:(PreKeyBundle *)preKeyBundle
 {
-    self = [super initWithTimestamp:message.timestamp
+    // We want the error message to appear after the message.
+    self = [super initWithTimestamp:message.timestamp + 1
                            inThread:thread
                   failedMessageType:TSErrorMessageWrongTrustedIdentityKey
                         recipientId:recipientId];

SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m

@@ -133,8 +133,9 @@ NSString *const OWSMessageContentJobFinderExtensionGroup = @"OWSBatchMessageProc
 - (void)addJobWithEnvelopeData:(NSData *)envelopeData plaintextData:(NSData *_Nullable)plaintextData
 {
     [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
-        [[[OWSMessageContentJob alloc] initWithEnvelopeData:envelopeData plaintextData:plaintextData]
+        OWSMessageContentJob *job =
-            saveWithTransaction:transaction];
+            [[OWSMessageContentJob alloc] initWithEnvelopeData:envelopeData plaintextData:plaintextData];
+        [job saveWithTransaction:transaction];
     }];
 }
 
@@ -205,6 +206,18 @@ NSString *const OWSMessageContentJobFinderExtensionGroup = @"OWSBatchMessageProc
     [database registerExtension:[self databaseExtension] withName:OWSMessageContentJobFinderExtensionName];
 }
 
+#pragma mark Logging
+
++ (NSString *)tag
+{
+    return [NSString stringWithFormat:@"[%@]", self.class];
+}
+
+- (NSString *)tag
+{
+    return self.class.tag;
+}
+
 @end
 
 #pragma mark - Queue Processing

SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m

@@ -266,12 +266,14 @@ NS_ASSUME_NONNULL_BEGIN
         TSIncomingMessage *incomingMessage = (TSIncomingMessage *)message;
         NSString *contactName = [contactsManager displayNameForPhoneIdentifier:incomingMessage.messageAuthorId];
 
-        [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp
+        // We want the info message to appear after the message.
+        [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp + 1
                                                                            thread:message.thread
                                                                     configuration:disappearingMessagesConfiguration
                                                               createdByRemoteName:contactName] save];
     } else {
-        [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp
+        // We want the info message to appear after the message.
+        [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp + 1
                                                                            thread:message.thread
                                                                     configuration:disappearingMessagesConfiguration]
             save];

SignalServiceKit/src/Messages/OWSIdentityManager.h

@@ -46,6 +46,7 @@ extern const NSUInteger kIdentityKeyLength;
  */
 - (nullable OWSRecipientIdentity *)untrustedIdentityForSendingToRecipientId:(NSString *)recipientId;
 
+// This method can be called from any thread.
 - (void)processIncomingSyncMessage:(OWSSignalServiceProtosVerified *)verified;
 
 @end

SignalServiceKit/src/Messages/OWSIdentityManager.m

@@ -561,7 +561,6 @@ NSString *const kNSNotificationName_IdentityStateDidChange = @"kNSNotificationNa
 
 - (void)processIncomingSyncMessage:(OWSSignalServiceProtosVerified *)verified
 {
-
     NSString *recipientId = verified.destination;
     if (recipientId.length < 1) {
         OWSFail(@"Verification state sync message missing recipientId.");

SignalServiceKit/src/Messages/OWSMessageDecrypter.m

@@ -247,7 +247,7 @@ NS_ASSUME_NONNULL_BEGIN
             NSData *plaintextData = [[cipher decrypt:cipherMessage] removePadding];
             successBlock(plaintextData);
         } @catch (NSException *exception) {
-            dispatch_async(dispatch_get_main_queue(), ^{
+            dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
                 [self processException:exception envelope:envelope];
                 NSString *errorDescription = [NSString
                     stringWithFormat:@"Exception while decrypting %@: %@", cipherTypeName, exception.description];
@@ -260,8 +260,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)processException:(NSException *)exception envelope:(OWSSignalServiceProtosEnvelope *)envelope
 {
-    OWSAssert([NSThread isMainThread]);
-
     DDLogError(@"%@ Got exception: %@ of type: %@ with reason: %@",
         self.tag,
         exception.description,
@@ -295,6 +293,7 @@ NS_ASSUME_NONNULL_BEGIN
             errorMessage = [TSErrorMessage corruptedMessageWithEnvelope:envelope withTransaction:transaction];
         }
 
+        OWSAssert(errorMessage);
         [errorMessage saveWithTransaction:transaction];
     }];
 
@@ -306,7 +305,10 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)notifyForErrorMessage:(TSErrorMessage *)errorMessage withEnvelope:(OWSSignalServiceProtosEnvelope *)envelope
 {
     TSThread *contactThread = [TSContactThread getOrCreateThreadWithContactId:envelope.source];
-    [[TextSecureKitEnv sharedEnv].notificationsManager notifyUserForErrorMessage:errorMessage inThread:contactThread];
+    dispatch_async(dispatch_get_main_queue(), ^{
+        [[TextSecureKitEnv sharedEnv].notificationsManager notifyUserForErrorMessage:errorMessage
+                                                                            inThread:contactThread];
+    });
 }
 
 #pragma mark - Logging

SignalServiceKit/src/Messages/OWSMessageManager.m

@@ -191,7 +191,9 @@ NS_ASSUME_NONNULL_BEGIN
         TSOutgoingMessage *outgoingMessage = (TSOutgoingMessage *)interaction;
         [outgoingMessage updateWithWasDeliveredWithTransaction:transaction];
     } else {
-        OWSFail(@"%@ Unexpected message with timestamp: %llu", self.tag, envelope.timestamp);
+        // Desktop currently sends delivery receipts for "unpersisted" messages
+        // like group updates, so these errors are expected to a certain extent.
+        DDLogError(@"%@ Unexpected message with timestamp: %llu", self.tag, envelope.timestamp);
     }
 }
 
@@ -292,7 +294,6 @@ NS_ASSUME_NONNULL_BEGIN
             OWSSyncGroupsRequestMessage *syncGroupsRequestMessage =
                 [[OWSSyncGroupsRequestMessage alloc] initWithThread:thread groupId:groupId];
             [self.messageSender sendMessage:syncGroupsRequestMessage
-                transaction:transaction
                 success:^{
                     DDLogWarn(@"%@ Successfully sent Request Group Info message.", self.tag);
                 }
@@ -645,7 +646,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)sendGroupUpdateForThread:(TSGroupThread *)gThread message:(TSOutgoingMessage *)message
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(gThread);
     OWSAssert(gThread.groupModel);
     OWSAssert(message);
@@ -715,9 +715,7 @@ NS_ASSUME_NONNULL_BEGIN
     // Only send this group update to the requester.
     [message updateWithSingleGroupRecipient:envelope.source transaction:transaction];
 
-    dispatch_async(dispatch_get_main_queue(), ^{
+    [self sendGroupUpdateForThread:gThread message:message];
-        [self sendGroupUpdateForThread:gThread message:message];
-    });
 }
 
 - (TSIncomingMessage *_Nullable)handleReceivedEnvelope:(OWSSignalServiceProtosEnvelope *)envelope
@@ -855,7 +853,7 @@ NS_ASSUME_NONNULL_BEGIN
         // Other clients allow attachments to be sent along with body, we want the text displayed as a separate
         // message
         if ([attachmentIds count] > 0 && body != nil && body.length > 0) {
-            // We want the text to be displayed under the attachment
+            // We want the text to be displayed under the attachment.
             uint64_t textMessageTimestamp = timestamp + 1;
             TSIncomingMessage *textMessage = [[TSIncomingMessage alloc] initWithTimestamp:textMessageTimestamp
                                                                                  inThread:thread

SignalServiceKit/src/Messages/OWSMessageSender.h

@@ -69,10 +69,6 @@ NS_SWIFT_NAME(MessageSender)
 - (void)sendMessage:(TSOutgoingMessage *)message
             success:(void (^)())successHandler
             failure:(void (^)(NSError *error))failureHandler;
-- (void)sendMessage:(TSOutgoingMessage *)message
-        transaction:(YapDatabaseReadWriteTransaction *_Nullable)transaction
-            success:(void (^)())successHandler
-            failure:(void (^)(NSError *error))failureHandler;
 
 /**
  * Takes care of allocating and uploading the attachment, then sends the message.

SignalServiceKit/src/Messages/OWSMessageSender.m

@@ -32,6 +32,7 @@
 #import "TSStorageManager+sessionStore.h"
 #import "TSStorageManager.h"
 #import "TSThread.h"
+#import "Threading.h"
 #import <AxolotlKit/AxolotlExceptions.h>
 #import <AxolotlKit/CipherMessage.h>
 #import <AxolotlKit/PreKeyBundle.h>
@@ -424,33 +425,36 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
 - (void)sendMessage:(TSOutgoingMessage *)message
             success:(void (^)())successHandler
             failure:(void (^)(NSError *error))failureHandler
-{
-    [self sendMessage:message transaction:nil success:successHandler failure:failureHandler];
-}
-
-- (void)sendMessage:(TSOutgoingMessage *)message
-        transaction:(YapDatabaseReadWriteTransaction *_Nullable)transaction
-            success:(void (^)())successHandler
-            failure:(void (^)(NSError *error))failureHandler
 {
     OWSAssert(message);
 
-    if (transaction) {
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-        [message updateWithMessageState:TSOutgoingMessageStateAttemptingOut transaction:transaction];
+        // This method will use a read/write transaction. This transaction
-    } else {
+        // will block until any open read/write transactions are complete.
+        //
+        // That's key - we don't want to send any messages in response
+        // to an incoming message until processing of that batch of messages
+        // is complete.
         [message updateWithMessageState:TSOutgoingMessageStateAttemptingOut];
-    }
-    OWSSendMessageOperation *sendMessageOperation = [[OWSSendMessageOperation alloc] initWithMessage:message
-                                                                                       messageSender:self
-                                                                                             success:successHandler
-                                                                                             failure:failureHandler];
 
-    // We call `startBackgroundTask` here to prevent our app from suspending while being backgrounded
+        OWSSendMessageOperation *sendMessageOperation =
-    // until the operation is completed - at which point the OWSSendMessageOperation ends it's background task.
+            [[OWSSendMessageOperation alloc] initWithMessage:message
-    [sendMessageOperation startBackgroundTask];
+                                               messageSender:self
+                                                     success:successHandler
+                                                     failure:failureHandler];
 
-    NSOperationQueue *sendingQueue = [self sendingQueueForMessage:message];
+        // startBackgroundTask must be called on the main thread.
-    [sendingQueue addOperation:sendMessageOperation];
+        dispatch_async(dispatch_get_main_queue(), ^{
+            // We call `startBackgroundTask` here to prevent our app from suspending while being backgrounded
+            // until the operation is completed - at which point the OWSSendMessageOperation ends it's background task.
+            [sendMessageOperation startBackgroundTask];
+
+            dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+                NSOperationQueue *sendingQueue = [self sendingQueueForMessage:message];
+                [sendingQueue addOperation:sendMessageOperation];
+            });
+        });
+    });
 }
 
 - (void)attemptToSendMessage:(TSOutgoingMessage *)message
@@ -556,9 +560,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
         }
         [message save];
 
-        dispatch_async(dispatch_get_main_queue(), ^{
+        [self sendMessage:message success:successHandler failure:failureHandler];
-            [self sendMessage:message success:successHandler failure:failureHandler];
-        });
     });
 }
 
@@ -1127,6 +1129,8 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
         TSContactThread *cThread =
             [TSContactThread getOrCreateThreadWithContactId:contactId transaction:transaction];
         [cThread saveWithTransaction:transaction];
+
+        // We want the incoming message to appear after the outgoing message.
         TSIncomingMessage *incomingMessage =
             [[TSIncomingMessage alloc] initWithTimestamp:(outgoingMessage.timestamp + 1)
                                                 inThread:cThread
@@ -1324,12 +1328,14 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
         // TODO: Why is this necessary?
         [message save];
     } else if (message.groupMetaMessage == TSGroupMessageQuit) {
-        [[[TSInfoMessage alloc] initWithTimestamp:message.timestamp
+        // We want the info message to appear after the message.
+        [[[TSInfoMessage alloc] initWithTimestamp:message.timestamp + 1
                                          inThread:thread
                                       messageType:TSInfoMessageTypeGroupQuit
                                     customMessage:message.customMessage] save];
     } else {
-        [[[TSInfoMessage alloc] initWithTimestamp:message.timestamp
+        // We want the info message to appear after the message.
+        [[[TSInfoMessage alloc] initWithTimestamp:message.timestamp + 1
                                          inThread:thread
                                       messageType:TSInfoMessageTypeGroupUpdate
                                     customMessage:message.customMessage] save];