release-6.6.0 fix #2 for GDTUploadCoordinator crashes (#3557)

* Make ivar references safer (#3555) Fixes 3547, in theory. This code all goes away/changes with the sqlite refactor, so making it safe is probably sufficient for now. No one was able to reproduce locally, most likely because this is a lifecycle issue. * Increment GDT to v1.1.1 GDTCCT has a dep on ~> 1.1, so this incremented version should get picked up. * Actually use the strongified variables I made
firebase · Aug 12, 2019 · 518a44e · 518a44e
1 parent cf9228a
commit 518a44e
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 19 deletions.
diff --git a/GoogleDataTransport.podspec b/GoogleDataTransport.podspec
@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name             = 'GoogleDataTransport'
-  s.version          = '1.1.0'
+  s.version          = '1.1.1'
   s.summary          = 'Google iOS SDK data transport.'
 
   s.description      = <<-DESC

diff --git a/GoogleDataTransport/GDTLibrary/GDTStorage.m b/GoogleDataTransport/GDTLibrary/GDTStorage.m
@@ -246,10 +246,24 @@ - (instancetype)initWithCoder:(NSCoder *)aDecoder {
 
 - (void)encodeWithCoder:(NSCoder *)aCoder {
   GDTStorage *sharedInstance = [self.class sharedInstance];
-  dispatch_sync(sharedInstance.storageQueue, ^{
-    [aCoder encodeObject:sharedInstance->_storedEvents forKey:kGDTStorageStoredEventsKey];
-    [aCoder encodeObject:sharedInstance->_targetToEventSet forKey:kGDTStorageTargetToEventSetKey];
-    [aCoder encodeObject:sharedInstance->_uploadCoordinator forKey:kGDTStorageUploadCoordinatorKey];
+  dispatch_queue_t storageQueue = sharedInstance.storageQueue;
+  if (!storageQueue) {
+    return;
+  }
+  dispatch_sync(storageQueue, ^{
+    NSMutableOrderedSet<GDTStoredEvent *> *storedEvents = sharedInstance->_storedEvents;
+    if (storedEvents) {
+      [aCoder encodeObject:storedEvents forKey:kGDTStorageStoredEventsKey];
+    }
+    NSMutableDictionary<NSNumber *, NSMutableSet<GDTStoredEvent *> *> *targetToEventSet =
+        sharedInstance->_targetToEventSet;
+    if (targetToEventSet) {
+      [aCoder encodeObject:targetToEventSet forKey:kGDTStorageTargetToEventSetKey];
+    }
+    GDTUploadCoordinator *uploadCoordinator = sharedInstance->_uploadCoordinator;
+    if (uploadCoordinator) {
+      [aCoder encodeObject:uploadCoordinator forKey:kGDTStorageUploadCoordinatorKey];
+    }
   });
 }
 

diff --git a/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m b/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m
@@ -208,32 +208,52 @@ - (void)appWillTerminate:(GDTApplication *)application {
 #pragma mark - GDTUploadPackageProtocol
 
 - (void)packageDelivered:(GDTUploadPackage *)package successful:(BOOL)successful {
+  if (!_coordinationQueue) {
+    return;
+  }
   dispatch_async(_coordinationQueue, ^{
     NSNumber *targetNumber = @(package.target);
-    [self->_targetToInFlightPackages removeObjectForKey:targetNumber];
-    id<GDTPrioritizer> prioritizer = self->_registrar.targetToPrioritizer[targetNumber];
-    if (!prioritizer) {
-      GDTLogError(GDTMCEPrioritizerError, @"A prioritizer should be registered for this target: %@",
-                  targetNumber);
+    NSMutableDictionary<NSNumber *, GDTUploadPackage *> *targetToInFlightPackages =
+        self->_targetToInFlightPackages;
+    GDTRegistrar *registrar = self->_registrar;
+    if (targetToInFlightPackages) {
+      [targetToInFlightPackages removeObjectForKey:targetNumber];
     }
-    if ([prioritizer respondsToSelector:@selector(packageDelivered:successful:)]) {
-      [prioritizer packageDelivered:package successful:successful];
+    if (registrar) {
+      id<GDTPrioritizer> prioritizer = registrar.targetToPrioritizer[targetNumber];
+      if (!prioritizer) {
+        GDTLogError(GDTMCEPrioritizerError,
+                    @"A prioritizer should be registered for this target: %@", targetNumber);
+      }
+      if ([prioritizer respondsToSelector:@selector(packageDelivered:successful:)]) {
+        [prioritizer packageDelivered:package successful:successful];
+      }
     }
     [self.storage removeEvents:package.events];
   });
 }
 
 - (void)packageExpired:(GDTUploadPackage *)package {
+  if (!_coordinationQueue) {
+    return;
+  }
   dispatch_async(_coordinationQueue, ^{
     NSNumber *targetNumber = @(package.target);
-    [self->_targetToInFlightPackages removeObjectForKey:targetNumber];
-    id<GDTPrioritizer> prioritizer = self->_registrar.targetToPrioritizer[targetNumber];
-    id<GDTUploader> uploader = self->_registrar.targetToUploader[targetNumber];
-    if ([prioritizer respondsToSelector:@selector(packageExpired:)]) {
-      [prioritizer packageExpired:package];
+    NSMutableDictionary<NSNumber *, GDTUploadPackage *> *targetToInFlightPackages =
+        self->_targetToInFlightPackages;
+    GDTRegistrar *registrar = self->_registrar;
+    if (targetToInFlightPackages) {
+      [targetToInFlightPackages removeObjectForKey:targetNumber];
     }
-    if ([uploader respondsToSelector:@selector(packageExpired:)]) {
-      [uploader packageExpired:package];
+    if (registrar) {
+      id<GDTPrioritizer> prioritizer = registrar.targetToPrioritizer[targetNumber];
+      id<GDTUploader> uploader = registrar.targetToUploader[targetNumber];
+      if ([prioritizer respondsToSelector:@selector(packageExpired:)]) {
+        [prioritizer packageExpired:package];
+      }
+      if ([uploader respondsToSelector:@selector(packageExpired:)]) {
+        [uploader packageExpired:package];
+      }
     }
   });
 }