Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,30 @@ final class HeartbeatStorage: HeartbeatStorageProtocol {
// MARK: - Instance Management

/// Statically allocated cache of `HeartbeatStorage` instances keyed by string IDs.
private static var cachedInstances: [String: WeakContainer<HeartbeatStorage>] = [:]
private nonisolated(unsafe) static var cachedInstances: [
String: WeakContainer<HeartbeatStorage>
] =
[:]

/// Used to synchronize mutations
private static let instancesLock = NSLock()

/// Gets an existing `HeartbeatStorage` instance with the given `id` if one exists. Otherwise,
/// makes a new instance with the given `id`.
///
/// - Parameter id: A string identifier.
/// - Returns: A `HeartbeatStorage` instance.
static func getInstance(id: String) -> HeartbeatStorage {
if let cachedInstance = cachedInstances[id]?.object {
return cachedInstance
} else {
let newInstance = HeartbeatStorage.makeHeartbeatStorage(id: id)
cachedInstances[id] = WeakContainer(object: newInstance)
return newInstance
instancesLock.withLock {
if let cachedInstance = cachedInstances[id]?.object {
print("fred")
return cachedInstance
} else {
print("bob")
let newInstance = HeartbeatStorage.makeHeartbeatStorage(id: id)
cachedInstances[id] = WeakContainer(object: newInstance)
return newInstance
}
}
}

Expand All @@ -88,7 +98,9 @@ final class HeartbeatStorage: HeartbeatStorageProtocol {

deinit {
// Removes the instance if it was cached.
Self.cachedInstances.removeValue(forKey: id)
_ = Self.instancesLock.withLock {
Self.cachedInstances.removeValue(forKey: id)
}
}

// MARK: - HeartbeatStorageProtocol
Expand Down
39 changes: 39 additions & 0 deletions FirebaseCore/Internal/Tests/Unit/HeartbeatStorageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,45 @@ class HeartbeatStorageTests: XCTestCase {
// Then
wait(for: expectations, timeout: 1.0, enforceOrder: true)
}

func testForMemoryLeakInInstanceManagemer() {
// Given
let id = "testID"
var weakRefs: [WeakContainer<HeartbeatStorage>] = []
// Lock is used to synchronize `weakRefs` during concurrent access.
let weakRefsLock = NSLock()

// When
// Simulate concurrent access. This will help expose race conditions that could cause a crash.
let group = DispatchGroup()
for _ in 0 ..< 100 {
group.enter()
DispatchQueue.global().async {
let instance = HeartbeatStorage.getInstance(id: id)
weakRefsLock.withLock {
weakRefs.append(WeakContainer(object: instance))
}
group.leave()
}
}
group.wait()

#if !os(macOS)
// Simulate memory pressure. This will force the system to more aggressively reclaim memory.
// This simulation makes the test more sensitive to potential leaks.
NotificationCenter.default.post(
name: UIApplication.didReceiveMemoryWarningNotification,
object: UIApplication.shared
)
#endif // !os(macOS)

// Then
// The `weakRefs` array's references should all be nil; otherwise, something is being
// unexpectedly strongly retained.
for weakRef in weakRefs {
XCTAssertNil(weakRef.object, "Potential memory leak detected.")
}
}
}

private class StorageFake: Storage {
Expand Down
Loading