[Core] Make instance manager conform to Swift 6 principles

Author: ncooke3

FirebaseCore/Internal/Sources/HeartbeatLogging/HeartbeatStorage.swift

@@ -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
+      }
     }
   }
 
@@ -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

FirebaseCore/Internal/Tests/Unit/HeartbeatStorageTests.swift

@@ -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 {