DeviceCache: workaround for potential deadlock (#2375)

Fixes #2252, #2364, [SDKONCALL-214], [SDKONCALL-241].
See also #2078 and #2079.

### Background

#2078 solved a potential deadlock in `DeviceCache`. This could happen
when a background thread was in the middle of writing to `UserDefaults`
through `DeviceCache`, and at the same time, the main thread wanted to
read from it.
This is normally fine, except that if there are any `NotificationCenter`
observations on the `UserDefaults` used, the background thread would
then post that notification and wait for the main thread, leading to a
deadlock. What #2078 did was to use a different method to make sure that
the observation was received in the calling thread.
Additionally, #2079 avoided that observation whenever we used a
different `UserDefaults`.

### Problem

When the SDK changed to a custom `UserDefaults` (#2046), we made the
choice to only do this if there was no prior data.
However, if an app has prior RevenueCat data saved in
`UserDefaults.standard`, that instance is used.

This potential deadlock is possible for any app with prior data, that
also contains observations to `UserDefaults`, either manually through
`NotificationCenter`, or using something like `@AppStorage` with
`SwiftUI`.

### Solution

`DeviceCache.cachedAppUserID` is a very commonly used method. This PR
changes it to an in-memory cache, initialized to the `UserDefaults`
value.
By doing that, most code paths that rely on this value won't risk
deadlocking with `UserDefaults` writes that jump threads.

### Long term

I've filed SDK-3034 to fully migrate data from `UserDefaults.standard`
to avoid this problem altogether in the future.


[SDKONCALL-214]:
https://revenuecats.atlassian.net/browse/SDKONCALL-214?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[SDKONCALL-238]:
https://revenuecats.atlassian.net/browse/SDKONCALL-238?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[SDKONCALL-241]:
https://revenuecats.atlassian.net/browse/SDKONCALL-241?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Sources/Caching/DeviceCache.swift

@@ -17,24 +17,17 @@ import Foundation
 // swiftlint:disable file_length type_body_length
 class DeviceCache {
 
-    var cachedAppUserID: String? {
-        self.userDefaults.read(Self.cachedAppUserID)
-    }
-    var cachedLegacyAppUserID: String? {
-        self.userDefaults.read {
-            $0.string(forKey: CacheKeys.legacyGeneratedAppUserDefaults)
-        }
-    }
+    var cachedAppUserID: String? { return self._cachedAppUserID.value }
+    var cachedLegacyAppUserID: String? { return self._cachedLegacyAppUserID.value }
     var cachedOfferings: Offerings? { self.offeringsCachedObject.cachedInstance }
 
     private let sandboxEnvironmentDetector: SandboxEnvironmentDetector
     private let userDefaults: SynchronizedUserDefaults
     private let notificationCenter: NotificationCenter
     private let offeringsCachedObject: InMemoryCachedObject<Offerings>
 
-    /// Keeps track of whether user ID has been set to detect users clearing `UserDefaults`
-    /// cleared from under the SDK
-    private let appUserIDHasBeenSet: Atomic<Bool> = false
+    private let _cachedAppUserID: Atomic<String?>
+    private let _cachedLegacyAppUserID: Atomic<String?>
 
     private var userDefaultsObserver: NSObjectProtocol?
 
@@ -46,7 +39,8 @@ class DeviceCache {
         self.offeringsCachedObject = offeringsCachedObject
         self.notificationCenter = notificationCenter
         self.userDefaults = .init(userDefaults: userDefaults)
-        self.appUserIDHasBeenSet.value = userDefaults.string(forKey: .appUserDefaults) != nil
+        self._cachedAppUserID = .init(userDefaults.string(forKey: .appUserDefaults))
+        self._cachedLegacyAppUserID = .init(userDefaults.string(forKey: .legacyGeneratedAppUserDefaults))
 
         Logger.verbose(Strings.purchase.device_cache_init(self))
 
@@ -74,7 +68,7 @@ class DeviceCache {
 
         // Note: this should never use `self.userDefaults` directly because this method
         // might be synchronized, and `Atomic` is not reentrant.
-        if self.appUserIDHasBeenSet.value && Self.cachedAppUserID(userDefaults) == nil {
+        if self.cachedAppUserID != nil && Self.cachedAppUserID(userDefaults) == nil {
             fatalError(Strings.purchase.cached_app_user_id_deleted.description)
         }
     }
@@ -92,8 +86,8 @@ class DeviceCache {
     func cache(appUserID: String) {
         self.userDefaults.write {
             $0.setValue(appUserID, forKey: CacheKeys.appUserDefaults)
-            self.appUserIDHasBeenSet.value = true
         }
+        self._cachedAppUserID.value = appUserID
     }
 
     func clearCaches(oldAppUserID: String, andSaveWithNewUserID newUserID: String) {
@@ -118,7 +112,8 @@ class DeviceCache {
 
             // Cache new appUserID.
             userDefaults.setValue(newUserID, forKey: CacheKeys.appUserDefaults)
-            self.appUserIDHasBeenSet.value = true
+            self._cachedAppUserID.value = newUserID
+            self._cachedLegacyAppUserID.value = nil
         }
     }
 

Sources/Identity/IdentityManager.swift

@@ -71,8 +71,10 @@ class IdentityManager: CurrentUserProvider {
     }
 
     var currentUserIsAnonymous: Bool {
-        let currentAppUserIDLooksAnonymous = Self.userIsAnonymous(self.currentAppUserID)
-        let isLegacyAnonymousAppUserID = self.currentAppUserID == self.deviceCache.cachedLegacyAppUserID
+        let userID = self.currentAppUserID
+
+        lazy var currentAppUserIDLooksAnonymous = Self.userIsAnonymous(userID)
+        lazy var isLegacyAnonymousAppUserID = userID == self.deviceCache.cachedLegacyAppUserID
 
         return currentAppUserIDLooksAnonymous || isLegacyAnonymousAppUserID
     }

Tests/UnitTests/Caching/DeviceCacheTests.swift

@@ -19,20 +19,29 @@ class DeviceCacheTests: TestCase {
     override func setUp() {
         self.sandboxEnvironmentDetector = MockSandboxEnvironmentDetector(isSandbox: false)
         self.mockUserDefaults = MockUserDefaults()
-        self.deviceCache = DeviceCache(sandboxEnvironmentDetector: self.sandboxEnvironmentDetector,
-                                       userDefaults: self.mockUserDefaults)
+        self.deviceCache = self.create()
     }
 
     func testLegacyCachedUserIDUsesRightKey() {
         self.mockUserDefaults.mockValues["com.revenuecat.userdefaults.appUserID"] = "cesar"
-        let userID: String? = self.deviceCache.cachedLegacyAppUserID
-        expect(userID).to(equal("cesar"))
+
+        // `DeviceCache` caches the ID in memory.
+        // Modifying the data under the hood won't be detected
+        // so re-create `DeviceCache` to force it to read it again.
+        let deviceCache = self.create()
+
+        expect(deviceCache.cachedLegacyAppUserID) == "cesar"
     }
 
     func testCachedUserIDUsesRightKey() {
         self.mockUserDefaults.mockValues["com.revenuecat.userdefaults.appUserID.new"] = "cesar"
-        let userID: String? = self.deviceCache.cachedAppUserID
-        expect(userID).to(equal("cesar"))
+
+        // `DeviceCache` caches the user ID in memory.
+        // Modifying the data under the hood won't be detected
+        // so re-create `DeviceCache` to force it to read it again.
+        let deviceCache = self.create()
+
+        expect(deviceCache.cachedAppUserID) == "cesar"
     }
 
     func testCacheUserIDUsesRightKey() {
@@ -42,6 +51,12 @@ class DeviceCacheTests: TestCase {
             .to(equal(userID))
     }
 
+    func testCacheUserIDUpdatesCache() {
+        let userID = "cesar"
+        self.deviceCache.cache(appUserID: userID)
+        expect(self.deviceCache.cachedAppUserID) == userID
+    }
+
     func testClearCachesForAppUserIDAndSaveNewUserIDRemovesCachedCustomerInfo() {
         self.deviceCache.clearCaches(oldAppUserID: "cesar", andSaveWithNewUserID: "newUser")
         let expectedCacheKey = "com.revenuecat.userdefaults.purchaserInfo.cesar"
@@ -69,6 +84,16 @@ class DeviceCacheTests: TestCase {
         expect(self.mockUserDefaults.mockValues["com.revenuecat.userdefaults.appUserID"]).to(beNil())
     }
 
+    func testClearCachesForAppUserIDAndSaveNewUserIDUpdatesCaches() {
+        self.mockUserDefaults.mockValues["com.revenuecat.userdefaults.appUserID"] = "cesar"
+        self.mockUserDefaults.mockValues["com.revenuecat.userdefaults.appUserID.new"] = "cesar"
+
+        self.deviceCache.clearCaches(oldAppUserID: "cesar", andSaveWithNewUserID: "newUser")
+
+        expect(self.deviceCache.cachedAppUserID) == "newUser"
+        expect(self.deviceCache.cachedLegacyAppUserID).to(beNil())
+    }
+
     func testClearCachesForAppUserIDAndSaveNewUserIDDoesntRemoveCachedSubscriberAttributesIfUnsynced() {
         let userID = "andy"
         let key = "band"
@@ -519,3 +544,12 @@ class DeviceCacheTests: TestCase {
     }
 
 }
+
+private extension DeviceCacheTests {
+
+    func create() -> DeviceCache {
+        return DeviceCache(sandboxEnvironmentDetector: self.sandboxEnvironmentDetector,
+                           userDefaults: self.mockUserDefaults)
+    }
+
+}