firebase / firebase-ios-sdk

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

Potential data race in Storage method #13369

Closed jesus-mg-ios closed 2 months ago

jesus-mg-ios commented 2 months ago

Description

image

Reproducing the issue

Define a storage instance and call storage.reference(forURL: String) from multiple threads and sometimes the warn in Xcode happened.

image
 let task = Task {
            try await withCheckedThrowingContinuation { continuation in
                let storageReference = self.storage.reference(forURL: referencePath)
                storageReference.getData() { data, error in
                    if let error {
                        continuation.resume(throwing: error)
                        return
                    }
                    guard let data, !data.isEmpty else {
                        continuation.resume(throwing: Error.error)
                        return
                    }
                    continuation.resume(returning: data)
                }
            } as Data
        }

 return await task.value

Firebase SDK Version

10.25.0

Xcode Version

15.2

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 Replace this line with the contents of your Package.resolved. ```

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 months 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 months ago

@jesus-mg-ios Thanks for the report. Would you check if #13381 fixes your test case?

paulb777 commented 2 months ago

Nevermind. static methods in an actor aren't thread safe. A fix will take more investigation.

jesus-mg-ios commented 2 months ago

Maybe a dispatch queue async with a barrier flag could do the trick.

paulb777 commented 2 months ago

Thanks. First, we'll see if we can address by migrating those internal APIs to Swift concurrency sometime in the next few weeks.