Access contacts atomically to avoid corruption (PR #1223)

4% of our crashes are from accessing latestContactsById with an invalid
address. This was partially addressed by assigning a value on init, but
it's still happening.

By converting the ivar to an atomic property we can avoid any funny
business that would happen when accessing this var while it's being
updated in a different thread.

Also, make sure we're copying to defensively strip mutability.

Also:
* bumped release target
* removed unused code
* dealloc at the top per style.
This commit is contained in:
Michael Kirk 2016-06-16 13:54:33 -07:00 committed by GitHub
parent 8f04f863b7
commit 4a1c53f624
3 changed files with 27 additions and 58 deletions

View File

@ -21,7 +21,7 @@
<key>CFBundlePackageType</key> <key>CFBundlePackageType</key>
<string>APPL</string> <string>APPL</string>
<key>CFBundleShortVersionString</key> <key>CFBundleShortVersionString</key>
<string>2.3.2</string> <string>2.3.3</string>
<key>CFBundleSignature</key> <key>CFBundleSignature</key>
<string>????</string> <string>????</string>
<key>CFBundleURLTypes</key> <key>CFBundleURLTypes</key>
@ -38,7 +38,7 @@
</dict> </dict>
</array> </array>
<key>CFBundleVersion</key> <key>CFBundleVersion</key>
<string>2.3.2.1</string> <string>2.3.3.0</string>
<key>ITSAppUsesNonExemptEncryption</key> <key>ITSAppUsesNonExemptEncryption</key>
<false/> <false/>
<key>LOGS_EMAIL</key> <key>LOGS_EMAIL</key>

View File

@ -21,18 +21,7 @@
typedef void (^ABAccessRequestCompletionBlock)(BOOL hasAccess); typedef void (^ABAccessRequestCompletionBlock)(BOOL hasAccess);
typedef void (^ABReloadRequestCompletionBlock)(NSArray *contacts); typedef void (^ABReloadRequestCompletionBlock)(NSArray *contacts);
@interface ContactsManager : NSObject <ContactsManagerProtocol> { @interface ContactsManager : NSObject <ContactsManagerProtocol>
@private
TOCFuture *futureAddressBook;
@private
ObservableValueController *observableContactsController;
@private
TOCCancelTokenSource *life;
@private
NSDictionary *latestContactsById;
@private
NSDictionary *latestWhisperUsersById;
}
@property CNContactStore *contactStore; @property CNContactStore *contactStore;

View File

@ -7,20 +7,28 @@
typedef BOOL (^ContactSearchBlock)(id, NSUInteger, BOOL *); typedef BOOL (^ContactSearchBlock)(id, NSUInteger, BOOL *);
@interface ContactsManager () { @interface ContactsManager ()
id addressBookReference;
} @property id addressBookReference;
@property TOCFuture *futureAddressBook;
@property ObservableValueController *observableContactsController;
@property TOCCancelTokenSource *life;
@property(atomic, copy) NSDictionary *latestContactsById;
@end @end
@implementation ContactsManager @implementation ContactsManager
- (void)dealloc {
[_life cancel];
}
- (id)init { - (id)init {
self = [super init]; self = [super init];
if (self) { if (self) {
life = [TOCCancelTokenSource new]; _life = [TOCCancelTokenSource new];
observableContactsController = [ObservableValueController observableValueControllerWithInitialValue:nil]; _observableContactsController = [ObservableValueController observableValueControllerWithInitialValue:nil];
latestContactsById = @{}; _latestContactsById = @{};
} }
return self; return self;
} }
@ -39,20 +47,16 @@ typedef BOOL (^ContactSearchBlock)(id, NSUInteger, BOOL *);
[self setupAddressBook]; [self setupAddressBook];
[observableContactsController watchLatestValueOnArbitraryThread:^(NSArray *latestContacts) { [self.observableContactsController watchLatestValueOnArbitraryThread:^(NSArray *latestContacts) {
@synchronized(self) { @synchronized(self) {
[self setupLatestContacts:latestContacts]; [self setupLatestContacts:latestContacts];
} }
} }
untilCancelled:life.token]; untilCancelled:_life.token];
}
- (void)dealloc {
[life cancel];
} }
- (void)verifyABPermission { - (void)verifyABPermission {
if (!addressBookReference) { if (!self.addressBookReference) {
[self setupAddressBook]; [self setupAddressBook];
} }
} }
@ -73,7 +77,7 @@ void onAddressBookChanged(ABAddressBookRef notifyAddressBook, CFDictionaryRef in
- (void)setupAddressBook { - (void)setupAddressBook {
dispatch_async(ADDRESSBOOK_QUEUE, ^{ dispatch_async(ADDRESSBOOK_QUEUE, ^{
[[ContactsManager asyncGetAddressBook] thenDo:^(id addressBook) { [[ContactsManager asyncGetAddressBook] thenDo:^(id addressBook) {
addressBookReference = addressBook; self.addressBookReference = addressBook;
ABAddressBookRef cfAddressBook = (__bridge ABAddressBookRef)addressBook; ABAddressBookRef cfAddressBook = (__bridge ABAddressBookRef)addressBook;
ABAddressBookRegisterExternalChangeCallback(cfAddressBook, onAddressBookChanged, (__bridge void *)self); ABAddressBookRegisterExternalChangeCallback(cfAddressBook, onAddressBookChanged, (__bridge void *)self);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
@ -106,12 +110,12 @@ void onAddressBookChanged(ABAddressBookRef notifyAddressBook, CFDictionaryRef in
[ContactsManager blockingContactDialog]; [ContactsManager blockingContactDialog];
} }
}); });
[observableContactsController updateValue:[self getContactsFromAddressBook:addressBookRef]]; [self.observableContactsController updateValue:[self getContactsFromAddressBook:addressBookRef]];
} }
- (void)setupLatestContacts:(NSArray *)contacts { - (void)setupLatestContacts:(NSArray *)contacts {
if (contacts) { if (contacts) {
latestContactsById = [ContactsManager keyContactsById:contacts]; self.latestContactsById = [ContactsManager keyContactsById:contacts];
} }
} }
@ -174,16 +178,10 @@ void onAddressBookChanged(ABAddressBookRef notifyAddressBook, CFDictionaryRef in
} }
} }
- (void)setupLatestRedPhoneUsers:(NSArray *)users {
if (users) {
latestWhisperUsersById = [ContactsManager keyContactsById:users];
}
}
#pragma mark - Observables #pragma mark - Observables
- (ObservableValue *)getObservableContacts { - (ObservableValue *)getObservableContacts {
return observableContactsController; return self.observableContactsController;
} }
#pragma mark - Address Book utils #pragma mark - Address Book utils
@ -251,7 +249,7 @@ void onAddressBookChanged(ABAddressBookRef notifyAddressBook, CFDictionaryRef in
} }
- (NSArray *)latestContactsWithSearchString:(NSString *)searchString { - (NSArray *)latestContactsWithSearchString:(NSString *)searchString {
return [latestContactsById.allValues filter:^int(Contact *contact) { return [self.latestContactsById.allValues filter:^int(Contact *contact) {
return searchString.length == 0 || [ContactsManager name:contact.fullName matchesQuery:searchString]; return searchString.length == 0 || [ContactsManager name:contact.fullName matchesQuery:searchString];
}]; }];
} }
@ -395,17 +393,11 @@ void onAddressBookChanged(ABAddressBookRef notifyAddressBook, CFDictionaryRef in
}]; }];
} }
- (Contact *)latestContactWithRecordId:(ABRecordID)recordId {
@synchronized(self) {
return latestContactsById[@(recordId)];
}
}
- (NSArray<Contact *> *)allContacts { - (NSArray<Contact *> *)allContacts {
NSMutableArray *allContacts = [NSMutableArray array]; NSMutableArray *allContacts = [NSMutableArray array];
for (NSString *key in latestContactsById.allKeys) { for (NSString *key in self.latestContactsById.allKeys) {
Contact *contact = [latestContactsById objectForKey:key]; Contact *contact = [self.latestContactsById objectForKey:key];
if ([contact isKindOfClass:[Contact class]]) { if ([contact isKindOfClass:[Contact class]]) {
[allContacts addObject:contact]; [allContacts addObject:contact];
@ -445,18 +437,6 @@ void onAddressBookChanged(ABAddressBookRef notifyAddressBook, CFDictionaryRef in
return [searchString rangeOfString:queryString options:searchOpts].location != NSNotFound; return [searchString rangeOfString:queryString options:searchOpts].location != NSNotFound;
} }
- (NSArray *)contactsForContactIds:(NSArray *)contactIds {
NSMutableArray *contacts = [NSMutableArray array];
for (NSNumber *favouriteId in contactIds) {
Contact *contact = [self latestContactWithRecordId:favouriteId.intValue];
if (contact) {
[contacts addObject:contact];
}
}
return [contacts copy];
}
#pragma mark - Whisper User Management #pragma mark - Whisper User Management
- (NSArray *)getSignalUsersFromContactsArray:(NSArray *)contacts { - (NSArray *)getSignalUsersFromContactsArray:(NSArray *)contacts {