From 3f1ba97f27051d633d783d2e3672f9ac7360d3ba Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Thu, 1 Aug 2024 13:30:48 -0400 Subject: [PATCH 1/6] [Swift 6] Marking unsafe --- FirebaseStorage/Sources/Storage.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index c9a33522b03..6a03189909a 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -283,10 +283,10 @@ import FirebaseCore /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are /// instances of Storage associated with the given app. - private static var instances: [String: Storage] = [:] + private nonisolated(unsafe) static var instances: [String: Storage] = [:] /// Lock to manage access to the instances array to avoid race conditions. - private static var instancesLock: os_unfair_lock = .init() + private nonisolated(unsafe) static var instancesLock: os_unfair_lock = .init() var host: String var scheme: String From d4b3b6284d7c5b3482e83468597b1c9a4147a56e Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Thu, 1 Aug 2024 16:39:47 -0400 Subject: [PATCH 2/6] Swift 5 compatible --- FirebaseStorage/Sources/Storage.swift | 42 +++++++++++++++++---------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index 6a03189909a..0f06fd3dcfc 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -21,6 +21,31 @@ import FirebaseCore // Avoids exposing internal FirebaseCore APIs to Swift users. @_implementationOnly import FirebaseCoreExtension +private final class StorageInstanceCache: @unchecked Sendable { + public static let shared = StorageInstanceCache() + + /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are + /// instances of Storage associated with the given app. + private var instances: [String: Storage] = [:] + + /// Lock to manage access to the instances array to avoid race conditions. + private var instancesLock: os_unfair_lock = .init() + + private init() {} + + func storage(app: FirebaseApp, bucket: String) -> Storage { + os_unfair_lock_lock(&instancesLock) + defer { os_unfair_lock_unlock(&instancesLock) } + + if let instance = instances[bucket] { + return instance + } + let newInstance = FirebaseStorage.Storage(app: app, bucket: bucket) + instances[bucket] = newInstance + return newInstance + } +} + /// Firebase Storage is a service that supports uploading and downloading binary objects, /// such as images, videos, and other files to Google Cloud Storage. Instances of `Storage` /// are not thread-safe, but can be accessed from any thread. @@ -73,15 +98,7 @@ import FirebaseCore } private class func storage(app: FirebaseApp, bucket: String) -> Storage { - os_unfair_lock_lock(&instancesLock) - defer { os_unfair_lock_unlock(&instancesLock) } - - if let instance = instances[bucket] { - return instance - } - let newInstance = FirebaseStorage.Storage(app: app, bucket: bucket) - instances[bucket] = newInstance - return newInstance + return StorageInstanceCache.shared.storage(app: app, bucket: bucket) } /// The `FirebaseApp` associated with this Storage instance. @@ -281,13 +298,6 @@ import FirebaseCore /// Once `configured` is true, the emulator can no longer be enabled. var configured = false - /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are - /// instances of Storage associated with the given app. - private nonisolated(unsafe) static var instances: [String: Storage] = [:] - - /// Lock to manage access to the instances array to avoid race conditions. - private nonisolated(unsafe) static var instancesLock: os_unfair_lock = .init() - var host: String var scheme: String var port: Int From ea1f31f19714022a3970b74d8b6c45b49e8da0d2 Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Thu, 1 Aug 2024 16:55:02 -0400 Subject: [PATCH 3/6] Remove not needed public access modifier --- FirebaseStorage/Sources/Storage.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index 0f06fd3dcfc..b1ca92af8aa 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -22,7 +22,7 @@ import FirebaseCore @_implementationOnly import FirebaseCoreExtension private final class StorageInstanceCache: @unchecked Sendable { - public static let shared = StorageInstanceCache() + static let shared = StorageInstanceCache() /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are /// instances of Storage associated with the given app. From 36132147b311c48f101975a5c312f2cb3def1f2f Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Thu, 1 Aug 2024 17:50:43 -0400 Subject: [PATCH 4/6] Move it into Storage scope --- FirebaseStorage/Sources/Storage.swift | 52 ++++++++++++++------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index b1ca92af8aa..d0d52a586e5 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -21,31 +21,6 @@ import FirebaseCore // Avoids exposing internal FirebaseCore APIs to Swift users. @_implementationOnly import FirebaseCoreExtension -private final class StorageInstanceCache: @unchecked Sendable { - static let shared = StorageInstanceCache() - - /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are - /// instances of Storage associated with the given app. - private var instances: [String: Storage] = [:] - - /// Lock to manage access to the instances array to avoid race conditions. - private var instancesLock: os_unfair_lock = .init() - - private init() {} - - func storage(app: FirebaseApp, bucket: String) -> Storage { - os_unfair_lock_lock(&instancesLock) - defer { os_unfair_lock_unlock(&instancesLock) } - - if let instance = instances[bucket] { - return instance - } - let newInstance = FirebaseStorage.Storage(app: app, bucket: bucket) - instances[bucket] = newInstance - return newInstance - } -} - /// Firebase Storage is a service that supports uploading and downloading binary objects, /// such as images, videos, and other files to Google Cloud Storage. Instances of `Storage` /// are not thread-safe, but can be accessed from any thread. @@ -350,3 +325,30 @@ private final class StorageInstanceCache: @unchecked Sendable { } } } + +extension Storage { + private final class InstanceCache: @unchecked Sendable { + static let shared = InstanceCache() + + /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are + /// instances of Storage associated with the given app. + private var instances: [String: Storage] = [:] + + /// Lock to manage access to the instances array to avoid race conditions. + private var instancesLock: os_unfair_lock = .init() + + private init() {} + + func storage(app: FirebaseApp, bucket: String) -> Storage { + os_unfair_lock_lock(&instancesLock) + defer { os_unfair_lock_unlock(&instancesLock) } + + if let instance = instances[bucket] { + return instance + } + let newInstance = FirebaseStorage.Storage(app: app, bucket: bucket) + instances[bucket] = newInstance + return newInstance + } + } +} From e1b5a84d9f9827aeaa8fd86fa2277595334034cd Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Fri, 2 Aug 2024 09:21:13 -0400 Subject: [PATCH 5/6] review --- FirebaseStorage/Sources/Storage.swift | 54 +++++++++++++-------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index d0d52a586e5..248007f6145 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -73,7 +73,7 @@ import FirebaseCore } private class func storage(app: FirebaseApp, bucket: String) -> Storage { - return StorageInstanceCache.shared.storage(app: app, bucket: bucket) + return InstanceCache.shared.storage(app: app, bucket: bucket) } /// The `FirebaseApp` associated with this Storage instance. @@ -240,6 +240,31 @@ import FirebaseCore } // MARK: - Internal and Private APIs + + private final class InstanceCache: @unchecked Sendable { + static let shared = InstanceCache() + + /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are + /// instances of Storage associated with the given app. + private var instances: [String: Storage] = [:] + + /// Lock to manage access to the instances array to avoid race conditions. + private var instancesLock: os_unfair_lock = .init() + + private init() {} + + func storage(app: FirebaseApp, bucket: String) -> Storage { + os_unfair_lock_lock(&instancesLock) + defer { os_unfair_lock_unlock(&instancesLock) } + + if let instance = instances[bucket] { + return instance + } + let newInstance = FirebaseStorage.Storage(app: app, bucket: bucket) + instances[bucket] = newInstance + return newInstance + } + } let fetcherService = StorageFetcherService() @@ -325,30 +350,3 @@ import FirebaseCore } } } - -extension Storage { - private final class InstanceCache: @unchecked Sendable { - static let shared = InstanceCache() - - /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are - /// instances of Storage associated with the given app. - private var instances: [String: Storage] = [:] - - /// Lock to manage access to the instances array to avoid race conditions. - private var instancesLock: os_unfair_lock = .init() - - private init() {} - - func storage(app: FirebaseApp, bucket: String) -> Storage { - os_unfair_lock_lock(&instancesLock) - defer { os_unfair_lock_unlock(&instancesLock) } - - if let instance = instances[bucket] { - return instance - } - let newInstance = FirebaseStorage.Storage(app: app, bucket: bucket) - instances[bucket] = newInstance - return newInstance - } - } -} From feae3e363ed0ec11297077e00d122922385727bd Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Fri, 2 Aug 2024 09:21:36 -0400 Subject: [PATCH 6/6] Style --- FirebaseStorage/Sources/Storage.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index 248007f6145..a41b901b04c 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -240,7 +240,7 @@ import FirebaseCore } // MARK: - Internal and Private APIs - + private final class InstanceCache: @unchecked Sendable { static let shared = InstanceCache()