firebase / firebase-ios-sdk

Firebase SDK for Apple App Development
https://firebase.google.com
Apache License 2.0
5.69k stars 1.48k forks source link

Firebase Storage Global queue usage leads to thread explosion/threading issues #10487

Closed andrewsexcellent closed 2 years ago

andrewsexcellent commented 2 years ago

Description

It seems that starting with version 10.0 and above, hitting the FirebaseStorage getData method, creates a thread explosion due to using the global queues and background as quality of service.

Here's the relevant portion: Screenshot 2022-11-13 at 22 14 20

Additionally if you call the getData method multiple times from a thread that's not the Main thread, causes the lock on GTMSessionFetcher to never release, trapping all further calls, which somehow also leads to the network service of the device to die.

Screenshot showing all the aforementioned threads stuck on GTMSessionFetcher's startSessionUsage 's @synchronized Screenshot 2022-11-11 at 11 45 54

Screenshot 2022-11-11 at 11 44 58

It's very easy to reproduce if you use the async/await continuation APIs to wrap the getData method.

All of this behaves properly in Firebase 9.6.0

Ideally the SDK shouldn't use global queues, due to them leading to thread explosions, allocating a couple of queues for Firebase would be a better experience. Also the SDK shouldn't use .background as the QoS parameter as that can lead to dependency inversion issues.

Here's Pierre Habouzit, a former GCD maintainer with his thoughts on the matter: https://twitter.com/pedantcoder/status/773903697474486273

A Swift evolution thread discussion on the matter: https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20170828/039368.html

Some more resources on GCD, global queues and priorities: https://gist.github.com/tclementdev/6af616354912b0347cdf6db159c37057

Reproducing the issue

class CloudStorageService {
    private lazy var storage = Storage.storage()

    private func getStorageData(reference: StorageReference) async throws -> Data {
        try await withCheckedThrowingContinuation { [weak self] continuation in
            reference.getData(maxSize: 1024 * 1024) { data, error in
                guard let self else {
                    continuation.resume(throwing: CloudStorageError.invalidResponse)
                    return
                }

                if let error {
                    continuation.resume(throwing: error)
                } else if let data {
                    continuation.resume(returning: data)
                }
            }
        }
    }

    func getStorageString(url: String) async throws -> String {
        let stringData = try await getStorageData(reference: storage.reference(forURL: url))
        guard let string = String(data: stringData, encoding: .utf8) else {
            throw CloudStorageError.invalidResponse
        }

        return string
    }
}

Calling

 let cloudStorageService = CloudStorageService()
 let storageString = try? await cloudStorageService.getStorageString(url: "Resource-ID") 
 let cloudStorageService2 = CloudStorageService()
 let storageString = try? await cloudStorageService2.getStorageString(url: "Resource-ID") 
 let cloudStorageService3 = CloudStorageService()
 let storageString = try? await cloudStorageService3.getStorageString(url: "Resource-ID") 
 let cloudStorageService4 = CloudStorageService()
 let storageString = try? await cloudStorageService4.getStorageString(url: "Resource-ID") 

Reproduces the issue.

Firebase SDK Version

10.1.0

Xcode Version

14.1.0

Installation Method

Swift Package Manager

Firebase Product(s)

Storage

Targeted Platforms

iOS

Relevant Log Output

No response

If using Swift Package Manager, the project's Package.resolved

Expand Package.resolved snippet
```json { "pins" : [ { "identity" : "abseil-cpp-swiftpm", "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/abseil-cpp-SwiftPM.git", "state" : { "revision" : "583de9bd60f66b40e78d08599cc92036c2e7e4e1", "version" : "0.20220203.2" } }, { "identity" : "boringssl-swiftpm", "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/boringssl-SwiftPM.git", "state" : { "revision" : "dd3eda2b05a3f459fc3073695ad1b28659066eab", "version" : "0.9.1" } }, { "identity" : "commonui_sdk_ios", "kind" : "remoteSourceControl", "location" : "https://github.com/zendesk/commonui_sdk_ios", "state" : { "revision" : "d6d40302c628bde17251c24553f18b5b47551b6b", "version" : "7.0.0" } }, { "identity" : "core_sdk_ios", "kind" : "remoteSourceControl", "location" : "https://github.com/zendesk/core_sdk_ios", "state" : { "revision" : "c4c402fda96e3c21b47c7df34b0bd4e2d0489681", "version" : "3.0.0" } }, { "identity" : "firebase-ios-sdk", "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/firebase-ios-sdk", "state" : { "revision" : "f0926478fda955aef0dfbf99263b5c24b2ca5db4", "version" : "10.1.0" } }, { "identity" : "googleappmeasurement", "kind" : "remoteSourceControl", "location" : "https://github.com/google/GoogleAppMeasurement.git", "state" : { "revision" : "71eb6700dd53a851473c48d392f00a3ab26699a6", "version" : "10.1.0" } }, { "identity" : "googledatatransport", "kind" : "remoteSourceControl", "location" : "https://github.com/google/GoogleDataTransport.git", "state" : { "revision" : "5056b15c5acbb90cd214fe4d6138bdf5a740e5a8", "version" : "9.2.0" } }, { "identity" : "googleutilities", "kind" : "remoteSourceControl", "location" : "https://github.com/google/GoogleUtilities.git", "state" : { "revision" : "68ea347bdb1a69e2d2ae2e25cd085b6ef92f64cb", "version" : "7.9.0" } }, { "identity" : "grpc-ios", "kind" : "remoteSourceControl", "location" : "https://github.com/grpc/grpc-ios.git", "state" : { "revision" : "8440b914756e0d26d4f4d054a1c1581daedfc5b6", "version" : "1.44.3-grpc" } }, { "identity" : "gtm-session-fetcher", "kind" : "remoteSourceControl", "location" : "https://github.com/google/gtm-session-fetcher.git", "state" : { "revision" : "d4289da23e978f37c344ea6a386e5546e2466294", "version" : "2.1.0" } }, { "identity" : "kingfisher", "kind" : "remoteSourceControl", "location" : "https://github.com/onevcat/Kingfisher.git", "state" : { "revision" : "44e891bdb61426a95e31492a67c7c0dfad1f87c5", "version" : "7.4.1" } }, { "identity" : "leveldb", "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/leveldb.git", "state" : { "revision" : "0706abcc6b0bd9cedfbb015ba840e4a780b5159b", "version" : "1.22.2" } }, { "identity" : "messaging_sdk_ios", "kind" : "remoteSourceControl", "location" : "https://github.com/zendesk/messaging_sdk_ios", "state" : { "revision" : "97678c795166b6bf8879ed51e3054ffaa033c883", "version" : "4.0.0" } }, { "identity" : "messagingapi_sdk_ios", "kind" : "remoteSourceControl", "location" : "https://github.com/zendesk/messagingapi_sdk_ios", "state" : { "revision" : "503f60e2bfd8bca2b39eb515ef0ac3607bdc4d1d", "version" : "4.0.0" } }, { "identity" : "nanopb", "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/nanopb.git", "state" : { "revision" : "819d0a2173aff699fb8c364b6fb906f7cdb1a692", "version" : "2.30909.0" } }, { "identity" : "nativemarkkit", "kind" : "remoteSourceControl", "location" : "https://github.com/andyfinnell/NativeMarkKit.git", "state" : { "revision" : "4833aa3f62ab3b846b2f8042418c71e5bebe167c", "version" : "2.1.1" } }, { "identity" : "promises", "kind" : "remoteSourceControl", "location" : "https://github.com/google/promises.git", "state" : { "revision" : "3e4e743631e86c8c70dbc6efdc7beaa6e90fd3bb", "version" : "2.1.1" } }, { "identity" : "purchases-ios", "kind" : "remoteSourceControl", "location" : "https://github.com/RevenueCat/purchases-ios.git", "state" : { "revision" : "f82a49e181a9ac775e8aebfac58859199bc8e7b7", "version" : "4.14.1" } }, { "identity" : "sdkconfigurations_sdk_ios", "kind" : "remoteSourceControl", "location" : "https://github.com/zendesk/sdkconfigurations_sdk_ios", "state" : { "revision" : "a99da4df16e09b89ca6c4b576ca139a7d04efcf5", "version" : "2.0.0" } }, { "identity" : "statefultabview", "kind" : "remoteSourceControl", "location" : "https://github.com/NicholasBellucci/StatefulTabView.git", "state" : { "revision" : "9faa432592874b6e20065f2aa63f20f72c5df71c", "version" : "0.1.10" } }, { "identity" : "support_providers_sdk_ios", "kind" : "remoteSourceControl", "location" : "https://github.com/zendesk/support_providers_sdk_ios", "state" : { "revision" : "cdcf4704834a6daae4f61b4130cc369bbf10c7bf", "version" : "6.0.0" } }, { "identity" : "support_sdk_ios", "kind" : "remoteSourceControl", "location" : "https://github.com/zendesk/support_sdk_ios", "state" : { "revision" : "208ee3a9dc3f94185fb4842a987c7085346b28a0", "version" : "6.0.0" } }, { "identity" : "swift-log", "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-log.git", "state" : { "revision" : "6fe203dc33195667ce1759bf0182975e4653ba1c", "version" : "1.4.4" } }, { "identity" : "swift-protobuf", "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-protobuf.git", "state" : { "revision" : "88c7d15e1242fdb6ecbafbc7926426a19be1e98a", "version" : "1.20.2" } }, { "identity" : "swiftui-introspect", "kind" : "remoteSourceControl", "location" : "https://github.com/siteline/SwiftUI-Introspect.git", "state" : { "branch" : "master", "revision" : "d3cbf32788250741d397cfb5bcd532c37aa1096d" } }, { "identity" : "swiftuix", "kind" : "remoteSourceControl", "location" : "https://github.com/SwiftUIX/SwiftUIX", "state" : { "revision" : "1c2ba19b91a1541b803f9d1b01b360cae5a752fa", "version" : "0.1.3" } } ], "version" : 2 } ```

If using CocoaPods, the project's Podfile.lock

Expand Podfile.lock snippet
```yml Replace this line with the contents of your Podfile.lock! ```
google-oss-bot commented 2 years ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

paulb777 commented 2 years ago

Thanks for the detailed report. It looks like we didn't correctly port the queue management code from Objective C to Swift. We'll dig into this.

The GTMSessionFetcher lock issue should be fixed by #10468 which is planned to release in this week's 10.2.0 version.