Fix race

FirebaseStorage/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Fix a potential data race in Storage initialization. (#13369)
+
 # 11.0.0
 - [fixed] Updated error handling to support both Swift error enum handling and NSError error
   handling. Some of the Swift enums have additional parameters which may be a **breaking** change.

FirebaseStorage/Sources/Internal/StorageDeleteTask.swift

@@ -24,11 +24,9 @@ import Foundation
 @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
 enum StorageDeleteTask {
   static func deleteTask(reference: StorageReference,
-                         fetcherService: GTMSessionFetcherService,
                          queue: DispatchQueue,
                          completion: ((_: Data?, _: Error?) -> Void)?) {
     StorageInternalTask(reference: reference,
-                        fetcherService: fetcherService,
                         queue: queue,
                         httpMethod: "DELETE",
                         fetcherComment: "DeleteTask",

FirebaseStorage/Sources/Internal/StorageFetcherService.swift

@@ -0,0 +1,86 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import Foundation
+
+#if COCOAPODS
+  import GTMSessionFetcher
+#else
+  import GTMSessionFetcherCore
+#endif
+
+/// Manage Storage's fetcherService
+@available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
+actor StorageFetcherService {
+  private var _fetcherService: GTMSessionFetcherService?
+
+  func fetcherService(_ storage: Storage) -> GTMSessionFetcherService {
+    if let _fetcherService {
+      return _fetcherService
+    }
+    let app = storage.app
+    var bucketMap = StorageFetcherService.fetcherServiceMap[app.name]
+    if bucketMap == nil {
+      bucketMap = [:]
+      StorageFetcherService.fetcherServiceMap[app.name] = bucketMap
+    }
+    var fetcherService = bucketMap?[storage.storageBucket]
+    if fetcherService == nil {
+      fetcherService = GTMSessionFetcherService()
+      fetcherService?.isRetryEnabled = true
+      fetcherService?.retryBlock = retryWhenOffline
+      fetcherService?.allowLocalhostRequest = true
+      fetcherService?.maxRetryInterval = storage.maxOperationRetryInterval
+      fetcherService?.testBlock = testBlock
+      let authorizer = StorageTokenAuthorizer(
+        googleAppID: app.options.googleAppID,
+        callbackQueue: storage.callbackQueue,
+        authProvider: storage.auth,
+        appCheck: storage.appCheck
+      )
+      fetcherService?.authorizer = authorizer
+      bucketMap?[storage.storageBucket] = fetcherService
+    }
+    if storage.usesEmulator {
+      fetcherService?.allowLocalhostRequest = true
+      fetcherService?.allowedInsecureSchemes = ["http"]
+    }
+    _fetcherService = fetcherService
+    return fetcherService!
+  }
+
+  /// Update the testBlock for unit testing. Save it as a property since this may be called before
+  /// fetcherService is initialized.
+  func updateTestBlock(_ block: @escaping GTMSessionFetcherTestBlock) {
+    testBlock = block
+  }
+
+  private var testBlock: GTMSessionFetcherTestBlock? = nil
+
+  /// Map of apps to a dictionary of buckets to GTMSessionFetcherService.
+  private static var fetcherServiceMap: [String: [String: GTMSessionFetcherService]] = [:]
+
+  private var retryWhenOffline: GTMSessionFetcherRetryBlock = {
+    (suggestedWillRetry: Bool,
+     error: Error?,
+     response: @escaping GTMSessionFetcherRetryResponse) in
+    var shouldRetry = suggestedWillRetry
+    // GTMSessionFetcher does not consider being offline a retryable error, but we do, so we
+    // special-case it here.
+    if !shouldRetry, error != nil {
+      shouldRetry = (error as? NSError)?.code == URLError.notConnectedToInternet.rawValue
+    }
+    response(shouldRetry)
+  }
+}

FirebaseStorage/Sources/Internal/StorageGetDownloadURLTask.swift

@@ -14,21 +14,13 @@
 
 import Foundation
 
-#if COCOAPODS
-  import GTMSessionFetcher
-#else
-  import GTMSessionFetcherCore
-#endif
-
 /// Task which provides the ability to get a download URL for an object in Firebase Storage.
 @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
 enum StorageGetDownloadURLTask {
   static func getDownloadURLTask(reference: StorageReference,
-                                 fetcherService: GTMSessionFetcherService,
                                  queue: DispatchQueue,
                                  completion: ((_: URL?, _: Error?) -> Void)?) {
     StorageInternalTask(reference: reference,
-                        fetcherService: fetcherService,
                         queue: queue,
                         httpMethod: "GET",
                         fetcherComment: "GetDownloadURLTask") { (data: Data?, error: Error?) in

FirebaseStorage/Sources/Internal/StorageGetMetadataTask.swift

@@ -14,21 +14,13 @@
 
 import Foundation
 
-#if COCOAPODS
-  import GTMSessionFetcher
-#else
-  import GTMSessionFetcherCore
-#endif
-
 /// Task which provides the ability to delete an object in Firebase Storage.
 @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
 enum StorageGetMetadataTask {
   static func getMetadataTask(reference: StorageReference,
-                              fetcherService: GTMSessionFetcherService,
                               queue: DispatchQueue,
                               completion: ((_: StorageMetadata?, _: Error?) -> Void)?) {
     StorageInternalTask(reference: reference,
-                        fetcherService: fetcherService,
                         queue: queue,
                         httpMethod: "GET",
                         fetcherComment: "GetMetadataTask") { (data: Data?, error: Error?) in

FirebaseStorage/Sources/Internal/StorageInternalTask.swift

@@ -27,35 +27,35 @@ class StorageInternalTask: StorageTask {
 
   @discardableResult
   init(reference: StorageReference,
-       fetcherService: GTMSessionFetcherService,
        queue: DispatchQueue,
        request: URLRequest? = nil,
        httpMethod: String,
        fetcherComment: String,
        completion: ((_: Data?, _: Error?) -> Void)?) {
-    super.init(reference: reference, service: fetcherService, queue: queue)
+    super.init(reference: reference, queue: queue)
 
     // Prepare a task and begins execution.
     dispatchQueue.async { [self] in
       self.state = .queueing
-      var request = request ?? self.baseRequest
-      request.httpMethod = httpMethod
-      request.timeoutInterval = self.reference.storage.maxOperationRetryTime
+      Task {
+        let fetcherService = await reference.storage.fetcherService
+          .fetcherService(reference.storage)
 
-      let fetcher = self.fetcherService.fetcher(with: request)
-      fetcher.comment = fetcherComment
-      self.fetcher = fetcher
+        var request = request ?? self.baseRequest
+        request.httpMethod = httpMethod
+        request.timeoutInterval = self.reference.storage.maxOperationRetryTime
 
-      Task {
-        let callbackQueue = reference.storage.fetcherService != nil ?
-          reference.storage.fetcherServiceForApp.callbackQueue : DispatchQueue.main
+        let fetcher = fetcherService.fetcher(with: request)
+        fetcher.comment = fetcherComment
+        self.fetcher = fetcher
+        let callbackQueue = reference.storage.callbackQueue
         do {
           let data = try await self.fetcher?.beginFetch()
-          callbackQueue?.async {
+          callbackQueue.async {
             completion?(data, nil)
           }
         } catch {
-          callbackQueue?.async {
+          callbackQueue.async {
             completion?(nil, StorageErrorCode.error(withServerError: error as NSError,
                                                     ref: self.reference))
           }

FirebaseStorage/Sources/Internal/StorageListTask.swift

@@ -14,17 +14,10 @@
 
 import Foundation
 
-#if COCOAPODS
-  import GTMSessionFetcher
-#else
-  import GTMSessionFetcherCore
-#endif
-
 /// A Task that lists the entries under a StorageReference
 @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
 enum StorageListTask {
   static func listTask(reference: StorageReference,
-                       fetcherService: GTMSessionFetcherService,
                        queue: DispatchQueue,
                        pageSize: Int64?,
                        previousPageToken: String?,
@@ -60,7 +53,6 @@ enum StorageListTask {
     )
 
     StorageInternalTask(reference: reference,
-                        fetcherService: fetcherService,
                         queue: queue,
                         request: request,
                         httpMethod: "GET",

FirebaseStorage/Sources/Internal/StorageUpdateMetadataTask.swift

@@ -14,17 +14,10 @@
 
 import Foundation
 
-#if COCOAPODS
-  import GTMSessionFetcher
-#else
-  import GTMSessionFetcherCore
-#endif
-
 /// A Task that lists the entries under a StorageReference
 @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
 enum StorageUpdateMetadataTask {
   static func updateMetadataTask(reference: StorageReference,
-                                 fetcherService: GTMSessionFetcherService,
                                  queue: DispatchQueue,
                                  metadata: StorageMetadata,
                                  completion: ((_: StorageMetadata?, _: Error?) -> Void)?) {
@@ -37,7 +30,6 @@ enum StorageUpdateMetadataTask {
     }
 
     StorageInternalTask(reference: reference,
-                        fetcherService: fetcherService,
                         queue: queue,
                         request: request,
                         httpMethod: "PATCH",