Merge pull request #138 from bitstadium/release/5.1.0

Release/5.1.0

Classes/Feedback/BITFeedbackManager.m

@@ -865,7 +865,9 @@ - (void)fetchMessageUpdates {
                              withMessage:nil
                        completionHandler:^(NSError * __unused err){
                          // inform the UI to update its data in case the list is already showing
-                         [[NSNotificationCenter defaultCenter] postNotificationName:BITHockeyFeedbackMessagesLoadingFinished object:nil];
+                         dispatch_async(dispatch_get_main_queue(),^{
+                           [[NSNotificationCenter defaultCenter] postNotificationName:BITHockeyFeedbackMessagesLoadingFinished object:nil];
+                         });
                        }];
 }
 
@@ -908,7 +910,9 @@ - (void)submitPendingMessages {
                            }
 
                            // inform the UI to update its data in case the list is already showing
-                           [[NSNotificationCenter defaultCenter] postNotificationName:BITHockeyFeedbackMessagesLoadingFinished object:nil];
+                           dispatch_async(dispatch_get_main_queue(),^{
+                             [[NSNotificationCenter defaultCenter] postNotificationName:BITHockeyFeedbackMessagesLoadingFinished object:nil];
+                           });
                          }];
   }
 }

Classes/Feedback/BITFeedbackWindowController.m

@@ -399,12 +399,8 @@ - (void)startLoadingIndicator {
 }
 
 - (void)stopLoadingIndicator {
-
-  // We need to update the UI on the main thread to make sure it updates right away.
-  dispatch_async( dispatch_get_main_queue(), ^{
-    [self.statusBarLoadingIndicator stopAnimation:self];
-    [self.statusBarLoadingIndicator setHidden:YES];
-  });
+  [self.statusBarLoadingIndicator stopAnimation:self];
+  [self.statusBarLoadingIndicator setHidden:YES];
   [self updateLastUpdate];
 }
 

Classes/Telemetry/BITChannel.h

@@ -8,7 +8,10 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-FOUNDATION_EXPORT char *BITSafeJsonEventsString;
+/**
+ * Buffer of telemtry events, will be written to disk. Make sure the buffer is used in a threadsafe way.
+ */
+FOUNDATION_EXPORT char *_Nullable BITTelemetryEventBuffer;
 
 /**
  *  Items get queued before they are persisted and sent out as a batch. This class managed the queue, and forwards the batch

Classes/Telemetry/BITChannel.m

@@ -11,7 +11,7 @@
 #import "BITPersistencePrivate.h"
 
 static char *const BITDataItemsOperationsQueue = "net.hockeyapp.senderQueue";
-char *BITSafeJsonEventsString;
+char *BITTelemetryEventBuffer;
 
 NSString *const BITChannelBlockedNotification = @"BITChannelBlockedNotification";
 
@@ -33,7 +33,7 @@ @implementation BITChannel
 
 - (instancetype)init {
   if ((self = [super init])) {
-    bit_resetSafeJsonStream(&BITSafeJsonEventsString);
+    bit_resetEventBuffer(&BITTelemetryEventBuffer);
     _dataItemCount = 0;
     if (bit_isDebuggerAttached()) {
       _maxBatchSize = BITDebugMaxBatchSize;
@@ -69,59 +69,88 @@ - (BOOL)isQueueBusy {
   return self.channelBlocked;
 }
 
-- (void)persistDataItemQueue {
+- (void)persistDataItemQueue:(char **)eventBuffer {
   [self invalidateTimer];
-  if (!BITSafeJsonEventsString || strlen(BITSafeJsonEventsString) == 0) {
+
+  // Make sure string (which points to BITTelemetryEventBuffer) is not changed.
+  char *previousBuffer = NULL;
+  char *newEmptyString = NULL;
+  do {
+    newEmptyString = strdup("");
+    previousBuffer = *eventBuffer;
+
+    // This swaps pointers and makes sure eventBuffer now has the balue of newEmptyString.
+    if (OSAtomicCompareAndSwapPtr(previousBuffer, newEmptyString, (void*)eventBuffer)) {
+      @synchronized(self) {
+        self.dataItemCount = 0;
+      }
+      break;
+    }
+  } while(true);
+
+  // Nothing to persist, freeing memory and existing.
+  if (!previousBuffer || strlen(previousBuffer) == 0) {
+    free(previousBuffer);
     return;
   }
-
-  NSData *bundle = [NSData dataWithBytes:BITSafeJsonEventsString length:strlen(BITSafeJsonEventsString)];
+
+  // Persist the data
+  NSData *bundle = [NSData dataWithBytes:previousBuffer length:strlen(previousBuffer)];
   [self.persistence persistBundle:bundle];
-
+  free(previousBuffer);
+
   // Reset both, the async-signal-safe and item counter.
   [self resetQueue];
 }
 
 - (void)resetQueue {
-  bit_resetSafeJsonStream(&BITSafeJsonEventsString);
-  self.dataItemCount = 0;
+  @synchronized (self) {
+    bit_resetEventBuffer(&BITTelemetryEventBuffer);
+    self.dataItemCount = 0;
+  }
 }
 
 #pragma mark - Adding to queue
 
 - (void)enqueueTelemetryItem:(BITTelemetryData *)item {
 
   if (!item) {
-    // Case 1: Item is nil: Do not enqueue item and abort operation
+
+    // Item is nil: Do not enqueue item and abort operation.
     BITHockeyLogWarning(@"WARNING: TelemetryItem was nil.");
     return;
   }
 
+  // First assigning self to weakSelf and then assigning this to strongSelf in the block is not very intuitive, this
+  // blog post explains it very well: https://dhoerl.wordpress.com/2013/04/23/i-finally-figured-out-weakself-and-strongself/
   __weak typeof(self) weakSelf = self;
   dispatch_async(self.dataItemsOperations, ^{
+
     typeof(self) strongSelf = weakSelf;
-
     if (strongSelf.isQueueBusy) {
-      // Case 2: Channel is in blocked state: Trigger sender, start timer to check after again after a while and abort operation.
+
+      // Case 1: Channel is in blocked state: Trigger sender, start timer to check after again after a while and abort operation.
       BITHockeyLogDebug(@"INFO: The channel is saturated. %@ was dropped.", item.debugDescription);
       if (![strongSelf timerIsRunning]) {
         [strongSelf startTimer];
       }
       return;
     }
-
-    // Enqueue item
-    NSDictionary *dict = [self dictionaryForTelemetryData:item];
-    [strongSelf appendDictionaryToJsonStream:dict];
-
-    if (strongSelf.dataItemCount >= self.maxBatchSize) {
-      // Case 3: Max batch count has been reached, so write queue to disk and delete all items.
-      [strongSelf persistDataItemQueue];
-
-    } else if (strongSelf.dataItemCount == 1) {
-      // Case 4: It is the first item, let's start the timer.
-      if (![strongSelf timerIsRunning]) {
-        [strongSelf startTimer];
+
+    // Enqueue item.
+    @synchronized(self) {
+      NSDictionary *dict = [strongSelf dictionaryForTelemetryData:item];
+      [strongSelf appendDictionaryToEventBuffer:dict];
+      if (strongSelf.dataItemCount >= strongSelf.maxBatchSize) {
+
+        // Case 2: Max batch count has been reached, so write queue to disk and delete all items.
+        [strongSelf persistDataItemQueue:&BITTelemetryEventBuffer];
+      } else if (strongSelf.dataItemCount > 0) {
+
+        // Case 3: It is the first item, let's start the timer.
+        if (![strongSelf timerIsRunning]) {
+          [strongSelf startTimer];
+        }
       }
     }
   });
@@ -169,39 +198,72 @@ - (NSString *)serializeDictionaryToJSONString:(NSDictionary *)dictionary {
 
 #pragma mark JSON Stream
 
-- (void)appendDictionaryToJsonStream:(NSDictionary *)dictionary {
+- (void)appendDictionaryToEventBuffer:(NSDictionary *)dictionary {
   if (dictionary) {
     NSString *string = [self serializeDictionaryToJSONString:dictionary];
-    
+
     // Since we can't persist every event right away, we write it to a simple C string.
     // This can then be written to disk by a signal handler in case of a crash.
-    bit_appendStringToSafeJsonStream(string, &(BITSafeJsonEventsString));
-    self.dataItemCount += 1;
+    @synchronized (self) {
+      bit_appendStringToEventBuffer(string, &BITTelemetryEventBuffer);
+      self.dataItemCount += 1;
+    }
   }
 }
 
-void bit_appendStringToSafeJsonStream(NSString *string, char **jsonString) {
-  if (jsonString == NULL) { return; }
-
-  if (!string) { return; }
-
-  if (*jsonString == NULL || strlen(*jsonString) == 0) {
-    bit_resetSafeJsonStream(jsonString);
+void bit_appendStringToEventBuffer(NSString *string, char **eventBuffer) {
+  if (eventBuffer == NULL) {
+    return;
   }
-
-  if (string.length == 0) { return; }
-
-  char *new_string = NULL;
-  // Concatenate old string with new JSON string and add a comma.
-  asprintf(&new_string, "%s%.*s\n", *jsonString, (int)MIN(string.length, (NSUInteger)INT_MAX), string.UTF8String);
-  free(*jsonString);
-  *jsonString = new_string;
+
+  if (!string) {
+    return;
+  }
+
+  if (*eventBuffer == NULL || strlen(*eventBuffer) == 0) {
+    bit_resetEventBuffer(eventBuffer);
+  }
+
+  if (string.length == 0) {
+    return;
+  }
+
+  do {
+    char *newBuffer = NULL;
+    char *previousBuffer = *eventBuffer;
+
+    // Concatenate old string with new JSON string and add a comma.
+    asprintf(&newBuffer, "%s%.*s\n", previousBuffer, (int)MIN(string.length, (NSUInteger)INT_MAX), string.UTF8String);
+
+    // Compare newBuffer and previousBuffer. If they point to the same address, we are safe to use them.
+    if (OSAtomicCompareAndSwapPtr(previousBuffer, newBuffer, (void*)eventBuffer)) {
+
+      // Free the intermediate pointer.
+      free(previousBuffer);
+      return;
+    } else {
+
+      // newBuffer has been changed by another thread.
+      free(newBuffer);
+    }
+  } while (true);
 }
 
-void bit_resetSafeJsonStream(char **string) {
-  if (!string) { return; }
-  free(*string);
-  *string = strdup("");
+void bit_resetEventBuffer(char **eventBuffer) {
+  if (!eventBuffer) { return; }
+
+  char *newEmptyString = NULL;
+  char *prevString = NULL;
+  do {
+    prevString = *eventBuffer;
+    newEmptyString = strdup("");
+
+    // Compare pointers to strings to make sure we are still threadsafe!
+    if (OSAtomicCompareAndSwapPtr(prevString, newEmptyString, (void*)eventBuffer)) {
+      free(prevString);
+      return;
+    }
+  } while(true);
 }
 
 #pragma mark - Batching
@@ -214,37 +276,43 @@ - (NSUInteger)maxBatchSize {
 }
 
 - (void)invalidateTimer {
-  if ([self timerIsRunning]) {
-    dispatch_source_cancel((dispatch_source_t)self.timerSource);
-    self.timerSource = nil;
+  @synchronized(self) {
+    if (self.timerSource != nil) {
+      dispatch_source_cancel((dispatch_source_t)self.timerSource);
+      self.timerSource = nil;
+    }
   }
 }
 
 -(BOOL)timerIsRunning {
-  return self.timerSource != nil;
+  @synchronized(self) {
+    return self.timerSource != nil;
+  }
 }
 
 - (void)startTimer {
-  // Reset timer, if it is already running
-  if ([self timerIsRunning]) {
+  @synchronized(self) {
+
+    // Reset timer, if it is already running.
     [self invalidateTimer];
-  }
-
-  self.timerSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, self.dataItemsOperations);
-  dispatch_source_set_timer((dispatch_source_t)self.timerSource, dispatch_walltime(NULL, NSEC_PER_SEC * self.batchInterval), 1ull * NSEC_PER_SEC, 1ull * NSEC_PER_SEC);
-  __weak typeof(self) weakSelf = self;
-  dispatch_source_set_event_handler((dispatch_source_t)self.timerSource, ^{
-    typeof(self) strongSelf = weakSelf;
-    if (strongSelf) {
-      if (strongSelf.dataItemCount > 0) {
-        [strongSelf persistDataItemQueue];
-      } else {
-        strongSelf.channelBlocked = NO;
+
+    dispatch_source_t timerSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, self.dataItemsOperations);
+    dispatch_source_set_timer(timerSource, dispatch_walltime(NULL, NSEC_PER_SEC * self.batchInterval), 1ull * NSEC_PER_SEC, 1ull * NSEC_PER_SEC);
+    __weak typeof(self) weakSelf = self;
+    dispatch_source_set_event_handler(timerSource, ^{
+      typeof(self) strongSelf = weakSelf;
+      if (strongSelf) {
+        if (strongSelf.dataItemCount > 0) {
+          [strongSelf persistDataItemQueue:&BITTelemetryEventBuffer];
+        } else {
+          strongSelf.channelBlocked = NO;
+        }
+        [strongSelf invalidateTimer];
       }
-      [strongSelf invalidateTimer];
-    }
-  });
-  dispatch_resume((dispatch_source_t)self.timerSource);
+    });
+    dispatch_resume(timerSource);
+    self.timerSource = timerSource;
+  }
 }
 
 /**

Classes/Telemetry/BITChannelPrivate.h

@@ -67,29 +67,28 @@ FOUNDATION_EXPORT NSString *const BITChannelBlockedNotification;
 /**
  *  Manually trigger the BITChannel to persist all items currently in its data item queue.
  */
-- (void)persistDataItemQueue;
+- (void)persistDataItemQueue:(char *_Nullable*_Nullable)eventBuffer;
 
 /**
  *  Adds the specified dictionary to the JSON Stream string.
  *
  *  @param dictionary the dictionary object which is to be added to the JSON Stream queue string.
  */
-- (void)appendDictionaryToJsonStream:(NSDictionary *)dictionary;
+- (void)appendDictionaryToEventBuffer:(NSDictionary *)dictionary;
 
 /**
  *  A C function that serializes a given dictionary to JSON and appends it to a char string
  *
- *  @param string A string which will be appended to the string.
- *  @param jsonStream The C string which the dictionary's JSON representation will be appended to.
+ *  @param string The C string which the dictionary's JSON representation will be appended to.
  */
-void bit_appendStringToSafeJsonStream(NSString *string, char *__nonnull*__nonnull jsonStream);
+void bit_appendStringToEventBuffer(NSString *string, char *__nonnull*__nonnull eventBuffer);
 
 /**
- *  Reset BITSafeJsonEventsString so we can start appending JSON dictionaries.
+ *  Reset the event buffer so we can start appending JSON dictionaries.
  *
- *  @param jsonStream The string that will be reset.
+ *  @param eventBuffer The string that will be reset.
  */
-void bit_resetSafeJsonStream(char *__nonnull*__nonnull jsonStream);
+void bit_resetEventBuffer(char *__nonnull*__nonnull eventBuffer);
 
 /**
  *  A method which indicates whether the telemetry pipeline is busy and no new data should be enqueued.