Merge branch 'charlesmchen/profileDeadlock2'

Signal/src/Profiles/OWSProfileManager.h

@@ -16,9 +16,9 @@ extern NSString *const kNSNotificationKey_ProfileGroupId;
 extern const NSUInteger kOWSProfileManager_NameDataLength;
 extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter;
 
-@class TSThread;
 @class OWSAES256Key;
 @class OWSMessageSender;
+@class TSThread;
 
 // This class can be safely accessed and used from any thread.
 @interface OWSProfileManager : NSObject <ProfileManagerProtocol>
@@ -63,10 +63,6 @@ extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter;
 
 - (void)addThreadToProfileWhitelist:(TSThread *)thread;
 
-- (BOOL)isThreadInProfileWhitelist:(TSThread *)thread;
-
-- (BOOL)isUserInProfileWhitelist:(NSString *)recipientId;
-
 - (void)setContactRecipientIds:(NSArray<NSString *> *)contactRecipientIds;
 
 #pragma mark - Other User's Profiles

Signal/src/Profiles/OWSProfileManager.m

@@ -30,16 +30,16 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (atomic, readonly) NSString *recipientId;
 @property (atomic, nullable) OWSAES256Key *profileKey;
-@property (nonatomic, nullable) NSString *profileName;
+@property (atomic, nullable) NSString *profileName;
-@property (nonatomic, nullable) NSString *avatarUrlPath;
+@property (atomic, nullable) NSString *avatarUrlPath;
 // This filename is relative to OWSProfileManager.profileAvatarsDirPath.
-@property (nonatomic, nullable) NSString *avatarFileName;
+@property (atomic, nullable) NSString *avatarFileName;
 
 // This should reflect when either:
 //
 // * The last successful update finished.
 // * The current in-flight update began.
-@property (nonatomic, nullable) NSDate *lastUpdateDate;
+@property (atomic, nullable) NSDate *lastUpdateDate;
 
 - (instancetype)init NS_UNAVAILABLE;
 
@@ -49,6 +49,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 @implementation UserProfile
 
+@synthesize profileName = _profileName;
+
 - (instancetype)initWithRecipientId:(NSString *)recipientId
 {
     self = [super initWithUniqueId:recipientId];
@@ -63,23 +65,20 @@ NS_ASSUME_NONNULL_BEGIN
     return self;
 }
 
-- (void)setProfileName:(nullable NSString *)profileName
+- (nullable NSString *)profileName
 {
-    _profileName = [profileName ows_stripped];
+    @synchronized(self)
-}
-
-#pragma mark - NSObject
-
-- (BOOL)isEqual:(UserProfile *)other
     {
-    return ([other isKindOfClass:[UserProfile class]] && [self.recipientId isEqualToString:other.recipientId] &&
+        return _profileName;
-        [self.profileName isEqualToString:other.profileName] && [self.avatarUrlPath isEqualToString:other.avatarUrlPath] &&
+    }
-        [self.avatarFileName isEqualToString:other.avatarFileName]);
 }
 
-- (NSUInteger)hash
+- (void)setProfileName:(nullable NSString *)profileName
+{
+    @synchronized(self)
     {
-    return self.recipientId.hash ^ self.profileName.hash ^ self.avatarUrlPath.hash ^ self.avatarFileName.hash;
+        _profileName = [profileName ows_stripped];
+    }
 }
 
 @end
@@ -234,11 +233,19 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 {
     OWSAssert(userProfile);
 
+    // Make a copy to use inside the transaction.
+    // To avoid deadlock, we want to avoid creating a new transaction while sync'd on self.
+    UserProfile *userProfileCopy;
     @synchronized(self)
     {
+        userProfileCopy = [userProfile copy];
+        // Other threads may modify this profile's properties
+        OWSAssert([userProfile isEqual:userProfileCopy]);
+    }
+
     // Make sure to save on the local db connection for consistency.
     [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-            [userProfile saveWithTransaction:transaction];
+        [userProfileCopy saveWithTransaction:transaction];
     }];
 
     BOOL isLocalUserProfile = userProfile == self.localUserProfile;
@@ -250,8 +257,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                                                 messageSender:self.messageSender
                                                profileManager:self];
 
-                [[NSNotificationCenter defaultCenter]
+            [[NSNotificationCenter defaultCenter] postNotificationNameAsync:kNSNotificationName_LocalProfileDidChange
-                    postNotificationNameAsync:kNSNotificationName_LocalProfileDidChange
                                                                      object:nil
                                                                    userInfo:nil];
         } else {
@@ -270,7 +276,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
         }
     });
 }
-}
 
 - (void)ensureLocalProfileCached
 {
@@ -295,10 +300,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                 DDLogInfo(@"%@ Building local profile.", self.tag);
                 _localUserProfile = [[UserProfile alloc] initWithRecipientId:kLocalProfileUniqueId];
                 _localUserProfile.profileKey = [OWSAES256Key generateRandomKey];
-                [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
+
-                    [_localUserProfile saveWithTransaction:transaction];
-                }];
-                // Sync local profile to any linked device
                 dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
                     @synchronized(self)
                     {
@@ -764,23 +766,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 {
     OWSAssert(recipientId.length > 0);
 
-    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+    [self addUsersToProfileWhitelist:@[ recipientId ]];
-        @synchronized(self)
-        {
-            if ([self isUserInProfileWhitelist:recipientId]) {
-                return;
-            }
-
-            self.userProfileWhitelistCache[recipientId] = @(YES);
-            [self.dbConnection setBool:YES forKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection];
-        }
-
-        [[NSNotificationCenter defaultCenter] postNotificationNameAsync:kNSNotificationName_ProfileWhitelistDidChange
-                                                                 object:nil
-                                                               userInfo:@{
-                                                                   kNSNotificationKey_ProfileRecipientId : recipientId,
-                                                               }];
-    });
 }
 
 - (void)addUsersToProfileWhitelist:(NSArray<NSString *> *)recipientIds
@@ -802,15 +788,18 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                 return;
             }
 
+            for (NSString *recipientId in recipientIds) {
+                self.userProfileWhitelistCache[recipientId] = @(YES);
+            }
+        }
+
         [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
             for (NSString *recipientId in recipientIds) {
                 [transaction setObject:@(YES)
                                 forKey:recipientId
                           inCollection:kOWSProfileManager_UserWhitelistCollection];
-                    self.userProfileWhitelistCache[recipientId] = @(YES);
             }
         }];
-        }
 
         for (NSString *recipientId in newRecipientIds) {
             [[NSNotificationCenter defaultCenter]
@@ -846,24 +835,24 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
     OWSAssert(groupId.length > 0);
 
     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-        @synchronized(self)
-        {
         NSString *groupIdKey = [groupId hexadecimalString];
 
+        @synchronized(self)
+        {
             if ([self isGroupIdInProfileWhitelist:groupId]) {
                 return;
             }
 
-            [self.dbConnection setBool:YES forKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection];
             self.groupProfileWhitelistCache[groupIdKey] = @(YES);
+        }
 
-            [[NSNotificationCenter defaultCenter]
+        [self.dbConnection setBool:YES forKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection];
-                postNotificationNameAsync:kNSNotificationName_ProfileWhitelistDidChange
+
+        [[NSNotificationCenter defaultCenter] postNotificationNameAsync:kNSNotificationName_ProfileWhitelistDidChange
                                                                  object:nil
                                                                userInfo:@{
                                                                    kNSNotificationKey_ProfileGroupId : groupId,
                                                                }];
-        }
     });
 }