Kaynağa Gözat

Fix strict concurrency checking errors (#10518)

Paul Beusterien 3 yıl önce
ebeveyn
işleme
b109e01e2f

+ 19 - 19
FirebaseStorage/Sources/Internal/StorageDeleteTask.swift

@@ -44,34 +44,34 @@ internal class StorageDeleteTask: StorageTask, StorageTaskManagement {
    * Prepares a task and begins execution.
    */
   internal func enqueue() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      guard let strongSelf = weakSelf else { return }
-      strongSelf.state = .queueing
-      var request = strongSelf.baseRequest
+    let completion = taskCompletion
+    taskCompletion = { (error: Error?) in
+      completion?(error)
+      // Reference self in completion handler in order to retain self until completion is called.
+      self.taskCompletion = nil
+    }
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      self.state = .queueing
+      var request = self.baseRequest
       request.httpMethod = "DELETE"
-      request.timeoutInterval = strongSelf.reference.storage.maxOperationRetryTime
-
-      let callback = strongSelf.taskCompletion
-      strongSelf.taskCompletion = nil
+      request.timeoutInterval = self.reference.storage.maxOperationRetryTime
 
-      let fetcher = strongSelf.fetcherService.fetcher(with: request)
+      let fetcher = self.fetcherService.fetcher(with: request)
       fetcher.comment = "DeleteTask"
-      strongSelf.fetcher = fetcher
+      self.fetcher = fetcher
 
-      strongSelf.fetcherCompletion = { (data: Data?, error: NSError?) in
+      self.fetcherCompletion = { [weak self] (data: Data?, error: NSError?) in
+        guard let self = self else { return }
         if let error = error, self.error == nil {
-          self.error = StorageErrorCode.error(withServerError: error, ref: strongSelf.reference)
+          self.error = StorageErrorCode.error(withServerError: error, ref: self.reference)
         }
-        callback?(self.error)
+        self.taskCompletion?(self.error)
         self.fetcherCompletion = nil
       }
 
-      strongSelf.fetcher?.beginFetch { data, error in
-        let strongSelf = weakSelf
-        if let fetcherCompletion = strongSelf?.fetcherCompletion {
-          fetcherCompletion(data, error as? NSError)
-        }
+      self.fetcher?.beginFetch { [weak self] data, error in
+        self?.fetcherCompletion?(data, error as? NSError)
       }
     }
   }

+ 19 - 18
FirebaseStorage/Sources/Internal/StorageGetDownloadURLTask.swift

@@ -44,21 +44,25 @@ internal class StorageGetDownloadURLTask: StorageTask, StorageTaskManagement {
    * Prepares a task and begins execution.
    */
   internal func enqueue() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      guard let strongSelf = weakSelf else { return }
-      var request = strongSelf.baseRequest
+    if let completion = taskCompletion {
+      taskCompletion = { (url: URL?, error: Error?) in
+        completion(url, error)
+        // Reference self in completion handler in order to retain self until completion is called.
+        self.taskCompletion = nil
+      }
+    }
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      var request = self.baseRequest
       request.httpMethod = "GET"
-      request.timeoutInterval = strongSelf.reference.storage.maxOperationRetryTime
+      request.timeoutInterval = self.reference.storage.maxOperationRetryTime
 
-      let callback = strongSelf.taskCompletion
-      strongSelf.taskCompletion = nil
-
-      let fetcher = strongSelf.fetcherService.fetcher(with: request)
+      let fetcher = self.fetcherService.fetcher(with: request)
       fetcher.comment = "GetDownloadURLTask"
-      strongSelf.fetcher = fetcher
+      self.fetcher = fetcher
 
-      strongSelf.fetcherCompletion = { (data: Data?, error: NSError?) in
+      self.fetcherCompletion = { [weak self] (data: Data?, error: NSError?) in
+        guard let self = self else { return }
         var downloadURL: URL?
         if let error = error {
           if self.error == nil {
@@ -68,7 +72,7 @@ internal class StorageGetDownloadURLTask: StorageTask, StorageTaskManagement {
           if let data = data,
              let responseDictionary = try? JSONSerialization
              .jsonObject(with: data) as? [String: Any] {
-            downloadURL = strongSelf.downloadURLFromMetadataDictionary(responseDictionary)
+            downloadURL = self.downloadURLFromMetadataDictionary(responseDictionary)
             if downloadURL == nil {
               self.error = NSError(domain: StorageErrorDomain,
                                    code: StorageErrorCode.unknown.rawValue,
@@ -79,15 +83,12 @@ internal class StorageGetDownloadURLTask: StorageTask, StorageTaskManagement {
             self.error = StorageErrorCode.error(withInvalidRequest: data)
           }
         }
-        callback?(downloadURL, self.error)
+        self.taskCompletion?(downloadURL, self.error)
         self.fetcherCompletion = nil
       }
 
-      strongSelf.fetcher?.beginFetch { data, error in
-        let strongSelf = weakSelf
-        if let fetcherCompletion = strongSelf?.fetcherCompletion {
-          fetcherCompletion(data, error as? NSError)
-        }
+      self.fetcher?.beginFetch { [weak self] data, error in
+        self?.fetcherCompletion?(data, error as? NSError)
       }
     }
   }

+ 19 - 18
FirebaseStorage/Sources/Internal/StorageGetMetadataTask.swift

@@ -44,22 +44,26 @@ internal class StorageGetMetadataTask: StorageTask, StorageTaskManagement {
    * Prepares a task and begins execution.
    */
   internal func enqueue() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      guard let strongSelf = weakSelf else { return }
-      strongSelf.state = .queueing
-      var request = strongSelf.baseRequest
+    if let completion = taskCompletion {
+      taskCompletion = { (metadata: StorageMetadata?, error: Error?) in
+        completion(metadata, error)
+        // Reference self in completion handler in order to retain self until completion is called.
+        self.taskCompletion = nil
+      }
+    }
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      self.state = .queueing
+      var request = self.baseRequest
       request.httpMethod = "GET"
-      request.timeoutInterval = strongSelf.reference.storage.maxOperationRetryTime
+      request.timeoutInterval = self.reference.storage.maxOperationRetryTime
 
-      let callback = strongSelf.taskCompletion
-      strongSelf.taskCompletion = nil
-
-      let fetcher = strongSelf.fetcherService.fetcher(with: request)
+      let fetcher = self.fetcherService.fetcher(with: request)
       fetcher.comment = "GetMetadataTask"
-      strongSelf.fetcher = fetcher
+      self.fetcher = fetcher
 
-      strongSelf.fetcherCompletion = { (data: Data?, error: NSError?) in
+      self.fetcherCompletion = { [weak self] (data: Data?, error: NSError?) in
+        guard let self = self else { return }
         var metadata: StorageMetadata?
         if let error = error {
           if self.error == nil {
@@ -75,15 +79,12 @@ internal class StorageGetMetadataTask: StorageTask, StorageTaskManagement {
             self.error = StorageErrorCode.error(withInvalidRequest: data)
           }
         }
-        callback?(metadata, self.error)
+        self.taskCompletion?(metadata, self.error)
         self.fetcherCompletion = nil
       }
 
-      strongSelf.fetcher?.beginFetch { data, error in
-        let strongSelf = weakSelf
-        if let fetcherCompletion = strongSelf?.fetcherCompletion {
-          fetcherCompletion(data, error as? NSError)
-        }
+      self.fetcher?.beginFetch { [weak self] data, error in
+        self?.fetcherCompletion?(data, error as? NSError)
       }
     }
   }

+ 27 - 22
FirebaseStorage/Sources/Internal/StorageListTask.swift

@@ -52,7 +52,11 @@ internal class StorageListTask: StorageTask, StorageTaskManagement {
     self.pageSize = pageSize
     self.previousPageToken = previousPageToken
     super.init(reference: reference, service: fetcherService, queue: queue)
-    taskCompletion = completion
+    taskCompletion = { (listResult: StorageListResult?, error: NSError?) in
+      completion?(listResult, error)
+      // Reference self in completion handler in order to retain self until completion is called.
+      self.taskCompletion = nil
+    }
   }
 
   deinit {
@@ -63,12 +67,18 @@ internal class StorageListTask: StorageTask, StorageTaskManagement {
    * Prepares a task and begins execution.
    */
   internal func enqueue() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      guard let strongSelf = weakSelf else { return }
+    if let completion = taskCompletion {
+      taskCompletion = { (listResult: StorageListResult?, error: NSError?) in
+        completion(listResult, error)
+        // Reference self in completion handler in order to retain self until completion is called.
+        self.taskCompletion = nil
+      }
+    }
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
       var queryParams = [String: String]()
 
-      let prefix = strongSelf.reference.fullPath
+      let prefix = self.reference.fullPath
       if prefix.count > 0 {
         queryParams["prefix"] = "\(prefix)/"
       }
@@ -80,34 +90,32 @@ internal class StorageListTask: StorageTask, StorageTaskManagement {
       // listAll() doesn't set a pageSize as this allows Firebase Storage to determine how many items
       // to return per page. This removes the need to backfill results if Firebase Storage filters
       // objects that are considered invalid (such as items with two consecutive slashes).
-      if let pageSize = strongSelf.pageSize {
+      if let pageSize = self.pageSize {
         queryParams["maxResults"] = "\(pageSize)"
       }
 
-      if let previousPageToken = strongSelf.previousPageToken {
+      if let previousPageToken = self.previousPageToken {
         queryParams["pageToken"] = previousPageToken
       }
 
-      let root = strongSelf.reference.root()
+      let root = self.reference.root()
       var request = StorageUtils.defaultRequestForReference(
         reference: root,
         queryParams: queryParams
       )
 
       request.httpMethod = "GET"
-      request.timeoutInterval = strongSelf.reference.storage.maxOperationRetryTime
+      request.timeoutInterval = self.reference.storage.maxOperationRetryTime
 
-      let callback = strongSelf.taskCompletion
-      strongSelf.taskCompletion = nil
-
-      let fetcher = strongSelf.fetcherService.fetcher(with: request)
+      let fetcher = self.fetcherService.fetcher(with: request)
       fetcher.comment = "ListTask"
-      strongSelf.fetcher = fetcher
+      self.fetcher = fetcher
 
-      strongSelf.fetcherCompletion = { (data: Data?, error: NSError?) in
+      self.fetcherCompletion = { [weak self] (data: Data?, error: NSError?) in
+        guard let self = self else { return }
         var listResult: StorageListResult?
         if let error = error, self.error == nil {
-          self.error = StorageErrorCode.error(withServerError: error, ref: strongSelf.reference)
+          self.error = StorageErrorCode.error(withServerError: error, ref: self.reference)
         } else {
           if let data = data,
              let responseDictionary = try? JSONSerialization
@@ -118,15 +126,12 @@ internal class StorageListTask: StorageTask, StorageTaskManagement {
           }
         }
 
-        callback?(listResult, self.error)
+        self.taskCompletion?(listResult, self.error)
         self.fetcherCompletion = nil
       }
 
-      strongSelf.fetcher?.beginFetch { data, error in
-        let strongSelf = weakSelf
-        if let fetcherCompletion = strongSelf?.fetcherCompletion {
-          fetcherCompletion(data, error as? NSError)
-        }
+      self.fetcher?.beginFetch { [weak self] data, error in
+        self?.fetcherCompletion?(data, error as? NSError)
       }
     }
   }

+ 18 - 18
FirebaseStorage/Sources/Internal/StorageUpdateMetadataTask.swift

@@ -47,28 +47,31 @@ internal class StorageUpdateMetadataTask: StorageTask, StorageTaskManagement {
    * Prepares a task and begins execution.
    */
   internal func enqueue() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      guard let strongSelf = weakSelf else { return }
-      var request = strongSelf.baseRequest
-      let updateDictionary = strongSelf.updateMetadata.updatedMetadata()
+    let completion = taskCompletion
+    taskCompletion = { (metadata: StorageMetadata?, error: Error?) in
+      completion?(metadata, error)
+      // Reference self in completion handler in order to retain self until completion is called.
+      self.taskCompletion = nil
+    }
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      var request = self.baseRequest
+      let updateDictionary = self.updateMetadata.updatedMetadata()
       let updateData = try? JSONSerialization.data(withJSONObject: updateDictionary)
       request.httpMethod = "PATCH"
-      request.timeoutInterval = strongSelf.reference.storage.maxOperationRetryTime
+      request.timeoutInterval = self.reference.storage.maxOperationRetryTime
       request.httpBody = updateData
       request.setValue("application/json; charset=UTF-8", forHTTPHeaderField: "Content-Type")
       if let count = updateData?.count {
         request.setValue("\(count)", forHTTPHeaderField: "Content-Length")
       }
 
-      let callback = strongSelf.taskCompletion
-      strongSelf.taskCompletion = nil
-
-      let fetcher = strongSelf.fetcherService.fetcher(with: request)
+      let fetcher = self.fetcherService.fetcher(with: request)
       fetcher.comment = "GetMetadataTask"
-      strongSelf.fetcher = fetcher
+      self.fetcher = fetcher
 
-      strongSelf.fetcherCompletion = { (data: Data?, error: NSError?) in
+      self.fetcherCompletion = { [weak self] (data: Data?, error: NSError?) in
+        guard let self = self else { return }
         var metadata: StorageMetadata?
         if let error = error {
           if self.error == nil {
@@ -84,17 +87,14 @@ internal class StorageUpdateMetadataTask: StorageTask, StorageTaskManagement {
             self.error = StorageErrorCode.error(withInvalidRequest: data)
           }
         }
-        callback?(metadata, self.error)
+        self.taskCompletion?(metadata, self.error)
         self.fetcherCompletion = nil
       }
 
       fetcher.comment = "UpdateMetadataTask"
 
-      strongSelf.fetcher?.beginFetch { data, error in
-        let strongSelf = weakSelf
-        if let fetcherCompletion = strongSelf?.fetcherCompletion {
-          fetcherCompletion(data, error as? NSError)
-        }
+      self.fetcher?.beginFetch { [weak self] data, error in
+        self?.fetcherCompletion?(data, error as? NSError)
       }
     }
   }

+ 63 - 68
FirebaseStorage/Sources/StorageDownloadTask.swift

@@ -43,24 +43,21 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement {
    * Pauses a task currently in progress. Calling this on a paused task has no effect.
    */
   @objc open func pause() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      guard let strongSelf = weakSelf else { return }
-      if strongSelf.state == .paused || strongSelf.state == .pausing {
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      if self.state == .paused || self.state == .pausing {
         return
       }
-      strongSelf.state = .pausing
+      self.state = .pausing
       // Use the resume callback to confirm pause status since it always runs after the last
       // NSURLSession update.
-      strongSelf.fetcher?.resumeDataBlock = { (data: Data) in
-        let strongerSelf = weakSelf
-        strongerSelf?.downloadData = data
-        strongerSelf?.state = .paused
-        if let snapshot = strongerSelf?.snapshot {
-          strongerSelf?.fire(for: .pause, snapshot: snapshot)
-        }
+      self.fetcher?.resumeDataBlock = { [weak self] (data: Data) in
+        guard let self = self else { return }
+        self.downloadData = data
+        self.state = .paused
+        self.fire(for: .pause, snapshot: self.snapshot)
       }
-      strongSelf.fetcher?.stopFetching()
+      self.fetcher?.stopFetching()
     }
   }
 
@@ -76,20 +73,21 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement {
    * Resumes a paused task. Calling this on a running task has no effect.
    */
   @objc open func resume() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      weakSelf?.state = .resuming
-      if let snapshot = weakSelf?.snapshot {
-        weakSelf?.fire(for: .resume, snapshot: snapshot)
-      }
-      weakSelf?.state = .running
-      weakSelf?.enqueueImplementation(resumeWith: self.downloadData)
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      self.state = .resuming
+      self.fire(for: .resume, snapshot: self.snapshot)
+      self.state = .running
+      self.enqueueImplementation(resumeWith: self.downloadData)
     }
   }
 
   private var fetcher: GTMSessionFetcher?
   private var fetcherCompletion: ((Data?, NSError?) -> Void)?
   internal var downloadData: Data?
+  // Hold completion in object to force it to be retained until completion block is called.
+  internal var completionData: ((Data?, Error?) -> Void)?
+  internal var completionURL: ((URL?, Error?) -> Void)?
 
   // MARK: - Internal Implementations
 
@@ -105,13 +103,12 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement {
   }
 
   internal func enqueueImplementation(resumeWith resumeData: Data? = nil) {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      guard let strongSelf = weakSelf else { return }
-      strongSelf.state = .queueing
-      var request = strongSelf.baseRequest
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      self.state = .queueing
+      var request = self.baseRequest
       request.httpMethod = "GET"
-      request.timeoutInterval = strongSelf.reference.storage.maxDownloadRetryTime
+      request.timeoutInterval = self.reference.storage.maxDownloadRetryTime
       var components = URLComponents(url: request.url!, resolvingAgainstBaseURL: false)
       components?.query = "alt=media"
       request.url = components?.url
@@ -121,41 +118,46 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement {
         fetcher = GTMSessionFetcher(downloadResumeData: resumeData)
         fetcher.comment = "Resuming DownloadTask"
       } else {
-        fetcher = strongSelf.fetcherService.fetcher(with: request)
+        fetcher = self.fetcherService.fetcher(with: request)
         fetcher.comment = "Resuming DownloadTask"
       }
-      fetcher.maxRetryInterval = strongSelf.reference.storage.maxDownloadRetryInterval
+      fetcher.maxRetryInterval = self.reference.storage.maxDownloadRetryInterval
 
-      if let fileURL = strongSelf.fileURL {
+      if let fileURL = self.fileURL {
         // Handle file downloads
         fetcher.destinationFileURL = fileURL
-        fetcher.downloadProgressBlock = { (bytesWritten: Int64,
-                                           totalBytesWritten: Int64,
-                                           totalBytesExpectedToWrite: Int64) in
-            weakSelf?.state = .progress
-            weakSelf?.progress.completedUnitCount = totalBytesWritten
-            weakSelf?.progress.totalUnitCount = totalBytesExpectedToWrite
-            if let snapshot = weakSelf?.snapshot {
-              weakSelf?.fire(for: .progress, snapshot: snapshot)
-            }
-            weakSelf?.state = .running
+        fetcher.downloadProgressBlock = { [weak self] (bytesWritten: Int64,
+                                                       totalBytesWritten: Int64,
+                                                       totalBytesExpectedToWrite: Int64) in
+            guard let self = self else { return }
+            self.state = .progress
+            self.progress.completedUnitCount = totalBytesWritten
+            self.progress.totalUnitCount = totalBytesExpectedToWrite
+            self.fire(for: .progress, snapshot: self.snapshot)
+            self.state = .running
         }
       } else {
         // Handle data downloads
-        fetcher.receivedProgressBlock = { (bytesWritten: Int64, totalBytesWritten: Int64) in
-          weakSelf?.state = .progress
-          weakSelf?.progress.completedUnitCount = totalBytesWritten
-          if let totalLength = weakSelf?.fetcher?.response?.expectedContentLength {
-            weakSelf?.progress.totalUnitCount = totalLength
-          }
-          if let snapshot = weakSelf?.snapshot {
-            weakSelf?.fire(for: .progress, snapshot: snapshot)
-          }
-          weakSelf?.state = .running
+        fetcher.receivedProgressBlock = { [weak self] (bytesWritten: Int64,
+                                                       totalBytesWritten: Int64) in
+            guard let self = self else { return }
+            self.state = .progress
+            self.progress.completedUnitCount = totalBytesWritten
+            if let totalLength = self.fetcher?.response?.expectedContentLength {
+              self.progress.totalUnitCount = totalLength
+            }
+            self.fire(for: .progress, snapshot: self.snapshot)
+            self.state = .running
         }
       }
-      strongSelf.fetcher = fetcher
-      strongSelf.fetcherCompletion = { (data: Data?, error: NSError?) in
+      self.fetcher = fetcher
+
+      // Capture self here to retain until completion.
+      self.fetcherCompletion = { [self] (data: Data?, error: NSError?) in
+        defer {
+          self.removeAllObservers()
+          self.fetcherCompletion = nil
+        }
         self.fire(for: .progress, snapshot: self.snapshot)
 
         // Handle potential issues with download
@@ -163,8 +165,6 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement {
           self.state = .failed
           self.error = StorageErrorCode.error(withServerError: error, ref: self.reference)
           self.fire(for: .failure, snapshot: self.snapshot)
-          self.removeAllObservers()
-          self.fetcherCompletion = nil
           return
         }
         // Download completed successfully, fire completion callbacks
@@ -173,26 +173,21 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement {
           self.downloadData = data
         }
         self.fire(for: .success, snapshot: self.snapshot)
-        self.removeAllObservers()
-        self.fetcherCompletion = nil
       }
-      strongSelf.state = .running
-      strongSelf.fetcher?.beginFetch { data, error in
-        let strongSelf = weakSelf
-        if let fetcherCompletion = strongSelf?.fetcherCompletion {
-          fetcherCompletion(data, error as? NSError)
-        }
+      self.state = .running
+      self.fetcher?.beginFetch { [self] data, error in
+        self.fetcherCompletion?(data, error as? NSError)
       }
     }
   }
 
   internal func cancel(withError error: NSError) {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      weakSelf?.state = .cancelled
-      weakSelf?.fetcher?.stopFetching()
-      weakSelf?.error = error
-      weakSelf?.fire(for: .failure, snapshot: self.snapshot)
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      self.state = .cancelled
+      self.fetcher?.stopFetching()
+      self.error = error
+      self.fire(for: .failure, snapshot: self.snapshot)
     }
   }
 }

+ 36 - 66
FirebaseStorage/Sources/StorageReference.swift

@@ -140,35 +140,12 @@ import Foundation
       putMetadata.path = path
       putMetadata.name = (path as NSString).lastPathComponent as String
     }
-    let fetcherService = storage.fetcherServiceForApp
     let task = StorageUploadTask(reference: self,
-                                 service: fetcherService,
+                                 service: storage.fetcherServiceForApp,
                                  queue: storage.dispatchQueue,
                                  data: uploadData,
                                  metadata: putMetadata)
-
-    if let completion = completion {
-      var completed = false
-      let callbackQueue = fetcherService.callbackQueue ?? DispatchQueue.main
-
-      task.observe(.success) { snapshot in
-        callbackQueue.async {
-          if !completed {
-            completed = true
-            completion(snapshot.metadata, nil)
-          }
-        }
-      }
-      task.observe(.failure) { snapshot in
-        callbackQueue.async {
-          if !completed {
-            completed = true
-            completion(nil, snapshot.error)
-          }
-        }
-      }
-    }
-    task.enqueue()
+    startAndObserveUploadTask(task: task, completion: completion)
     return task
   }
 
@@ -217,35 +194,12 @@ import Foundation
       putMetadata.path = path
       putMetadata.name = (path as NSString).lastPathComponent as String
     }
-    let fetcherService = storage.fetcherServiceForApp
     let task = StorageUploadTask(reference: self,
-                                 service: fetcherService,
+                                 service: storage.fetcherServiceForApp,
                                  queue: storage.dispatchQueue,
                                  file: fileURL,
                                  metadata: putMetadata)
-
-    if let completion = completion {
-      var completed = false
-      let callbackQueue = fetcherService.callbackQueue ?? DispatchQueue.main
-
-      task.observe(.success) { snapshot in
-        callbackQueue.async {
-          if !completed {
-            completed = true
-            completion(snapshot.metadata, nil)
-          }
-        }
-      }
-      task.observe(.failure) { snapshot in
-        callbackQueue.async {
-          if !completed {
-            completed = true
-            completion(nil, snapshot.error)
-          }
-        }
-      }
-    }
-    task.enqueue()
+    startAndObserveUploadTask(task: task, completion: completion)
     return task
   }
 
@@ -271,25 +225,23 @@ import Foundation
                                    queue: storage.dispatchQueue,
                                    file: nil)
 
-    var completed = false
+    task.completionData = completion
     let callbackQueue = fetcherService.callbackQueue ?? DispatchQueue.main
 
     task.observe(.success) { snapshot in
       let error = self.checkSizeOverflow(task: snapshot.task, maxSize: maxSize)
       callbackQueue.async {
-        if !completed {
-          completed = true
+        if let completion = task.completionData {
           let data = error == nil ? task.downloadData : nil
           completion(data, error)
+          task.completionData = nil
         }
       }
     }
     task.observe(.failure) { snapshot in
       callbackQueue.async {
-        if !completed {
-          completed = true
-          completion(nil, snapshot.error)
-        }
+        task.completionData?(nil, snapshot.error)
+        task.completionData = nil
       }
     }
     task.observe(.progress) { snapshot in
@@ -365,23 +317,19 @@ import Foundation
                                    file: fileURL)
 
     if let completion = completion {
-      var completed = false
+      task.completionURL = completion
       let callbackQueue = fetcherService.callbackQueue ?? DispatchQueue.main
 
       task.observe(.success) { snapshot in
         callbackQueue.async {
-          if !completed {
-            completed = true
-            completion(fileURL, nil)
-          }
+          task.completionURL?(fileURL, nil)
+          task.completionURL = nil
         }
       }
       task.observe(.failure) { snapshot in
         callbackQueue.async {
-          if !completed {
-            completed = true
-            completion(nil, snapshot.error)
-          }
+          task.completionURL?(nil, snapshot.error)
+          task.completionURL = nil
         }
       }
     }
@@ -705,4 +653,26 @@ import Foundation
     }
     return nil
   }
+
+  private func startAndObserveUploadTask(task: StorageUploadTask,
+                                         completion: ((_: StorageMetadata?, _: Error?) -> Void)?) {
+    if let completion = completion {
+      task.completionMetadata = completion
+      let callbackQueue = storage.fetcherServiceForApp.callbackQueue ?? DispatchQueue.main
+
+      task.observe(.success) { snapshot in
+        callbackQueue.async {
+          task.completionMetadata?(snapshot.metadata, nil)
+          task.completionMetadata = nil
+        }
+      }
+      task.observe(.failure) { snapshot in
+        callbackQueue.async {
+          task.completionMetadata?(nil, snapshot.error)
+          task.completionMetadata = nil
+        }
+      }
+    }
+    task.enqueue()
+  }
 }

+ 55 - 64
FirebaseStorage/Sources/StorageUploadTask.swift

@@ -36,21 +36,20 @@ import Foundation
    * Prepares a task and begins execution.
    */
   @objc open func enqueue() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      guard let strongSelf = weakSelf else { return }
-      if let contentValidationError = strongSelf.contentUploadError() {
-        strongSelf.error = contentValidationError
-        strongSelf.finishTaskWithStatus(status: .failure, snapshot: strongSelf.snapshot)
+    // Capturing self so that the upload is done whether or not there is a callback.
+    dispatchQueue.async { [self] in
+      if let contentValidationError = self.contentUploadError() {
+        self.error = contentValidationError
+        self.finishTaskWithStatus(status: .failure, snapshot: self.snapshot)
         return
       }
 
-      strongSelf.state = .queueing
-      var request = strongSelf.baseRequest
+      self.state = .queueing
+      var request = self.baseRequest
       request.httpMethod = "POST"
-      request.timeoutInterval = strongSelf.reference.storage.maxUploadRetryTime
+      request.timeoutInterval = self.reference.storage.maxUploadRetryTime
 
-      let dataRepresentation = strongSelf.uploadMetadata.dictionaryRepresentation()
+      let dataRepresentation = self.uploadMetadata.dictionaryRepresentation()
       let bodyData = try? JSONSerialization.data(withJSONObject: dataRepresentation)
 
       request.httpBody = bodyData
@@ -64,48 +63,47 @@ import Foundation
          let path = components?.path {
         components?.percentEncodedPath = "/upload\(path)"
       }
-      guard let path = strongSelf.GCSEscapedString(self.uploadMetadata.path) else {
+      guard let path = self.GCSEscapedString(self.uploadMetadata.path) else {
         fatalError("Internal error enqueueing a Storage task")
       }
       components?.percentEncodedQuery = "uploadType=resumable&name=\(path)"
 
       request.url = components?.url
 
-      guard let contentType = strongSelf.uploadMetadata.contentType else {
+      guard let contentType = self.uploadMetadata.contentType else {
         fatalError("Internal error enqueueing a Storage task")
       }
       let uploadFetcher = GTMSessionUploadFetcher(
         request: request,
         uploadMIMEType: contentType,
         chunkSize: Int64.max,
-        fetcherService: strongSelf.fetcherService
+        fetcherService: self.fetcherService
       )
-      if let data = strongSelf.uploadData {
+      if let data = self.uploadData {
         uploadFetcher.uploadData = data
         uploadFetcher.comment = "Data UploadTask"
-      } else if let fileURL = strongSelf.fileURL {
+      } else if let fileURL = self.fileURL {
         uploadFetcher.uploadFileURL = fileURL
         uploadFetcher.comment = "File UploadTask"
       }
-      uploadFetcher.maxRetryInterval = strongSelf.reference.storage.maxUploadRetryInterval
-
-      uploadFetcher.sendProgressBlock = { (bytesSent: Int64, totalBytesSent: Int64,
-                                           totalBytesExpectedToSend: Int64) in
-          weakSelf?.state = .progress
-          weakSelf?.progress.completedUnitCount = totalBytesSent
-          weakSelf?.progress.totalUnitCount = totalBytesExpectedToSend
-          weakSelf?.metadata = weakSelf?.uploadMetadata
-          if let snapshot = weakSelf?.snapshot {
-            weakSelf?.fire(for: .progress, snapshot: snapshot)
-          }
-          weakSelf?.state = .running
+      uploadFetcher.maxRetryInterval = self.reference.storage.maxUploadRetryInterval
+
+      uploadFetcher.sendProgressBlock = { [weak self] (bytesSent: Int64, totalBytesSent: Int64,
+                                                       totalBytesExpectedToSend: Int64) in
+          guard let self = self else { return }
+          self.state = .progress
+          self.progress.completedUnitCount = totalBytesSent
+          self.progress.totalUnitCount = totalBytesExpectedToSend
+          self.metadata = self.uploadMetadata
+          self.fire(for: .progress, snapshot: self.snapshot)
+          self.state = .running
       }
-      strongSelf.uploadFetcher = uploadFetcher
+      self.uploadFetcher = uploadFetcher
 
       // Process fetches
-      strongSelf.state = .running
+      self.state = .running
 
-      strongSelf.fetcherCompletion = { (data: Data?, error: NSError?) in
+      self.fetcherCompletion = { [self] (data: Data?, error: NSError?) in
         // Fire last progress updates
         self.fire(for: .progress, snapshot: self.snapshot)
 
@@ -134,11 +132,8 @@ import Foundation
         }
         self.finishTaskWithStatus(status: .success, snapshot: self.snapshot)
       }
-      strongSelf.uploadFetcher?.beginFetch { (data: Data?, error: Error?) in
-        let strongSelf = weakSelf
-        if let fetcherCompletion = strongSelf?.fetcherCompletion {
-          fetcherCompletion(data, error as NSError?)
-        }
+      self.uploadFetcher?.beginFetch { [weak self] (data: Data?, error: Error?) in
+        self?.fetcherCompletion?(data, error as NSError?)
       }
     }
   }
@@ -147,16 +142,14 @@ import Foundation
    * Pauses a task currently in progress.
    */
   @objc open func pause() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      weakSelf?.state = .paused
-      weakSelf?.uploadFetcher?.pauseFetching()
-      if weakSelf?.state != .success {
-        weakSelf?.metadata = weakSelf?.uploadMetadata
-      }
-      if let snapshot = weakSelf?.snapshot {
-        weakSelf?.fire(for: .pause, snapshot: snapshot)
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      self.state = .paused
+      self.uploadFetcher?.pauseFetching()
+      if self.state != .success {
+        self.metadata = self.uploadMetadata
       }
+      self.fire(for: .pause, snapshot: self.snapshot)
     }
   }
 
@@ -164,20 +157,18 @@ import Foundation
    * Cancels a task.
    */
   @objc open func cancel() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      weakSelf?.state = .cancelled
-      weakSelf?.uploadFetcher?.stopFetching()
-      if weakSelf?.state != .success {
-        weakSelf?.metadata = weakSelf?.uploadMetadata
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      self.state = .cancelled
+      self.uploadFetcher?.stopFetching()
+      if self.state != .success {
+        self.metadata = self.uploadMetadata
       }
-      weakSelf?.error = StorageErrorCode.error(
+      self.error = StorageErrorCode.error(
         withServerError: StorageErrorCode.cancelled as NSError,
         ref: self.reference
       )
-      if let snapshot = weakSelf?.snapshot {
-        weakSelf?.fire(for: .failure, snapshot: snapshot)
-      }
+      self.fire(for: .failure, snapshot: self.snapshot)
     }
   }
 
@@ -185,17 +176,15 @@ import Foundation
    * Resumes a paused task.
    */
   @objc open func resume() {
-    weak var weakSelf = self
-    dispatchQueue.async {
-      weakSelf?.state = .resuming
-      weakSelf?.uploadFetcher?.resumeFetching()
-      if weakSelf?.state != .success {
-        weakSelf?.metadata = weakSelf?.uploadMetadata
-      }
-      if let snapshot = weakSelf?.snapshot {
-        weakSelf?.fire(for: .resume, snapshot: snapshot)
+    dispatchQueue.async { [weak self] in
+      guard let self = self else { return }
+      self.state = .resuming
+      self.uploadFetcher?.resumeFetching()
+      if self.state != .success {
+        self.metadata = self.uploadMetadata
       }
-      weakSelf?.state = .running
+      self.fire(for: .resume, snapshot: self.snapshot)
+      self.state = .running
     }
   }
 
@@ -203,6 +192,8 @@ import Foundation
   private var fetcherCompletion: ((Data?, NSError?) -> Void)?
   private var uploadMetadata: StorageMetadata
   private var uploadData: Data?
+  // Hold completion in object to force it to be retained until completion block is called.
+  internal var completionMetadata: ((StorageMetadata?, Error?) -> Void)?
 
   // MARK: - Internal Implementations