Fix crash loop detector logic when upgrading

Signal/AppLaunch/AppDelegate.swift

@@ -253,8 +253,9 @@ final class AppDelegate: UIResponder, UIApplicationDelegate {
             }
         }
 
-        // Do this even if `appVersion` isn't used -- there's side effects.
         let appVersion = AppVersionImpl.shared
+        appVersion.dumpToLog()
+        appVersion.updateFirstVersionIfNeeded()
 
         let launchContext = LaunchContext(
             appContext: mainAppContext,
@@ -264,6 +265,12 @@ final class AppDelegate: UIResponder, UIApplicationDelegate {
             launchStartedAt: launchStartedAt,
         )
 
+        let userDefaults = mainAppContext.appUserDefaults()
+        if appVersion.lastAppVersionForCrashDetection != appVersion.currentAppVersion {
+            userDefaults.removeObject(forKey: Constants.appLaunchesAttemptedKey)
+        }
+        appVersion.updateLastVersionForCrashDetection()
+
         // We need to do this _after_ we set up logging, when the keychain is unlocked,
         // but before we access the database or files on disk.
         let preflightError = checkIfAllowedToLaunch(
@@ -288,7 +295,6 @@ final class AppDelegate: UIResponder, UIApplicationDelegate {
         // If this is a regular launch, increment the "launches attempted" counter.
         // If repeatedly start launching but never finish them (ie the app is
         // crashing while launching), we'll notice in `checkIfAllowedToLaunch`.
-        let userDefaults = mainAppContext.appUserDefaults()
         let appLaunchesAttempted = userDefaults.integer(forKey: Constants.appLaunchesAttemptedKey)
         userDefaults.set(appLaunchesAttempted + 1, forKey: Constants.appLaunchesAttemptedKey)
 
@@ -1015,10 +1021,7 @@ final class AppDelegate: UIResponder, UIApplicationDelegate {
         }
 
         let launchAttemptFailureThreshold = DebugFlags.betaLogging ? 2 : 3
-        if
-            appVersion.lastAppVersion == appVersion.currentAppVersion,
-            userDefaults.integer(forKey: Constants.appLaunchesAttemptedKey) >= launchAttemptFailureThreshold
-        {
+        if userDefaults.integer(forKey: Constants.appLaunchesAttemptedKey) >= launchAttemptFailureThreshold {
             if case .readCorrupted = databaseCorruptionState.status {
                 return .possibleReadCorruptionCrashed
             }

Signal/util/SyncPushTokensJob.swift

@@ -57,8 +57,6 @@ class SyncPushTokensJob: NSObject {
 
         if SSKEnvironment.shared.preferencesRef.pushToken != pushToken {
             reason = "changed"
-        } else if AppVersionImpl.shared.lastAppVersion != AppVersionImpl.shared.currentAppVersion {
-            reason = "upgraded"
         } else if !Self.hasUploadedTokensOnce.get() {
             reason = "launched"
         } else {

SignalNSE/NSEEnvironment.swift

@@ -86,6 +86,9 @@ class NSEEnvironment {
         // Note that this does much more than set a flag; it will also run all deferred blocks.
         appReadiness.setAppIsReady()
 
-        AppVersionImpl.shared.nseLaunchDidComplete()
+        let appVersion = AppVersionImpl.shared
+        appVersion.dumpToLog()
+        appVersion.updateFirstVersionIfNeeded()
+        appVersion.nseLaunchDidComplete()
     }
 }

SignalServiceKit/Util/AppVersion.swift

@@ -21,7 +21,7 @@ public protocol AppVersion {
 
     /// The version of the app the last time it was launched. `nil` if the app
     /// hasn't been launched.
-    var lastAppVersion: String? { get }
+    var lastAppVersionForCrashDetection: String? { get }
 
     /// Internally, we use a version format with 4 dotted values to uniquely
     /// identify builds. The first three values are the the release version, the
@@ -52,6 +52,8 @@ public protocol AppVersion {
     )
 
     func dumpToLog()
+    func updateFirstVersionIfNeeded()
+    func updateLastVersionForCrashDetection()
 }
 
 extension AppVersion {
@@ -116,7 +118,7 @@ public class AppVersionImpl: AppVersion {
     private let firstVersionKey = "kNSUserDefaults_FirstAppVersion"
     private let backupAppVersionKey = "kNSUserDefaults_BackupAppVersion"
     private let firstBackupAppVersionKey = "kNSUserDefaults_FirstBackupAppVersion"
-    private let lastVersionKey = "kNSUserDefaults_LastVersion"
+    private let lastVersionForCrashDetectionKey = "kNSUserDefaults_LastVersion"
     private let lastCompletedLaunchVersionKey = "kNSUserDefaults_LastCompletedLaunchAppVersion"
     private let lastCompletedMainAppLaunchVersionKey = "kNSUserDefaults_LastCompletedLaunchAppVersion_MainApp"
     private let lastCompletedSAELaunchVersionKey = "kNSUserDefaults_LastCompletedLaunchAppVersion_SAE"
@@ -124,13 +126,10 @@ public class AppVersionImpl: AppVersion {
     private let firstMainAppLaunchDateAfterUpdateKey = "FirstMainAppLaunchDateAfterUpdate"
 
     public static let shared: AppVersion = {
-        let result = AppVersionImpl(
+        return AppVersionImpl(
             bundle: Bundle.main,
-            userDefaults: CurrentAppContext().appUserDefaults()
+            userDefaults: CurrentAppContext().appUserDefaults(),
         )
-        result.dumpToLog()
-        result.updateLaunchedVersionInfo()
-        return result
     }()
 
     // MARK: - Properties
@@ -172,7 +171,7 @@ public class AppVersionImpl: AppVersion {
 
     /// The version of the app the last time it was launched. `nil` if the app
     /// hasn't been launched.
-    public var lastAppVersion: String? { userDefaults.string(forKey: lastVersionKey) }
+    public var lastAppVersionForCrashDetection: String? { userDefaults.string(forKey: lastVersionForCrashDetectionKey) }
 
     /// Internally, we use a version format with 4 dotted values to uniquely
     /// identify builds. The first three values are the the release version, the
@@ -242,11 +241,14 @@ public class AppVersionImpl: AppVersion {
         self.userDefaults = userDefaults
     }
 
-    private func updateLaunchedVersionInfo() {
+    public func updateFirstVersionIfNeeded() {
         if userDefaults.string(forKey: firstVersionKey) == nil {
             userDefaults.set(currentAppVersion, forKey: firstVersionKey)
         }
-        userDefaults.set(currentAppVersion, forKey: lastVersionKey)
+    }
+
+    public func updateLastVersionForCrashDetection() {
+        userDefaults.set(currentAppVersion, forKey: lastVersionForCrashDetectionKey)
     }
 
     public func dumpToLog() {
@@ -257,7 +259,6 @@ public class AppVersionImpl: AppVersion {
         if let firstBackupAppVersion {
             Logger.info("firstBackupAppVersion: \(formatForLogging(firstBackupAppVersion))")
         }
-        Logger.info("lastAppVersion: \(formatForLogging(lastAppVersion))")
         Logger.info("currentAppVersion: \(formatForLogging(currentAppVersion))")
         Logger.info("lastCompletedLaunchAppVersion: \(formatForLogging(lastCompletedLaunchAppVersion))")
         Logger.info("lastCompletedLaunchMainAppVersion: \(formatForLogging(lastCompletedLaunchMainAppVersion))")
@@ -363,7 +364,7 @@ public class MockAppVerion: AppVersion {
 
     public var firstBackupAppVersion: String?
 
-    public var lastAppVersion: String? = "1.0"
+    public var lastAppVersionForCrashDetection: String? = "1.0"
 
     public var currentAppVersion: String = "1.0.0.0"
 
@@ -393,6 +394,10 @@ public class MockAppVerion: AppVersion {
     ) {}
 
     public func dumpToLog() {}
+
+    public func updateFirstVersionIfNeeded() {}
+
+    public func updateLastVersionForCrashDetection() {}
 }
 
 #endif

SignalShareExtension/ShareViewController.swift

@@ -259,7 +259,10 @@ public class ShareViewController: OWSNavigationController, ShareViewDelegate, SA
         let localAci = DependenciesBridge.shared.tsAccountManager.localIdentifiersWithMaybeSneakyTransaction?.aci
         Logger.info("localAci: \(localAci?.logString ?? "<none>")")
 
-        AppVersionImpl.shared.saeLaunchDidComplete()
+        let appVersion = AppVersionImpl.shared
+        appVersion.dumpToLog()
+        appVersion.updateFirstVersionIfNeeded()
+        appVersion.saeLaunchDidComplete()
 
         Logger.info("")
     }