From 24f3362df1dbb5875576b5f4754ecda0b51bc268 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 2 May 2018 19:15:46 -0400 Subject: [PATCH] ensure expirations started // FREEBIE --- .../ViewControllers/DebugUI/DebugUIMessages.m | 28 ++++++++++ .../Messages/OWSDisappearingMessagesFinder.h | 4 ++ .../Messages/OWSDisappearingMessagesFinder.m | 52 ++++++++++++++++++- .../src/Messages/OWSDisappearingMessagesJob.h | 2 + .../src/Messages/OWSDisappearingMessagesJob.m | 18 +++++++ 5 files changed, 103 insertions(+), 1 deletion(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index 68b712d29..78c07e0ed 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -27,6 +27,12 @@ NS_ASSUME_NONNULL_BEGIN +@interface TSIncomingMessage (DebugUI) + +@property (nonatomic, getter=wasRead) BOOL read; + +@end + @interface TSOutgoingMessage (PostDatingDebug) - (void)setReceivedAtTimestamp:(uint64_t)value; @@ -232,6 +238,10 @@ NS_ASSUME_NONNULL_BEGIN @"%@ Failed to send Request Group Info message with error: %@", self.logTag, error); }]; }], + [OWSTableItem itemWithTitle:@"Message with stalled timer" + actionBlock:^{ + [DebugUIMessages createDisappearingMessagesWhichFailedToStartInThread:thread]; + }], [OWSTableItem itemWithTitle:@"Inject 10 fake incoming messages" actionBlock:^{ [DebugUIMessages injectFakeIncomingMessages:10 thread:thread]; @@ -4155,6 +4165,24 @@ typedef OWSContact * (^OWSContactBlock)(void); }]; } ++ (void)createDisappearingMessagesWhichFailedToStartInThread:(TSThread *)thread +{ + uint64_t now = [NSDate ows_millisecondTimeStamp]; + TSIncomingMessage *message = [[TSIncomingMessage alloc] + initIncomingMessageWithTimestamp:now + inThread:thread + authorId:thread.recipientIdentifiers.firstObject + sourceDeviceId:0 + messageBody:[NSString stringWithFormat:@"Should disappear 60s after %tu", now] + attachmentIds:[NSMutableArray new] + expiresInSeconds:60 + quotedMessage:nil + contactShare:nil]; + // private setter to avoid starting expire machinery. + message.read = YES; + [message save]; +} + + (void)testIndicScriptsInThread:(TSThread *)thread { NSArray *strings = @[ diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesFinder.h b/SignalServiceKit/src/Messages/OWSDisappearingMessagesFinder.h index b1e375e7c..339e504c9 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesFinder.h +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesFinder.h @@ -14,10 +14,14 @@ NS_ASSUME_NONNULL_BEGIN - (void)enumerateExpiredMessagesWithBlock:(void (^_Nonnull)(TSMessage *message))block transaction:(YapDatabaseReadTransaction *)transaction; + - (void)enumerateUnstartedExpiringMessagesInThread:(TSThread *)thread block:(void (^_Nonnull)(TSMessage *message))block transaction:(YapDatabaseReadTransaction *)transaction; +- (void)enumerateMessagesWhichFailedToStartExpiringWithBlock:(void (^_Nonnull)(TSMessage *message))block + transaction:(YapDatabaseReadTransaction *)transaction; + /** * @return * uint64_t millisecond timestamp wrapped in a number. Retrieve with `unsignedLongLongvalue`. diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesFinder.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesFinder.m index ca44ae977..55eda4110 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesFinder.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesFinder.m @@ -41,6 +41,34 @@ static NSString *const OWSDisappearingMessageFinderExpiresAtIndex = @"index_mess return [messageIds copy]; } +- (NSArray *)fetchMessageIdsWhichFailedToStartExpiring:(YapDatabaseReadTransaction *_Nonnull)transaction +{ + OWSAssert(transaction); + + NSMutableArray *messageIds = [NSMutableArray new]; + NSString *formattedString = + [NSString stringWithFormat:@"WHERE %@ = 0", OWSDisappearingMessageFinderExpiresAtColumn]; + + YapDatabaseQuery *query = [YapDatabaseQuery queryWithFormat:formattedString]; + [[transaction ext:OWSDisappearingMessageFinderExpiresAtIndex] + enumerateKeysAndObjectsMatchingQuery:query + usingBlock:^void(NSString *collection, NSString *key, id object, BOOL *stop) { + if (![object isKindOfClass:[TSMessage class]]) { + OWSFail(@"%@ in %s object was unexpected class: %@", + self.logTag, + __PRETTY_FUNCTION__, + object); + return; + } + TSMessage *message = (TSMessage *)object; + if ([message shouldStartExpireTimerWithTransaction:transaction]) { + [messageIds addObject:key]; + } + }]; + + return [messageIds copy]; +} + - (NSArray *)fetchExpiredMessageIdsWithTransaction:(YapDatabaseReadTransaction *_Nonnull)transaction { OWSAssert(transaction); @@ -99,11 +127,33 @@ static NSString *const OWSDisappearingMessageFinderExpiresAtIndex = @"index_mess if ([message isKindOfClass:[TSMessage class]]) { block(message); } else { - DDLogError(@"%@ unexpected object: %@", self.logTag, message); + OWSProdLogAndFail(@"%@ unexpected object: %@", self.logTag, message); } } } +- (void)enumerateMessagesWhichFailedToStartExpiringWithBlock:(void (^_Nonnull)(TSMessage *message))block + transaction:(YapDatabaseReadTransaction *)transaction +{ + OWSAssert(transaction); + + for (NSString *expiringMessageId in [self fetchMessageIdsWhichFailedToStartExpiring:transaction]) { + + TSMessage *_Nullable message = [TSMessage fetchObjectWithUniqueID:expiringMessageId transaction:transaction]; + if (![message isKindOfClass:[TSMessage class]]) { + OWSProdLogAndFail(@"%@ unexpected object: %@", self.logTag, message); + continue; + } + + if (![message shouldStartExpireTimerWithTransaction:transaction]) { + OWSProdLogAndFail(@"%@ object: %@ shouldn't expire.", self.logTag, message); + continue; + } + + block(message); + } +} + /** * Don't use this in production. Useful for testing. * We don't want to instantiate potentially many messages at once. diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h index 032c61add..1d611d8e8 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h @@ -44,6 +44,8 @@ NS_ASSUME_NONNULL_BEGIN // and continue cleaning in the background. - (void)startIfNecessary; +- (void)cleanupMessagesWhichFailedToStartExpiringWithTransaction:(YapDatabaseReadWriteTransaction *)transaction; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index b4ffb03fc..e4aa534ca 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -279,6 +279,12 @@ void AssertIsOnDisappearingMessagesQueue() self.hasStarted = YES; dispatch_async(OWSDisappearingMessagesJob.serialQueue, ^{ + // Theoretically this shouldn't be necessary, but there was a race condition when receiving a backlog + // of messages across timer changes which could cause a disappearing message's timer to never be started. + [self.databaseConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { + [self cleanupMessagesWhichFailedToStartExpiringWithTransaction:transaction]; + }]; + [self runLoop]; }); }); @@ -387,6 +393,18 @@ void AssertIsOnDisappearingMessagesQueue() self.nextDisappearanceDate = nil; } +#pragma mark - Cleanup + +- (void)cleanupMessagesWhichFailedToStartExpiringWithTransaction:(YapDatabaseReadWriteTransaction *)transaction +{ + [self.disappearingMessagesFinder enumerateMessagesWhichFailedToStartExpiringWithBlock:^( + TSMessage *_Nonnull message) { + DDLogWarn(@"%@ starting old timer for message timestamp: %tu", self.logTag, message.timestamp); + [self setExpirationForMessage:message expirationStartedAt:message.timestampForSorting transaction:transaction]; + } + transaction:transaction]; +} + #pragma mark - Notifications - (void)applicationDidBecomeActive:(NSNotification *)notification