Browse Source

Move the dispatch of gauge metric calls to its dedicated thread asynchronously (#7601)

Visu 5 years ago
parent
commit
885ddd921a

+ 1 - 0
FirebasePerformance/CHANGELOG.md

@@ -2,6 +2,7 @@
 * Deprecate Clearcut event transport mechanism.
 * Enable dynamic framework support. (#7569)
 * Remove the warning to include Firebase Analytics as Perf does not depend on Analytics (#7487)
+* Fix the crash on gauge manager due to race condition. (#7535)
 
 # Version 7.7.0
 * Add community supported tvOS.

+ 3 - 1
FirebasePerformance/Sources/Gauges/FPRGaugeManager+Private.h

@@ -42,7 +42,9 @@
 
 /**
  * Prepares for dispatching the current set of gauge data to Google Data Transport.
+ *
+ * @param sessionId SessionId that will be used for dispatching the gauge data
  */
-- (void)prepareAndDispatchGaugeData;
+- (void)prepareAndDispatchCollectedGaugeDataWithSessionId:(nullable NSString *)sessionId;
 
 @end

+ 22 - 18
FirebasePerformance/Sources/Gauges/FPRGaugeManager.m

@@ -37,7 +37,7 @@ NSInteger const kGaugeDataBatchSize = 25;
 @property(nonatomic) NSMutableArray *gaugeData;
 
 /** @brief Currently active sessionID. */
-@property(nonatomic, readwrite) NSString *currentSessionId;
+@property(nonatomic, readwrite, copy) NSString *currentSessionId;
 
 @end
 
@@ -117,7 +117,8 @@ NSInteger const kGaugeDataBatchSize = 25;
 }
 
 - (void)startCollectingGauges:(FPRGauges)gauges forSessionId:(NSString *)sessionId {
-  [self prepareAndDispatchGaugeData];
+  // Dispatch the already available gauge data with old sessionId.
+  [self prepareAndDispatchCollectedGaugeDataWithSessionId:self.currentSessionId];
 
   self.currentSessionId = sessionId;
   if (self.gaugeCollectionEnabled) {
@@ -143,7 +144,8 @@ NSInteger const kGaugeDataBatchSize = 25;
 
   self.activeGauges = self.activeGauges & ~(gauges);
 
-  [self prepareAndDispatchGaugeData];
+  // Flush out all the already collected gauge metrics
+  [self prepareAndDispatchCollectedGaugeDataWithSessionId:self.currentSessionId];
 }
 
 - (void)collectAllGauges {
@@ -172,31 +174,33 @@ NSInteger const kGaugeDataBatchSize = 25;
 
 #pragma mark - Utils
 
-- (void)prepareAndDispatchGaugeData {
-  NSArray *currentBatch = self.gaugeData;
-  NSString *currentSessionId = self.currentSessionId;
-  self.gaugeData = [[NSMutableArray alloc] init];
-  dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-    if (currentBatch.count > 0) {
-      [[FPRClient sharedInstance] logGaugeMetric:currentBatch forSessionId:currentSessionId];
-      FPRLogDebug(kFPRGaugeManagerDataCollected, @"Logging %lu gauge metrics.",
-                  (unsigned long)currentBatch.count);
-    }
+- (void)prepareAndDispatchCollectedGaugeDataWithSessionId:(nullable NSString *)sessionId {
+  dispatch_async(self.gaugeDataProtectionQueue, ^{
+    NSArray *dispatchGauges = [self.gaugeData copy];
+    self.gaugeData = [[NSMutableArray alloc] init];
+
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+      if (dispatchGauges.count > 0 && sessionId != nil) {
+        [[FPRClient sharedInstance] logGaugeMetric:dispatchGauges forSessionId:sessionId];
+        FPRLogDebug(kFPRGaugeManagerDataCollected, @"Logging %lu gauge metrics.",
+                    (unsigned long)dispatchGauges.count);
+      }
+    });
   });
 }
 
 /**
  * Adds the gauge to the batch and decide on when to dispatch the events to Google Data Transport.
  *
- * @param gaugeData Gauge data received from the collectors.
+ * @param gauge Gauge data received from the collectors.
  */
-- (void)addGaugeData:(id)gaugeData {
+- (void)addGaugeData:(id)gauge {
   dispatch_async(self.gaugeDataProtectionQueue, ^{
-    if (gaugeData) {
-      [self.gaugeData addObject:gaugeData];
+    if (gauge) {
+      [self.gaugeData addObject:gauge];
 
       if (self.gaugeData.count >= kGaugeDataBatchSize) {
-        [self prepareAndDispatchGaugeData];
+        [self prepareAndDispatchCollectedGaugeDataWithSessionId:self.currentSessionId];
       }
     }
   });

+ 2 - 2
FirebasePerformance/Tests/Unit/Gauges/FPRGaugeManagerTests.m

@@ -154,7 +154,7 @@
 - (void)testBatchingOfGaugeEvents {
   FPRGaugeManager *manager = [FPRGaugeManager sharedInstance];
   id mock = [OCMockObject partialMockForObject:manager];
-  OCMExpect([mock prepareAndDispatchGaugeData]).andDo(nil);
+  OCMExpect([mock prepareAndDispatchCollectedGaugeDataWithSessionId:@"abc"]).andDo(nil);
   [manager startCollectingGauges:FPRGaugeCPU forSessionId:@"abc"];
   [manager.cpuGaugeCollector stopCollecting];
   for (int i = 0; i < kGaugeDataBatchSize; i++) {
@@ -172,7 +172,7 @@
   id mock = [OCMockObject partialMockForObject:manager];
   [manager startCollectingGauges:FPRGaugeCPU forSessionId:@"abc"];
   [manager.cpuGaugeCollector stopCollecting];
-  OCMReject([mock prepareAndDispatchGaugeData]);
+  OCMReject([mock prepareAndDispatchCollectedGaugeDataWithSessionId:@"abc"]);
   for (int i = 0; i < kGaugeDataBatchSize - 1; i++) {
     [manager.cpuGaugeCollector collectMetric];
   }