ref: Remove a possible race condition related to reading previous app…

… state (#2250)

Sources/Sentry/SentryAppStateManager.m

@@ -57,6 +57,11 @@ - (SentryAppState *)buildCurrentAppState
                                    systemBootTimestamp:self.sysctl.systemBootTimestamp];
 }
 
+- (SentryAppState *)loadPreviousAppState
+{
+    return [self.fileManager readPreviousAppState];
+}
+
 - (SentryAppState *)loadCurrentAppState
 {
     return [self.fileManager readAppState];
@@ -67,7 +72,7 @@ - (void)storeCurrentAppState
     [self.fileManager storeAppState:[self buildCurrentAppState]];
 }
 
-- (void)removeCurrentAppState
+- (void)deleteAppState
 {
     [self.fileManager deleteAppState];
 }

Sources/Sentry/SentryFileManager.m

@@ -23,6 +23,7 @@
 @property (nonatomic, copy) NSString *currentSessionFilePath;
 @property (nonatomic, copy) NSString *crashedSessionFilePath;
 @property (nonatomic, copy) NSString *lastInForegroundFilePath;
+@property (nonatomic, copy) NSString *previousAppStateFilePath;
 @property (nonatomic, copy) NSString *appStateFilePath;
 @property (nonatomic, copy) NSString *timezoneOffsetFilePath;
 @property (nonatomic, assign) NSUInteger currentFileCounter;
@@ -65,6 +66,8 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options
         self.lastInForegroundFilePath =
             [self.sentryPath stringByAppendingPathComponent:@"lastInForeground.timestamp"];
 
+        self.previousAppStateFilePath =
+            [self.sentryPath stringByAppendingPathComponent:@"previous.app.state"];
         self.appStateFilePath = [self.sentryPath stringByAppendingPathComponent:@"app.state"];
         self.timezoneOffsetFilePath =
             [self.sentryPath stringByAppendingPathComponent:@"timezone.offset"];
@@ -395,32 +398,71 @@ - (void)storeAppState:(SentryAppState *)appState
     }
 }
 
+- (void)moveAppStateToPreviousAppState
+{
+    @synchronized(self.appStateFilePath) {
+        NSFileManager *fileManager = [NSFileManager defaultManager];
+        NSError *error = nil;
+        [fileManager moveItemAtPath:self.appStateFilePath
+                             toPath:self.previousAppStateFilePath
+                              error:&error];
+
+        // We don't want to log an error if the file doesn't exist.
+        if (nil != error && error.code != NSFileNoSuchFileError) {
+            [SentryLog
+                logWithMessage:[NSString
+                                   stringWithFormat:
+                                       @"Failed to move app state to previous app state: %@", error]
+                      andLevel:kSentryLevelError];
+        }
+    }
+}
+
 - (SentryAppState *_Nullable)readAppState
+{
+    @synchronized(self.appStateFilePath) {
+        return [self readAppStateFrom:self.appStateFilePath];
+    }
+}
+
+- (SentryAppState *_Nullable)readPreviousAppState
+{
+    @synchronized(self.previousAppStateFilePath) {
+        return [self readAppStateFrom:self.previousAppStateFilePath];
+    }
+}
+
+- (SentryAppState *_Nullable)readAppStateFrom:(NSString *)path
 {
     NSFileManager *fileManager = [NSFileManager defaultManager];
     NSData *currentData = nil;
-    @synchronized(self.appStateFilePath) {
-        currentData = [fileManager contentsAtPath:self.appStateFilePath];
-        if (nil == currentData) {
-            return nil;
-        }
+    currentData = [fileManager contentsAtPath:path];
+    if (nil == currentData) {
+        return nil;
     }
     return [SentrySerialization appStateWithData:currentData];
 }
 
 - (void)deleteAppState
+{
+    @synchronized(self.appStateFilePath) {
+        [self deleteAppStateFrom:self.appStateFilePath];
+        [self deleteAppStateFrom:self.previousAppStateFilePath];
+    }
+}
+
+- (void)deleteAppStateFrom:(NSString *)path
 {
     NSError *error = nil;
     NSFileManager *fileManager = [NSFileManager defaultManager];
-    @synchronized(self.appStateFilePath) {
-        [fileManager removeItemAtPath:self.appStateFilePath error:&error];
+    [fileManager removeItemAtPath:path error:&error];
 
-        // We don't want to log an error if the file doesn't exist.
-        if (nil != error && error.code != NSFileNoSuchFileError) {
-            [SentryLog
-                logWithMessage:[NSString stringWithFormat:@"Failed to delete app state %@", error]
-                      andLevel:kSentryLevelError];
-        }
+    // We don't want to log an error if the file doesn't exist.
+    if (nil != error && error.code != NSFileNoSuchFileError) {
+        [SentryLog
+            logWithMessage:[NSString stringWithFormat:@"Failed to delete app state from %@: %@",
+                                     path, error]
+                  andLevel:kSentryLevelError];
     }
 }
 

Sources/Sentry/SentryOutOfMemoryLogic.m

@@ -40,7 +40,7 @@ - (BOOL)isOOM
     }
 
 #if SENTRY_HAS_UIKIT
-    SentryAppState *previousAppState = [self.appStateManager loadCurrentAppState];
+    SentryAppState *previousAppState = [self.appStateManager loadPreviousAppState];
     SentryAppState *currentAppState = [self.appStateManager buildCurrentAppState];
 
     // If there is no previous app state, we can't do anything.

Sources/Sentry/SentryOutOfMemoryTracker.m

@@ -129,7 +129,7 @@ - (void)stop
         removeObserver:self
                   name:SentryNSNotificationCenterWrapper.willTerminateNotificationName
                 object:nil];
-    [self.appStateManager removeCurrentAppState];
+    [self.appStateManager deleteAppState];
 #endif
 }
 

Sources/Sentry/SentrySDK.m

@@ -146,7 +146,10 @@ + (void)startWithOptionsObject:(SentryOptions *)options
     startInvocations++;
 
     [SentryLog configure:options.debug diagnosticLevel:options.diagnosticLevel];
+
     SentryClient *newClient = [[SentryClient alloc] initWithOptions:options];
+    [newClient.fileManager moveAppStateToPreviousAppState];
+
     // The Hub needs to be initialized with a client so that closing a session
     // can happen.
     [SentrySDK setCurrentHub:[[SentryHub alloc] initWithClient:newClient andScope:nil]];

Sources/Sentry/include/SentryAppStateManager.h

@@ -26,11 +26,13 @@ SENTRY_NO_INIT
  */
 - (SentryAppState *)buildCurrentAppState;
 
+- (SentryAppState *)loadPreviousAppState;
+
 - (SentryAppState *)loadCurrentAppState;
 
 - (void)storeCurrentAppState;
 
-- (void)removeCurrentAppState;
+- (void)deleteAppState;
 
 - (void)updateAppState:(void (^)(SentryAppState *))block;
 

Sources/Sentry/include/SentryFileManager.h

@@ -60,7 +60,9 @@ SENTRY_NO_INIT
 - (NSString *)storeDictionary:(NSDictionary *)dictionary toPath:(NSString *)path;
 
 - (void)storeAppState:(SentryAppState *)appState;
+- (void)moveAppStateToPreviousAppState;
 - (SentryAppState *_Nullable)readAppState;
+- (SentryAppState *_Nullable)readPreviousAppState;
 - (void)deleteAppState;
 
 - (NSNumber *_Nullable)readTimezoneOffset;

Tests/SentryTests/Helper/SentryFileManagerTests.swift

@@ -439,6 +439,15 @@ class SentryFileManagerTests: XCTestCase {
         sut.deleteAppState()
         XCTAssertNil(sut.readAppState())
     }
+
+    func testDeletePreviousAppState() {
+        sut.store(TestData.appState)
+        sut.moveAppStateToPreviousAppState()
+        sut.deleteAppState()
+
+        XCTAssertNil(sut.readAppState())
+        XCTAssertNil(sut.readPreviousAppState())
+    }
 
     func testStore_WhenFileImmutable_AppStateIsNotOverwritten() {
         sut.store(TestData.appState)
@@ -471,6 +480,14 @@ class SentryFileManagerTests: XCTestCase {
         XCTAssertNotNil(sut.readAppState())
     }
 
+    func testMoveAppStateAndReadPreviousAppState() {
+        sut.store(TestData.appState)
+        sut.moveAppStateToPreviousAppState()
+
+        let actual = sut.readPreviousAppState()
+        XCTAssertEqual(TestData.appState, actual)
+    }
+
     func testStoreAndReadTimezoneOffset() {
         sut.storeTimezoneOffset(7_200)
         XCTAssertEqual(sut.readTimezoneOffset(), 7_200)
@@ -600,7 +617,6 @@ class SentryFileManagerTests: XCTestCase {
     private func assertValidAppStateStored() {
         let actual = sut.readAppState()
         XCTAssertEqual(TestData.appState, actual)
-
     }
 
     private func advanceTime(bySeconds: TimeInterval) {

Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift

@@ -189,6 +189,7 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase {
         sut.start()
         goToForeground()
 
+        fixture.fileManager.moveAppStateToPreviousAppState()
         sut.start()
         assertOOMEventSent()
     }
@@ -208,7 +209,8 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase {
     func testAppOOM_WithOnlyHybridSdkDidBecomeActive() {
         sut.start()
         hybridSdkDidBecomeActive()
-
+
+        fixture.fileManager.moveAppStateToPreviousAppState()
         sut.start()
         assertOOMEventSent()
     }
@@ -217,7 +219,8 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase {
         sut.start()
         goToForeground()
         hybridSdkDidBecomeActive()
-
+
+        fixture.fileManager.moveAppStateToPreviousAppState()
         sut.start()
         assertOOMEventSent()
     }
@@ -226,7 +229,8 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase {
         sut.start()
         hybridSdkDidBecomeActive()
         goToForeground()
-
+
+        fixture.fileManager.moveAppStateToPreviousAppState()
         sut.start()
         assertOOMEventSent()
     }
@@ -253,15 +257,15 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase {
     func testStop_StopsObserving_NoMoreFileManagerInvocations() throws {
         let fileManager = try TestFileManager(options: Options(), andCurrentDateProvider: TestCurrentDateProvider())
         sut = fixture.getSut(fileManager: fileManager)
-        
+
         sut.start()
         sut.stop()
 
         hybridSdkDidBecomeActive()
         goToForeground()
         terminateApp()
 
-        XCTAssertEqual(1, fileManager.readAppStateInvocations.count)
+        XCTAssertEqual(1, fileManager.readPreviousAppStateInvocations.count)
     }
 
     private func givenPreviousAppState(appState: SentryAppState) {

Tests/SentryTests/Integrations/SentryCrash/SentryCrashIntegrationTests.swift

@@ -319,6 +319,7 @@ class SentryCrashIntegrationTests: NotificationCenterTestCase {
         let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: UIDevice.current.identifierForVendor?.uuidString ?? "", isDebugging: false, systemBootTimestamp: fixture.currentDateProvider.date())
         appState.isActive = true
         fixture.fileManager.store(appState)
+        fixture.fileManager.moveAppStateToPreviousAppState()
     }
     #endif
 

Tests/SentryTests/TestClient.swift

@@ -161,4 +161,10 @@ class TestFileManager: SentryFileManager {
         readAppStateInvocations.record(Void())
         return nil
     }
+
+    var readPreviousAppStateInvocations = Invocations<Void>()
+    override func readPreviousAppState() -> SentryAppState? {
+        readPreviousAppStateInvocations.record(Void())
+        return nil
+    }
 }