From d9f7a67d06881cbf4ed42039299aa9739e579443 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 13 Dec 2022 11:53:17 +0100 Subject: [PATCH] fix: ARC issue for FileManager The SentryFileManager kept a strong reference to self for running code 10 seconds delayed. The strong reference stops ARC from deallocating the SentryFileManager. This is fixed now by using a weak reference. --- CHANGELOG.md | 1 + Sources/Sentry/SentryFileManager.m | 8 +++-- .../Helper/SentryFileManagerTests.swift | 36 ++++++++++++++----- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b9976a4e3..1c3e847635 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This version adds a dependency on Swift. - The SDK no longer reports an OOM when a crash happens after closing the SDK (#2468) - Don't capture zero size screenshots ([#2459](https://github.com/getsentry/sentry-cocoa/pull/2459)) - Use the preexisting app release version format for profiles (#2470) +- Fix ARC issue for FileManager (#2525) ### Breaking Changes diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 022cd3290b..115e32bb5b 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -77,11 +77,15 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options self.currentFileCounter = 0; self.maxEnvelopes = options.maxCacheItems; + __weak SentryFileManager *weakSelf = self; [dispatchQueueWrapper dispatchAfter:10 block:^{ - SENTRY_LOG_DEBUG(@"Dispatched deletion of old envelopes from %@", self); - [self deleteOldEnvelopesFromAllSentryPaths]; + if (weakSelf == nil) { + return; + } + SENTRY_LOG_DEBUG(@"Dispatched deletion of old envelopes from %@", weakSelf); + [weakSelf deleteOldEnvelopesFromAllSentryPaths]; }]; } return self; diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 7791e61a17..9a1ef8e2d1 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -136,14 +136,7 @@ class SentryFileManagerTests: XCTestCase { func testDeleteOldEnvelopes() throws { fixture.dispatchQueueWrapper.dispatchAfterExecutesBlock = true - let envelope = TestConstants.envelope - let path = sut.store(envelope) - - let timeIntervalSince1970 = fixture.currentDateProvider.date().timeIntervalSince1970 - (90 * 24 * 60 * 60) - let date = Date(timeIntervalSince1970: timeIntervalSince1970 - 1) - try FileManager.default.setAttributes([FileAttributeKey.creationDate: date], ofItemAtPath: path) - - XCTAssertEqual(sut.getAllEnvelopes().count, 1) + try givenOldEnvelopes() sut = fixture.getSut() @@ -181,6 +174,22 @@ class SentryFileManagerTests: XCTestCase { XCTAssertEqual(sut.getAllEnvelopes().count, 1) } + + func testFileManagerDeallocated_OldEnvelopesNotDeleted() throws { + try givenOldEnvelopes() + + fixture.dispatchQueueWrapper.dispatchAfterExecutesBlock = false + + // Initialize sut in extra function so ARC deallocates it + func getSut() { + _ = fixture.getSut() + } + getSut() + + fixture.dispatchQueueWrapper.invokeLastDispatchAfter() + + XCTAssertEqual(sut.getAllEnvelopes().count, 1) + } func testCreateDirDoesNotThrow() throws { try SentryFileManager.createDirectory(atPath: "a") @@ -634,6 +643,17 @@ class SentryFileManagerTests: XCTestCase { XCTFail("Failed to store garbage in Envelopes folder.") } } + + private func givenOldEnvelopes() throws { + let envelope = TestConstants.envelope + let path = sut.store(envelope) + + let timeIntervalSince1970 = fixture.currentDateProvider.date().timeIntervalSince1970 - (90 * 24 * 60 * 60) + let date = Date(timeIntervalSince1970: timeIntervalSince1970 - 1) + try FileManager.default.setAttributes([FileAttributeKey.creationDate: date], ofItemAtPath: path) + + XCTAssertEqual(sut.getAllEnvelopes().count, 1) + } private func storeEvent() { do {