Rework concurrency in the profile manager.

// FREEBIE

Signal/src/Profiles/OWSProfileManager.m

@@ -20,18 +20,14 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-// UserProfile properties should only be mutated on the main thread.
+// UserProfile properties may be read from any thread, but should
+// only be mutated when synchronized on the profile manager.
 @interface UserProfile : TSYapDatabaseObject
 
-// These properties may be accessed from any thread.
 @property (atomic, readonly) NSString *recipientId;
 @property (atomic, nullable) OWSAES128Key *profileKey;
-
-// These properties may be accessed only from the main thread.
 @property (nonatomic, nullable) NSString *profileName;
-
 @property (nonatomic, nullable) NSString *avatarUrlPath;
-
 // This filename is relative to OWSProfileManager.profileAvatarsDirPath.
 @property (nonatomic, nullable) NSString *avatarFileName;
 
@@ -39,8 +35,6 @@ NS_ASSUME_NONNULL_BEGIN
 //
 // * The last successful update finished.
 // * The current in-flight update began.
-//
-// This property may be accessed from any thread.
 @property (nonatomic, nullable) NSDate *lastUpdateDate;
 
 - (instancetype)init NS_UNAVAILABLE;
@@ -102,26 +96,26 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 @property (nonatomic, readonly) YapDatabaseConnection *dbConnection;
 @property (nonatomic, readonly) TSNetworkManager *networkManager;
 
+// These properties can be accessed on any thread, while synchronized on self.
 @property (atomic, nullable) UserProfile *localUserProfile;
-// This property should only be mutated on the main thread,
+@property (atomic, nullable) UIImage *localCachedAvatarImage;
-@property (nonatomic, nullable) UIImage *localCachedAvatarImage;
 
 // These caches are lazy-populated.  The single point of truth is the database.
 //
-// These three properties can be accessed on any thread.
+// These properties can be accessed on any thread, while synchronized on self.
 @property (atomic, readonly) NSMutableDictionary<NSString *, NSNumber *> *userProfileWhitelistCache;
 @property (atomic, readonly) NSMutableDictionary<NSString *, NSNumber *> *groupProfileWhitelistCache;
 
-// This property should only be mutated on the main thread,
+// These properties can be accessed on any thread, while synchronized on self.
-@property (nonatomic, readonly) NSCache<NSString *, UIImage *> *otherUsersProfileAvatarImageCache;
+@property (atomic, readonly) NSCache<NSString *, UIImage *> *otherUsersProfileAvatarImageCache;
-
-// This property should only be mutated on the main thread,
 @property (atomic, readonly) NSMutableSet<NSString *> *currentAvatarDownloads;
 
 @end
 
 #pragma mark -
 
+// Access to most state should happen while synchronized on the profile manager.
+// Writes should happen off the main thread, wherever possible.
 @implementation OWSProfileManager
 
 + (instancetype)sharedManager
@@ -226,55 +220,63 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
     return instance;
 }
 
-// All writes to user profiles should occur on the main thread.
 - (void)saveUserProfile:(UserProfile *)userProfile
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(userProfile);
 
+    @synchronized(self)
+    {
         // Make sure to save on the local db connection for consistency.
         [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
             [userProfile saveWithTransaction:transaction];
         }];
 
-    if (userProfile == self.localUserProfile) {
+        BOOL isLocalUserProfile = userProfile == self.localUserProfile;
+
+        dispatch_async(dispatch_get_main_queue(), ^{
+            if (isLocalUserProfile) {
                 [[NSNotificationCenter defaultCenter] postNotificationName:kNSNotificationName_LocalProfileDidChange
                                                                     object:nil
                                                                   userInfo:nil];
             } else {
-        [[NSNotificationCenter defaultCenter] postNotificationName:kNSNotificationName_OtherUsersProfileDidChange
+                [[NSNotificationCenter defaultCenter]
+                    postNotificationName:kNSNotificationName_OtherUsersProfileDidChange
                                   object:nil
                                 userInfo:nil];
             }
+        });
+    }
 }
 
 #pragma mark - Local Profile
 
 - (OWSAES128Key *)localProfileKey
 {
+    @synchronized(self)
+    {
         OWSAssert(self.localUserProfile.profileKey.keyData.length == kAES128_KeyByteLength);
 
         return self.localUserProfile.profileKey;
+    }
 }
 
 - (BOOL)hasLocalProfile
 {
-    OWSAssert([NSThread isMainThread]);
-
     return (self.localProfileName.length > 0 || self.localProfileAvatarImage != nil);
 }
 
 - (nullable NSString *)localProfileName
 {
-    OWSAssert([NSThread isMainThread]);
+    @synchronized(self)
-
+    {
         return self.localUserProfile.profileName;
+    }
 }
 
 - (nullable UIImage *)localProfileAvatarImage
 {
-    OWSAssert([NSThread isMainThread]);
+    @synchronized(self)
-
+    {
         if (!self.localCachedAvatarImage) {
             if (self.localUserProfile.avatarFileName) {
                 self.localCachedAvatarImage = [self loadProfileAvatarWithFilename:self.localUserProfile.avatarFileName];
@@ -282,40 +284,50 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
         }
 
         return self.localCachedAvatarImage;
+    }
 }
 
 - (void)updateLocalProfileName:(nullable NSString *)profileName
                    avatarImage:(nullable UIImage *)avatarImage
-                       success:(void (^)())successBlock
+                       success:(void (^)())successBlockParameter
                        failure:(void (^)())failureBlockParameter
 {
-    OWSAssert([NSThread isMainThread]);
+    OWSAssert(successBlockParameter);
-    OWSAssert(successBlock);
     OWSAssert(failureBlockParameter);
 
-    // Ensure that the failure block is called on the main thread.
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        @synchronized(self)
+        {
+            // Ensure that the success and failure blocks are called on the main thread.
             void (^failureBlock)() = ^{
                 dispatch_async(dispatch_get_main_queue(), ^{
                     failureBlockParameter();
                 });
             };
+            void (^successBlock)() = ^{
+                dispatch_async(dispatch_get_main_queue(), ^{
+                    successBlockParameter();
+                });
+            };
 
             // The final steps are to:
             //
             // * Try to update the service.
             // * Update client state on success.
-    void (^tryToUpdateService)(NSString *_Nullable, NSString *_Nullable) = ^(
+            void (^tryToUpdateService)(NSString *_Nullable, NSString *_Nullable)
-        NSString *_Nullable avatarUrlPath, NSString *_Nullable avatarFileName) {
+                = ^(NSString *_Nullable avatarUrlPath, NSString *_Nullable avatarFileName) {
                       [self updateServiceWithProfileName:profileName
                           success:^{
-                // All reads and writes to user profiles should happen on the main thread.
+                              dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-                dispatch_async(dispatch_get_main_queue(), ^{
+                                  @synchronized(self)
+                                  {
                                       UserProfile *userProfile = self.localUserProfile;
                                       OWSAssert(userProfile);
                                       userProfile.profileName = profileName;
 
                                       // TODO remote avatarUrlPath changes as result of fetching form -
-                    // we should probably invalidate it at that point, and refresh again when uploading file completes.
+                                      // we should probably invalidate it at that point, and refresh again when
+                                      // uploading file completes.
                                       userProfile.avatarUrlPath = avatarUrlPath;
                                       userProfile.avatarFileName = avatarFileName;
 
@@ -324,6 +336,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                                       self.localCachedAvatarImage = avatarImage;
 
                                       successBlock();
+                                  }
                               });
                           }
                           failure:^{
@@ -335,7 +348,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
             OWSAssert(userProfile);
 
             if (avatarImage) {
-
                 // If we have a new avatar image, we must first:
                 //
                 // * Encode it to JPEG.
@@ -370,6 +382,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                 DDLogVerbose(@"%@ Updating local profile on service with no avatar.", self.tag);
                 tryToUpdateService(nil, nil);
             }
+        }
+    });
 }
 
 - (void)writeAvatarToDisk:(UIImage *)avatar
@@ -580,18 +594,24 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
 - (void)addUserToProfileWhitelist:(NSString *)recipientId
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(recipientId.length > 0);
 
-    [self.dbConnection setBool:YES forKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection];
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        @synchronized(self)
+        {
             self.userProfileWhitelistCache[recipientId] = @(YES);
+            [self.dbConnection setBool:YES forKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection];
+        }
+    });
 }
 
 - (void)addUsersToProfileWhitelist:(NSArray<NSString *> *)recipientIds
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(recipientIds);
 
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        @synchronized(self)
+        {
             NSMutableArray<NSString *> *newRecipientIds = [NSMutableArray new];
             for (NSString *recipientId in recipientIds) {
                 if (!self.userProfileWhitelistCache[recipientId]) {
@@ -605,33 +625,44 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
             [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
                 for (NSString *recipientId in recipientIds) {
-            [transaction setObject:@(YES) forKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection];
+                    [transaction setObject:@(YES)
+                                    forKey:recipientId
+                              inCollection:kOWSProfileManager_UserWhitelistCollection];
                     self.userProfileWhitelistCache[recipientId] = @(YES);
                 }
             }];
+        }
+    });
 }
 
 - (BOOL)isUserInProfileWhitelist:(NSString *)recipientId
 {
     OWSAssert(recipientId.length > 0);
 
+    @synchronized(self)
+    {
         NSNumber *_Nullable value = self.userProfileWhitelistCache[recipientId];
         if (value) {
             return [value boolValue];
         }
 
-    value = @([self.dbConnection hasObjectForKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection]);
+        value =
+            @([self.dbConnection hasObjectForKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection]);
         self.userProfileWhitelistCache[recipientId] = value;
         return [value boolValue];
+    }
 }
 
 - (void)addGroupIdToProfileWhitelist:(NSData *)groupId
 {
     OWSAssert(groupId.length > 0);
 
+    @synchronized(self)
+    {
         NSString *groupIdKey = [groupId hexadecimalString];
         [self.dbConnection setObject:@(1) forKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection];
         self.groupProfileWhitelistCache[groupIdKey] = @(YES);
+    }
 }
 
 - (void)addThreadToProfileWhitelist:(TSThread *)thread
@@ -652,16 +683,19 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 {
     OWSAssert(groupId.length > 0);
 
+    @synchronized(self)
+    {
         NSString *groupIdKey = [groupId hexadecimalString];
         NSNumber *_Nullable value = self.groupProfileWhitelistCache[groupIdKey];
         if (value) {
             return [value boolValue];
         }
 
-    value =
+        value = @(nil !=
-        @(nil != [self.dbConnection objectForKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection]);
+            [self.dbConnection objectForKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection]);
         self.groupProfileWhitelistCache[groupIdKey] = value;
         return [value boolValue];
+    }
 }
 
 - (BOOL)isThreadInProfileWhitelist:(TSThread *)thread
@@ -680,7 +714,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
 - (void)setContactRecipientIds:(NSArray<NSString *> *)contactRecipientIds
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(contactRecipientIds);
 
     // TODO: The persisted whitelist could either be:
@@ -695,13 +728,15 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
 - (void)setProfileKeyData:(NSData *)profileKeyData forRecipientId:(NSString *)recipientId;
 {
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        @synchronized(self)
+        {
             OWSAES128Key *_Nullable profileKey = [OWSAES128Key keyWithData:profileKeyData];
             if (profileKey == nil) {
                 OWSFail(@"Failed to make profile key for key data");
                 return;
             }
 
-    dispatch_async(dispatch_get_main_queue(), ^{
             UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId];
             OWSAssert(userProfile);
             if (userProfile.profileKey && [userProfile.profileKey.keyData isEqual:profileKey.keyData]) {
@@ -719,6 +754,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
             [self saveUserProfile:userProfile];
 
             [self refreshProfileForRecipientId:recipientId ignoreThrottling:YES];
+        }
     });
 }
 
@@ -726,20 +762,26 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 {
     OWSAssert(recipientId.length > 0);
 
+    @synchronized(self)
+    {
         UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId];
         OWSAssert(userProfile);
         return userProfile.profileKey;
+    }
 }
 
 - (nullable NSString *)profileNameForRecipientId:(NSString *)recipientId
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(recipientId.length > 0);
 
     [self refreshProfileForRecipientId:recipientId];
 
+    @synchronized(self)
+    {
         UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId];
         return userProfile.profileName;
+        return self.localUserProfile.profileName;
+    }
 }
 
 - (nullable NSData *)profileAvatarDataForRecipientId:(NSString *)recipientId
@@ -753,11 +795,12 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
 - (nullable UIImage *)profileAvatarForRecipientId:(NSString *)recipientId
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(recipientId.length > 0);
 
     [self refreshProfileForRecipientId:recipientId];
 
+    @synchronized(self)
+    {
         UIImage *_Nullable image = [self.otherUsersProfileAvatarImageCache objectForKey:recipientId];
         if (image) {
             return image;
@@ -774,17 +817,21 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
         }
 
         return image;
+    }
 }
 
 - (void)downloadAvatarForUserProfile:(UserProfile *)userProfile
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(userProfile);
 
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        @synchronized(self)
+        {
             if (userProfile.avatarUrlPath.length < 1) {
                 OWSFail(@"%@ Malformed avatar URL: %@", self.tag, userProfile.avatarUrlPath);
                 return;
             }
+            NSString *_Nullable avatarUrlPathAtStart = userProfile.avatarUrlPath;
 
             if (userProfile.profileKey.keyData.length < 1 || userProfile.avatarUrlPath.length < 1) {
                 return;
@@ -804,7 +851,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
             NSString *tempDirectory = NSTemporaryDirectory();
             NSString *tempFilePath = [tempDirectory stringByAppendingPathComponent:fileName];
 
-    NSURL *avatarUrlPath = [NSURL URLWithString:userProfile.avatarUrlPath relativeToURL:self.avatarHTTPManager.baseURL];
+            NSURL *avatarUrlPath =
+                [NSURL URLWithString:userProfile.avatarUrlPath relativeToURL:self.avatarHTTPManager.baseURL];
             NSURLRequest *request = [NSURLRequest requestWithURL:avatarUrlPath];
             NSURLSessionDownloadTask *downloadTask = [self.avatarHTTPManager downloadTaskWithRequest:request
                 progress:^(NSProgress *_Nonnull downloadProgress) {
@@ -818,7 +866,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                     // Ensure disk IO and decryption occurs off the main thread.
                     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
                         NSData *_Nullable encryptedData = (error ? nil : [NSData dataWithContentsOfFile:tempFilePath]);
-                NSData *_Nullable decryptedData = [self decryptProfileData:encryptedData profileKey:profileKeyAtStart];
+                        NSData *_Nullable decryptedData =
+                            [self decryptProfileData:encryptedData profileKey:profileKeyAtStart];
                         UIImage *_Nullable image = nil;
                         if (decryptedData) {
                             BOOL success = [decryptedData writeToFile:filePath atomically:YES];
@@ -827,7 +876,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                             }
                         }
 
-                dispatch_async(dispatch_get_main_queue(), ^{
+                        @synchronized(self)
+                        {
                             [self.currentAvatarDownloads removeObject:userProfile.recipientId];
 
                             UserProfile *currentUserProfile =
@@ -835,6 +885,11 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                             if (currentUserProfile.profileKey.keyData.length < 1
                                 || ![currentUserProfile.profileKey isEqual:userProfile.profileKey]) {
                                 DDLogWarn(@"%@ Ignoring avatar download for obsolete user profile.", self.tag);
+                            } else if (![avatarUrlPathAtStart isEqualToString:currentUserProfile.avatarUrlPath]) {
+                                DDLogInfo(@"%@ avatar url has changed during download", self.tag);
+                                if (currentUserProfile.avatarUrlPath.length > 0) {
+                                    [self downloadAvatarForUserProfile:currentUserProfile];
+                                }
                             } else if (error) {
                                 DDLogError(@"%@ avatar download failed: %@", self.tag, error);
                             } else if (!encryptedData) {
@@ -844,16 +899,19 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
                             } else if (!image) {
                                 DDLogError(@"%@ avatar image could not be loaded: %@", self.tag, error);
                             } else {
+                                // TODO: Verify that the avatar URL hasn't changed since the download began.
                                 [self.otherUsersProfileAvatarImageCache setObject:image forKey:userProfile.recipientId];
 
                                 userProfile.avatarFileName = fileName;
 
                                 [self saveUserProfile:userProfile];
                             }
-                });
+                        }
                     });
                 }];
             [downloadTask resume];
+        }
+    });
 }
 
 - (void)refreshProfileForRecipientId:(NSString *)recipientId
@@ -863,9 +921,11 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
 - (void)refreshProfileForRecipientId:(NSString *)recipientId ignoreThrottling:(BOOL)ignoreThrottling
 {
-    OWSAssert([NSThread isMainThread]);
     OWSAssert(recipientId.length > 0);
 
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        @synchronized(self)
+        {
             UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId];
 
             if (!userProfile.profileKey) {
@@ -877,7 +937,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
             // Throttle and debounce the updates.
             const NSTimeInterval kMaxRefreshFrequency = 5 * kMinuteInterval;
-    if (userProfile.lastUpdateDate && fabs([userProfile.lastUpdateDate timeIntervalSinceNow]) < kMaxRefreshFrequency) {
+            if (userProfile.lastUpdateDate
+                && fabs([userProfile.lastUpdateDate timeIntervalSinceNow]) < kMaxRefreshFrequency) {
                 // This profile was updated recently or already has an update in flight.
                 return;
             }
@@ -889,6 +950,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
             [ProfileFetcherJob runWithRecipientId:recipientId
                                    networkManager:self.networkManager
                                  ignoreThrottling:ignoreThrottling];
+        }
+    });
 }
 
 - (void)updateProfileForRecipientId:(NSString *)recipientId
@@ -899,7 +962,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
     // Ensure decryption, etc. off main thread.
     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-
+        @synchronized(self)
+        {
             UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId];
             if (!userProfile.profileKey) {
                 return;
@@ -910,7 +974,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
             BOOL isAvatarSame = [self isNullableStringEqual:userProfile.avatarUrlPath toString:avatarUrlPath];
 
-        dispatch_async(dispatch_get_main_queue(), ^{
             userProfile.profileName = profileName;
             userProfile.avatarUrlPath = avatarUrlPath;
 
@@ -926,7 +989,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
             userProfile.lastUpdateDate = [NSDate new];
 
             [self saveUserProfile:userProfile];
-        });
+        }
     });
 }