From 16acdfb4d1d2a800ca07cb85c83e898d7521162d Mon Sep 17 00:00:00 2001
From: Michael Haney <haneym@google.com>
Date: Mon, 12 Aug 2019 11:19:53 -0700
Subject: [PATCH] Make ivar references safer

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.
---
 GoogleDataTransport/GDTLibrary/GDTStorage.m   | 23 ++++++--
 .../GDTLibrary/GDTUploadCoordinator.m         | 54 +++++++++++++------
 2 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/GoogleDataTransport/GDTLibrary/GDTStorage.m b/GoogleDataTransport/GDTLibrary/GDTStorage.m
index ea73f1aebb4..22a202f609c 100644
--- a/GoogleDataTransport/GDTLibrary/GDTStorage.m
+++ b/GoogleDataTransport/GDTLibrary/GDTStorage.m
@@ -246,10 +246,25 @@ - (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:sharedInstance->_targetToEventSet forKey:kGDTStorageTargetToEventSetKey];
+    }
+    GDTUploadCoordinator *uploadCoordinator = sharedInstance->_uploadCoordinator;
+    if (uploadCoordinator) {
+      [aCoder encodeObject:sharedInstance->_uploadCoordinator
+                    forKey:kGDTStorageUploadCoordinatorKey];
+    }
   });
 }
 
diff --git a/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m b/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m
index afa1e09c70c..f61be15aa43 100644
--- a/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m
+++ b/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m
@@ -208,32 +208,54 @@ - (void)appWillTerminate:(GDTApplication *)application {
 #pragma mark - GDTUploadPackageProtocol
 
 - (void)packageDelivered:(GDTUploadPackage *)package successful:(BOOL)successful {
-  dispatch_async(_coordinationQueue, ^{
+  dispatch_queue_t coordinationQueue = _coordinationQueue;
+  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 {
-  dispatch_async(_coordinationQueue, ^{
+  dispatch_queue_t coordinationQueue = _coordinationQueue;
+  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];
+      }
     }
   });
 }