Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
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,43 @@ final class HeartbeatStorage: HeartbeatStorageProtocol {
// MARK: - Instance Management

/// Statically allocated cache of `HeartbeatStorage` instances keyed by string IDs.
private static var cachedInstances: [String: WeakContainer<HeartbeatStorage>] = [:]
#if compiler(>=6)
// In Swift 6, this property is not concurrency-safe because it is
// nonisolated global shared mutable state. Because this target's
// deployment version does not support Swift concurrency, it is marked as
// `nonisolated(unsafe)` to disable concurrency-safety checks. The
// property's access is protected by an external synchronization mechanism
// (see `instancesLock` property).
private nonisolated(unsafe) static var cachedInstances: [
String: WeakContainer<HeartbeatStorage>
] =
[:]
#else
// TODO(Xcode 16): Delete this block when minimum supported Xcode is
// Xcode 16.
private static var cachedInstances: [
String: WeakContainer<HeartbeatStorage>
] =
[:]
#endif // compiler(>=6)

/// Used to synchronize concurrent access to the `cachedInstances` property.
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 {
return cachedInstance
} else {
let newInstance = HeartbeatStorage.makeHeartbeatStorage(id: id)
cachedInstances[id] = WeakContainer(object: newInstance)
return newInstance
}
}
}

Expand All @@ -88,7 +111,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
30 changes: 30 additions & 0 deletions FirebaseCore/Internal/Tests/Unit/HeartbeatStorageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,36 @@ class HeartbeatStorageTests: XCTestCase {
// Then
wait(for: expectations, timeout: 1.0, enforceOrder: true)
}

func testForMemoryLeakInInstanceManager() {
// 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()

// 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