Merge pull request #552 from qonversion/fix/sc-35444/propertiesRaces

Attempt to fix crashes and race conditions in User Properties manager
qonversion · Oct 17, 2024 · 88feef8 · 88feef8
2 parents 34ba186 + f0c0825
commit 88feef8
Showing 1 changed file with 41 additions and 27 deletions.
diff --git a/Sources/Qonversion/Qonversion/Main/QNUserPropertiesManager/QNUserPropertiesManager.m b/Sources/Qonversion/Qonversion/Main/QNUserPropertiesManager/QNUserPropertiesManager.m
@@ -26,10 +26,10 @@ @interface QNUserPropertiesManager()
 
 @property (nonatomic) QNInMemoryStorage *inMemoryStorage;
 
-@property (nonatomic, strong) NSMutableArray<QONUserPropertiesEmptyCompletionHandler> *completionBlocks;
+@property (atomic, strong) NSMutableArray<QONUserPropertiesEmptyCompletionHandler> *completionBlocks;
 
-@property (nonatomic, assign, readwrite) BOOL sendingScheduled;
-@property (nonatomic, assign, readwrite) BOOL updatingCurrently;
+@property (atomic, assign, readwrite) BOOL sendingScheduled;
+@property (atomic, assign, readwrite) BOOL updatingCurrently;
 @property (nonatomic, assign, readwrite) NSUInteger retryDelay;
 @property (nonatomic, assign, readwrite) NSUInteger retriesCounter;
 
@@ -94,25 +94,31 @@ - (void)forceSendProperties:(QONUserPropertiesEmptyCompletionHandler)completion
     return;
   }
 
-  [self.completionBlocks addObject:completion];
+  @synchronized (self) {
+    [self.completionBlocks addObject:completion];
+  }
 
   [self sendProperties:YES];
 }
 
 - (void)sendPropertiesWithDelay:(NSUInteger)delay {
-  if (!_sendingScheduled) {
-    _sendingScheduled = YES;
-    __block __weak QNUserPropertiesManager *weakSelf = self;
-    [_backgroundQueue addOperationWithBlock:^{
-      dispatch_async(dispatch_get_main_queue(), ^{
-        [weakSelf performSelector:@selector(sendPropertiesInBackground) withObject:nil afterDelay:delay];
-      });
-    }];
+  @synchronized (self) {
+    if (!self.sendingScheduled) {
+      self.sendingScheduled = YES;
+      __block __weak QNUserPropertiesManager *weakSelf = self;
+      [self.backgroundQueue addOperationWithBlock:^{
+        dispatch_async(dispatch_get_main_queue(), ^{
+          [weakSelf performSelector:@selector(sendPropertiesInBackground) withObject:nil afterDelay:delay];
+        });
+      }];
+    }
   }
 }
 
 - (void)sendPropertiesInBackground {
-  _sendingScheduled = NO;
+  @synchronized (self) {
+    self.sendingScheduled = NO;
+  }
   [self sendProperties];
 }
 
@@ -121,44 +127,52 @@ - (void)sendProperties {
 }
 
 - (void)sendProperties:(BOOL)force {
-  if ([QNUtils isEmptyString:_apiClient.apiKey]) {
+  if ([QNUtils isEmptyString:self.apiClient.apiKey]) {
     QONVERSION_ERROR(@"ERROR: apiKey cannot be nil or empty, set apiKey with launchWithKey:");
     return;
   }
 
   @synchronized (self) {
-    if (_updatingCurrently && !force) {
+    if (self.updatingCurrently && !force) {
       return;
     }
-    _updatingCurrently = YES;
+    self.updatingCurrently = YES;
   }
 
   [self runOnBackgroundQueue:^{
-    NSDictionary *properties = [self->_inMemoryStorage.storageDictionary copy];
+    NSDictionary *properties = [self.inMemoryStorage.storageDictionary copy];
 
     if (!properties || ![properties respondsToSelector:@selector(valueForKey:)]) {
-      self->_updatingCurrently = NO;
+      @synchronized (self) {
+        self.updatingCurrently = NO;
+      }
       return;
     }
 
     if (properties.count == 0) {
-      self->_updatingCurrently = NO;
+      @synchronized (self) {
+        self.updatingCurrently = NO;
+      }
       return;
     }
 
     self.inMemoryStorage.storageDictionary = @{};
     __block __weak QNUserPropertiesManager *weakSelf = self;
     [self.apiClient sendProperties:properties
                         completion:^(NSDictionary * _Nullable dict, NSError * _Nullable error) {
-      weakSelf.updatingCurrently = NO;
 
-      NSArray *completions = [weakSelf.completionBlocks copy];
+      NSArray *completions = @[];
+      @synchronized (self) {
+        weakSelf.updatingCurrently = NO;
+
+        completions = [weakSelf.completionBlocks copy];
+        [weakSelf.completionBlocks removeAllObjects];
+      }
+
       for (QONUserPropertiesEmptyCompletionHandler storedCompletion in completions) {
         storedCompletion();
       }
 
-      [weakSelf.completionBlocks removeAllObjects];
-
       if (error) {
         // copy of an existing array to prevent erasing properties set while the current request is in progress
         NSMutableDictionary *allProperties = [self.inMemoryStorage.storageDictionary mutableCopy];
@@ -210,7 +224,7 @@ - (BOOL)runOnBackgroundQueue:(void (^)(void))block {
     block();
     return NO;
   } else {
-    [_backgroundQueue addOperationWithBlock:block];
+    [self.backgroundQueue addOperationWithBlock:block];
     return YES;
   }
 }
@@ -223,18 +237,18 @@ - (void)collectIntegrationsData {
 }
 
 - (void)collectIntegrationsDataInBackground {
-  [_device adjustUserIDWithCompletion:^(NSString * _Nullable userId) {
+  [self.device adjustUserIDWithCompletion:^(NSString * _Nullable userId) {
     if (![QNUtils isEmptyString:userId]) {
       [self setUserProperty:@"_q_adjust_adid" value:userId];
     }
   }];
 
-  NSString *fbAnonID = _device.fbAnonID;
+  NSString *fbAnonID = self.device.fbAnonID;
   if (![QNUtils isEmptyString:fbAnonID]) {
     [self setUserProperty:@"_q_fb_anon_id" value:fbAnonID];
   }
 
-  NSString *afUserID = _device.afUserID;
+  NSString *afUserID = self.device.afUserID;
   if (![QNUtils isEmptyString:afUserID]) {
     [self setUserProperty:@"_q_appsflyer_user_id" value:afUserID];
   }