From b6d2b76eea7b913f9f2fb72e6ecef8f9f3ac0dfc Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 14 Nov 2022 13:30:52 +0100 Subject: [PATCH 1/3] Do not delete the app state The app state is needed on the next app start, to be copied to previous app state. This is needed to determine the app start type. Closes 2376 --- Sources/Sentry/SentryAppStateManager.m | 8 ------- Sources/Sentry/SentryFileManager.m | 23 ------------------- .../Sentry/include/SentryAppStateManager.h | 2 -- Sources/Sentry/include/SentryFileManager.h | 1 - 4 files changed, 34 deletions(-) diff --git a/Sources/Sentry/SentryAppStateManager.m b/Sources/Sentry/SentryAppStateManager.m index 23889da8f66..0302320f5e1 100644 --- a/Sources/Sentry/SentryAppStateManager.m +++ b/Sources/Sentry/SentryAppStateManager.m @@ -114,8 +114,6 @@ - (void)stop removeObserver:self name:SentryNSNotificationCenterWrapper.willTerminateNotificationName object:nil]; - - [self deleteAppState]; } } @@ -124,7 +122,6 @@ - (void)dealloc // In dealloc it's safe to unsubscribe for all, see // https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver [NSNotificationCenter.defaultCenter removeObserver:self]; - [self deleteAppState]; } /** @@ -199,11 +196,6 @@ - (void)storeCurrentAppState [self.fileManager storeAppState:[self buildCurrentAppState]]; } -- (void)deleteAppState -{ - [self.fileManager deleteAppState]; -} - #endif @end diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index daefd998bc9..9c9ee2043d3 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -581,29 +581,6 @@ - (SentryAppState *_Nullable)readAppStateFrom:(NSString *)path 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]; - [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 from %@: %@", - path, error] - andLevel:kSentryLevelError]; - } -} - - (NSNumber *_Nullable)readTimezoneOffset { SENTRY_LOG_DEBUG(@"Reading timezone offset"); diff --git a/Sources/Sentry/include/SentryAppStateManager.h b/Sources/Sentry/include/SentryAppStateManager.h index 3b9e9ac4efc..d50ab4ff1ad 100644 --- a/Sources/Sentry/include/SentryAppStateManager.h +++ b/Sources/Sentry/include/SentryAppStateManager.h @@ -35,8 +35,6 @@ SENTRY_NO_INIT - (void)storeCurrentAppState; -- (void)deleteAppState; - - (void)updateAppState:(void (^)(SentryAppState *))block; #endif diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index e53edf6c2c1..42794d2ba58 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -72,7 +72,6 @@ SENTRY_NO_INIT - (void)moveAppStateToPreviousAppState; - (SentryAppState *_Nullable)readAppState; - (SentryAppState *_Nullable)readPreviousAppState; -- (void)deleteAppState; - (void)moveBreadcrumbsToPreviousBreadcrumbs; - (NSArray *)readPreviousBreadcrumbs; From b40a9ad90a662c7ff13b1fc32e45940385e6fecd Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 14 Nov 2022 13:37:23 +0100 Subject: [PATCH 2/3] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7c9bae6276..2f2a751f563 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixes - Too long flush duration (#2370) +- Do not delete the app state when OOM tracking is disabled (#2382) ## 7.30.2 From 74bc2e1d6b5910497d75b2a622c01ba0f0b740b4 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 14 Nov 2022 13:45:03 +0100 Subject: [PATCH 3/3] Fix tests --- Sources/Sentry/SentryFileManager.m | 23 ++++++++++++++++ Sources/Sentry/include/SentryFileManager.h | 1 + .../Helper/SentryAppStateManagerTests.swift | 27 +------------------ 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 9c9ee2043d3..daefd998bc9 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -581,6 +581,29 @@ - (SentryAppState *_Nullable)readAppStateFrom:(NSString *)path 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]; + [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 from %@: %@", + path, error] + andLevel:kSentryLevelError]; + } +} + - (NSNumber *_Nullable)readTimezoneOffset { SENTRY_LOG_DEBUG(@"Reading timezone offset"); diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index 42794d2ba58..e53edf6c2c1 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -72,6 +72,7 @@ SENTRY_NO_INIT - (void)moveAppStateToPreviousAppState; - (SentryAppState *_Nullable)readAppState; - (SentryAppState *_Nullable)readPreviousAppState; +- (void)deleteAppState; - (void)moveBreadcrumbsToPreviousBreadcrumbs; - (NSArray *)readPreviousBreadcrumbs; diff --git a/Tests/SentryTests/Helper/SentryAppStateManagerTests.swift b/Tests/SentryTests/Helper/SentryAppStateManagerTests.swift index 98b5f854aa6..5ec2d90526d 100644 --- a/Tests/SentryTests/Helper/SentryAppStateManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryAppStateManagerTests.swift @@ -65,39 +65,14 @@ class SentryAppStateManagerTests: XCTestCase { XCTAssertNil(fixture.fileManager.readAppState()) } - func testStopDeletesAppState() { + func testStopDoesNotDeleteAppState() { XCTAssertNil(fixture.fileManager.readAppState()) sut.start() XCTAssertNotNil(fixture.fileManager.readAppState()) - sut.stop() - XCTAssertNil(fixture.fileManager.readAppState()) - } - - func testStopOnlyRunsLogicWhenStartCountBecomesZero() { - XCTAssertNil(fixture.fileManager.readAppState()) - - sut.start() - XCTAssertNotNil(fixture.fileManager.readAppState()) - - sut.start() - sut.stop() XCTAssertNotNil(fixture.fileManager.readAppState()) - - sut.stop() - XCTAssertNil(fixture.fileManager.readAppState()) - } - - func testStoreAndDeleteAppState() { - XCTAssertNil(fixture.fileManager.readAppState()) - - sut.storeCurrentAppState() - XCTAssertNotNil(fixture.fileManager.readAppState()) - - sut.deleteAppState() - XCTAssertNil(fixture.fileManager.readAppState()) } func testUpdateAppState() {