Skip to content

Commit

Permalink
Add missing weak specifiers
Browse files Browse the repository at this point in the history
  • Loading branch information
paulb777 committed May 8, 2023
1 parent 7453895 commit 92469b9
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 20 deletions.
3 changes: 3 additions & 0 deletions FirebaseFunctions/CHANGELOG.md
@@ -1,3 +1,6 @@
# 10.10.0
- [fixed] Fixed potential memory leak of Functions instances. (#11248)

# 10.0.0
- [fixed] Remove unnecessary and unused `encoder` and `decoder` parameters from
`public func call(_ data: Request) async throws -> Response` API. (#9891)
Expand Down
Expand Up @@ -36,7 +36,7 @@ protocol FunctionsProvider {
// MARK: - Private Variables

/// The app associated with all functions instances in this container.
private let app: FirebaseApp
private weak var app: FirebaseApp?

/// A map of active instances, grouped by app. Keys are FirebaseApp names and values are arrays
/// containing all instances of Functions associated with the given app.
Expand Down
18 changes: 18 additions & 0 deletions FirebaseFunctions/Tests/Unit/FunctionsTests.swift
Expand Up @@ -167,4 +167,22 @@ class FunctionsTests: XCTestCase {
}
waitForExpectations(timeout: 1.5)
}

/// Test that Functions instances get deallocated.
func testFunctionsLifecycle() throws {
weak var weakApp: FirebaseApp?
weak var weakFunctions: Functions?
try autoreleasepool {
let options = FirebaseOptions(googleAppID: "0:0000000000000:ios:0000000000000000",
gcmSenderID: "00000000000000000-00000000000-000000000")
options.projectID = "myProjectID"
let app1 = FirebaseApp(instanceWithName: "transitory app", options: options)
weakApp = try XCTUnwrap(app1)
let functions = Functions(app: app1, region: "transitory-region", customDomain: nil)
weakFunctions = functions
XCTAssertNotNil(weakFunctions)
}
XCTAssertNil(weakApp)
XCTAssertNil(weakFunctions)
}
}
10 changes: 5 additions & 5 deletions FirebaseFunctions/Tests/Unit/SerializerTests.swift
Expand Up @@ -198,7 +198,7 @@ class SerializerTests: XCTestCase {
1 as Int32,
"two",
[3 as Int32, ["@type": "type.googleapis.com/google.protobuf.Int64Value",
"value": "9876543210"]],
"value": "9876543210"]] as [Any],
] as NSArray
XCTAssertEqual(input, try serializer.encode(input) as? NSArray)
}
Expand All @@ -209,23 +209,23 @@ class SerializerTests: XCTestCase {
1 as Int64,
"two",
[3 as Int32, ["@type": "type.googleapis.com/google.protobuf.Int64Value",
"value": "9876543210"]],
"value": "9876543210"]] as [Any],
] as NSArray
let expected = [1 as Int64, "two", [3 as Int32, 9_876_543_210 as Int64]] as NSArray
let expected = [1 as Int64, "two", [3 as Int32, 9_876_543_210 as Int64] as [Any]] as NSArray
XCTAssertEqual(expected, try serializer.decode(input) as? NSArray)
}

func testEncodeMap() {
let input = [
"foo": 1 as Int32,
"bar": "hello",
"baz": [3 as Int32, 9_876_543_210 as Int64],
"baz": [3 as Int32, 9_876_543_210 as Int64] as [Any],
] as NSDictionary
let expected = [
"foo": 1,
"bar": "hello",
"baz": [3, ["@type": "type.googleapis.com/google.protobuf.Int64Value",
"value": "9876543210"]],
"value": "9876543210"]] as [Any] as [Any],
] as NSDictionary
let serializer = FUNSerializer()
XCTAssertEqual(expected, try serializer.encode(input) as? NSDictionary)
Expand Down
3 changes: 3 additions & 0 deletions FirebaseStorage/CHANGELOG.md
@@ -1,3 +1,6 @@
# 10.10.0
- [fixed] Fixed potential memory leak of Storage instances. (#11248)

# 10.7.0
- [added] Provide server errors via the `NSUnderlyingErrorKey`.

Expand Down
4 changes: 2 additions & 2 deletions FirebaseStorage/Sources/Storage.swift
Expand Up @@ -70,7 +70,7 @@ import FirebaseAuthInterop
@objc(storageForApp:) open class func storage(app: FirebaseApp) -> Storage {
let provider = ComponentType<StorageProvider>.instance(for: StorageProvider.self,
in: app.container)
return provider.storage(for: Storage.bucket(for: app))
return provider.storage(for: Storage.bucket(for: app), app: app)
}

/**
Expand All @@ -85,7 +85,7 @@ import FirebaseAuthInterop
open class func storage(app: FirebaseApp, url: String) -> Storage {
let provider = ComponentType<StorageProvider>.instance(for: StorageProvider.self,
in: app.container)
return provider.storage(for: Storage.bucket(for: app, urlString: url))
return provider.storage(for: Storage.bucket(for: app, urlString: url), app: app)
}

/**
Expand Down
6 changes: 3 additions & 3 deletions FirebaseStorage/Sources/StorageComponent.swift
Expand Up @@ -22,14 +22,14 @@ import FirebaseCore

@objc(FIRStorageProvider)
protocol StorageProvider {
@objc func storage(for bucket: String) -> Storage
@objc func storage(for bucket: String, app: FirebaseApp) -> Storage
}

@objc(FIRStorageComponent) class StorageComponent: NSObject, Library, StorageProvider {
// MARK: - Private Variables

/// The app associated with all Storage instances in this container.
private let app: FirebaseApp
private weak var app: FirebaseApp?

/// A map of active instances, grouped by app. Keys are FirebaseApp names and values are arrays
/// containing all instances of Storage associated with the given app.
Expand Down Expand Up @@ -63,7 +63,7 @@ protocol StorageProvider {

// MARK: - StorageProvider conformance

func storage(for bucket: String) -> Storage {
func storage(for bucket: String, app: FirebaseApp) -> Storage {
os_unfair_lock_lock(&instancesLock)

// Unlock before the function returns.
Expand Down
35 changes: 26 additions & 9 deletions FirebaseStorage/Tests/Unit/StorageComponentTests.swift
Expand Up @@ -34,8 +34,9 @@ class StorageComponentTests: StorageTestHelpers {

/// Tests that a Storage instance can be created properly by the StorageComponent.
func testStorageInstanceCreation() throws {
let component = StorageComponent(app: StorageComponentTests.app)
let storage = component.storage(for: "someBucket")
let app = try XCTUnwrap(StorageComponentTests.app)
let component = StorageComponent(app: app)
let storage = component.storage(for: "someBucket", app: app)
XCTAssertNotNil(storage)
}

Expand All @@ -61,26 +62,42 @@ class StorageComponentTests: StorageTestHelpers {

/// Tests that instances of Storage created are different.
func testMultipleStorageInstancesCreated() throws {
let app = try XCTUnwrap(StorageComponentTests.app)
let registrants = NSMutableSet(array: [StorageComponent.self])
let container = FirebaseComponentContainer(
app: StorageComponentTests.app,
registrants: registrants
)
let container = FirebaseComponentContainer(app: app, registrants: registrants)

let provider = ComponentType<StorageProvider>.instance(for: StorageProvider.self,
in: container)
XCTAssertNotNil(provider)

let storage1 = provider.storage(for: "randomBucket")
let storage2 = provider.storage(for: "randomBucket")
let storage1 = provider.storage(for: "randomBucket", app: app)
let storage2 = provider.storage(for: "randomBucket", app: app)
XCTAssertNotNil(storage1)

// Ensure they're the same instance.
XCTAssert(storage1 === storage2)

let storage3 = provider.storage(for: "differentBucket")
let storage3 = provider.storage(for: "differentBucket", app: app)
XCTAssertNotNil(storage3)

XCTAssert(storage1 !== storage3)
}

/// Test that Storage instances get deallocated.
func testStorageLifecycle() throws {
weak var weakApp: FirebaseApp?
weak var weakStorage: Storage?
try autoreleasepool {
let options = FirebaseOptions(googleAppID: "0:0000000000000:ios:0000000000000000",
gcmSenderID: "00000000000000000-00000000000-000000000")
options.projectID = "myProjectID"
let app1 = FirebaseApp(instanceWithName: "transitory app", options: options)
weakApp = try XCTUnwrap(app1)
let storage = Storage(app: app1, bucket: "transitory bucket")
weakStorage = storage
XCTAssertNotNil(weakStorage)
}
XCTAssertNil(weakApp)
XCTAssertNil(weakStorage)
}
}

0 comments on commit 92469b9

Please sign in to comment.