Skip to content

Commit 279ac2a

Browse files
paulb777paulb777
authored
[storage] Migrate to actor to fix a potential data race in initialization (#13428)
1 parent 432a371 commit 279ac2a

20 files changed

+367
-518
lines changed

‎FirebaseStorage/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# Unreleased
2+
- [fixed] Fix a potential data race in Storage initialization. (#13369)
3+
14
# 11.0.0
25
- [fixed] Updated error handling to support both Swift error enum handling and NSError error
36
handling. Some of the Swift enums have additional parameters which may be a **breaking** change.

‎FirebaseStorage/Sources/Internal/StorageDeleteTask.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,9 @@ import Foundation
2424
@available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
2525
enum StorageDeleteTask {
2626
static func deleteTask(reference: StorageReference,
27-
fetcherService: GTMSessionFetcherService,
2827
queue: DispatchQueue,
2928
completion: ((_: Data?, _: Error?) -> Void)?) {
3029
StorageInternalTask(reference: reference,
31-
fetcherService: fetcherService,
3230
queue: queue,
3331
httpMethod: "DELETE",
3432
fetcherComment: "DeleteTask",

‎FirebaseStorage/Sources/Internal/StorageFetcherService.swift

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright 2024 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import Foundation
16+
17+
#if COCOAPODS
18+
import GTMSessionFetcher
19+
#else
20+
import GTMSessionFetcherCore
21+
#endif
22+
23+
/// Manage Storage's fetcherService
24+
@available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
25+
actor StorageFetcherService {
26+
private var _fetcherService: GTMSessionFetcherService?
27+
28+
func fetcherService(_ storage: Storage) -> GTMSessionFetcherService {
29+
if let _fetcherService {
30+
return _fetcherService
31+
}
32+
let app = storage.app
33+
if StorageFetcherService.fetcherServiceMap[app.name] == nil {
34+
StorageFetcherService.fetcherServiceMap[app.name] = [:]
35+
}
36+
var fetcherService = StorageFetcherService.fetcherServiceMap[app.name]?[storage.storageBucket]
37+
if fetcherService == nil {
38+
fetcherService = GTMSessionFetcherService()
39+
fetcherService?.isRetryEnabled = true
40+
fetcherService?.retryBlock = retryWhenOffline
41+
fetcherService?.allowLocalhostRequest = true
42+
fetcherService?.maxRetryInterval = storage.maxOperationRetryInterval
43+
fetcherService?.testBlock = testBlock
44+
let authorizer = StorageTokenAuthorizer(
45+
googleAppID: app.options.googleAppID,
46+
callbackQueue: storage.callbackQueue,
47+
authProvider: storage.auth,
48+
appCheck: storage.appCheck
49+
)
50+
fetcherService?.authorizer = authorizer
51+
StorageFetcherService.fetcherServiceMap[app.name]?[storage.storageBucket] = fetcherService
52+
}
53+
if storage.usesEmulator {
54+
fetcherService?.allowLocalhostRequest = true
55+
fetcherService?.allowedInsecureSchemes = ["http"]
56+
}
57+
_fetcherService = fetcherService
58+
return fetcherService!
59+
}
60+
61+
/// Update the testBlock for unit testing. Save it as a property since this may be called before
62+
/// fetcherService is initialized.
63+
func updateTestBlock(_ block: @escaping GTMSessionFetcherTestBlock) {
64+
testBlock = block
65+
if let _fetcherService {
66+
_fetcherService.testBlock = testBlock
67+
}
68+
}
69+
70+
private var testBlock: GTMSessionFetcherTestBlock?
71+
72+
/// Map of apps to a dictionary of buckets to GTMSessionFetcherService.
73+
private static var fetcherServiceMap: [String: [String: GTMSessionFetcherService]] = [:]
74+
75+
private var retryWhenOffline: GTMSessionFetcherRetryBlock = {
76+
(suggestedWillRetry: Bool,
77+
error: Error?,
78+
response: @escaping GTMSessionFetcherRetryResponse) in
79+
var shouldRetry = suggestedWillRetry
80+
// GTMSessionFetcher does not consider being offline a retryable error, but we do, so we
81+
// special-case it here.
82+
if !shouldRetry, error != nil {
83+
shouldRetry = (error as? NSError)?.code == URLError.notConnectedToInternet.rawValue
84+
}
85+
response(shouldRetry)
86+
}
87+
}

‎FirebaseStorage/Sources/Internal/StorageGetDownloadURLTask.swift

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,13 @@
1414

1515
import Foundation
1616

17-
#if COCOAPODS
18-
import GTMSessionFetcher
19-
#else
20-
import GTMSessionFetcherCore
21-
#endif
22-
2317
/// Task which provides the ability to get a download URL for an object in Firebase Storage.
2418
@available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
2519
enum StorageGetDownloadURLTask {
2620
static func getDownloadURLTask(reference: StorageReference,
27-
fetcherService: GTMSessionFetcherService,
2821
queue: DispatchQueue,
2922
completion: ((_: URL?, _: Error?) -> Void)?) {
3023
StorageInternalTask(reference: reference,
31-
fetcherService: fetcherService,
3224
queue: queue,
3325
httpMethod: "GET",
3426
fetcherComment: "GetDownloadURLTask") { (data: Data?, error: Error?) in

‎FirebaseStorage/Sources/Internal/StorageGetMetadataTask.swift

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,13 @@
1414

1515
import Foundation
1616

17-
#if COCOAPODS
18-
import GTMSessionFetcher
19-
#else
20-
import GTMSessionFetcherCore
21-
#endif
22-
2317
/// Task which provides the ability to delete an object in Firebase Storage.
2418
@available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
2519
enum StorageGetMetadataTask {
2620
static func getMetadataTask(reference: StorageReference,
27-
fetcherService: GTMSessionFetcherService,
2821
queue: DispatchQueue,
2922
completion: ((_: StorageMetadata?, _: Error?) -> Void)?) {
3023
StorageInternalTask(reference: reference,
31-
fetcherService: fetcherService,
3224
queue: queue,
3325
httpMethod: "GET",
3426
fetcherComment: "GetMetadataTask") { (data: Data?, error: Error?) in

‎FirebaseStorage/Sources/Internal/StorageInternalTask.swift

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,35 +27,35 @@ class StorageInternalTask: StorageTask {
2727

2828
@discardableResult
2929
init(reference: StorageReference,
30-
fetcherService: GTMSessionFetcherService,
3130
queue: DispatchQueue,
3231
request: URLRequest? = nil,
3332
httpMethod: String,
3433
fetcherComment: String,
3534
completion: ((_: Data?, _: Error?) -> Void)?) {
36-
super.init(reference: reference, service: fetcherService, queue: queue)
35+
super.init(reference: reference, queue: queue)
3736

3837
// Prepare a task and begins execution.
3938
dispatchQueue.async { [self] in
4039
self.state = .queueing
41-
var request = request ?? self.baseRequest
42-
request.httpMethod = httpMethod
43-
request.timeoutInterval = self.reference.storage.maxOperationRetryTime
40+
Task {
41+
let fetcherService = await reference.storage.fetcherService
42+
.fetcherService(reference.storage)
4443

45-
let fetcher = self.fetcherService.fetcher(with: request)
46-
fetcher.comment = fetcherComment
47-
self.fetcher = fetcher
44+
var request = request ?? self.baseRequest
45+
request.httpMethod = httpMethod
46+
request.timeoutInterval = self.reference.storage.maxOperationRetryTime
4847

49-
Task {
50-
let callbackQueue = reference.storage.fetcherService != nil ?
51-
reference.storage.fetcherServiceForApp.callbackQueue : DispatchQueue.main
48+
let fetcher = fetcherService.fetcher(with: request)
49+
fetcher.comment = fetcherComment
50+
self.fetcher = fetcher
51+
let callbackQueue = reference.storage.callbackQueue
5252
do {
5353
let data = try await self.fetcher?.beginFetch()
54-
callbackQueue?.async {
54+
callbackQueue.async {
5555
completion?(data, nil)
5656
}
5757
} catch {
58-
callbackQueue?.async {
58+
callbackQueue.async {
5959
completion?(nil, StorageErrorCode.error(withServerError: error as NSError,
6060
ref: self.reference))
6161
}

‎FirebaseStorage/Sources/Internal/StorageListTask.swift

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,10 @@
1414

1515
import Foundation
1616

17-
#if COCOAPODS
18-
import GTMSessionFetcher
19-
#else
20-
import GTMSessionFetcherCore
21-
#endif
22-
2317
/// A Task that lists the entries under a StorageReference
2418
@available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
2519
enum StorageListTask {
2620
static func listTask(reference: StorageReference,
27-
fetcherService: GTMSessionFetcherService,
2821
queue: DispatchQueue,
2922
pageSize: Int64?,
3023
previousPageToken: String?,
@@ -60,7 +53,6 @@ enum StorageListTask {
6053
)
6154

6255
StorageInternalTask(reference: reference,
63-
fetcherService: fetcherService,
6456
queue: queue,
6557
request: request,
6658
httpMethod: "GET",

‎FirebaseStorage/Sources/Internal/StorageUpdateMetadataTask.swift

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,10 @@
1414

1515
import Foundation
1616

17-
#if COCOAPODS
18-
import GTMSessionFetcher
19-
#else
20-
import GTMSessionFetcherCore
21-
#endif
22-
2317
/// A Task that lists the entries under a StorageReference
2418
@available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *)
2519
enum StorageUpdateMetadataTask {
2620
static func updateMetadataTask(reference: StorageReference,
27-
fetcherService: GTMSessionFetcherService,
2821
queue: DispatchQueue,
2922
metadata: StorageMetadata,
3023
completion: ((_: StorageMetadata?, _: Error?) -> Void)?) {
@@ -37,7 +30,6 @@ enum StorageUpdateMetadataTask {
3730
}
3831

3932
StorageInternalTask(reference: reference,
40-
fetcherService: fetcherService,
4133
queue: queue,
4234
request: request,
4335
httpMethod: "PATCH",

0 commit comments

Comments
 (0)