Fix a Core Data concurrency issue when CrashLogging is called from a …

…background thread (wordpress-mobile#20846)
jeff4a501 · Jun 19, 2023 · 58297cb · 58297cb
1 parent 76d9a4c
commit 58297cb
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 6 deletions.
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -11,6 +11,7 @@
 * [**] Block editor: [iOS] Fix dictation regression, in which typing/dictating at the same time caused content loss. [https://github.com/WordPress/gutenberg/pull/49452]
 * [*] Block editor: Display lock icon in disabled state of `Cell` component [https://github.com/wordpress-mobile/gutenberg-mobile/pull/5798]
 * [*] Block editor: Show "No title"/"No description" placeholder for not belonged videos in VideoPress block [https://github.com/wordpress-mobile/gutenberg-mobile/pull/5840]
+* [*] Resolve an issue that was causing the app crash when `CrashLogging.logError` is called from a background thread. [#20846]
 
 22.5
 -----

diff --git a/WordPress/Classes/Utility/Logging/WPCrashLoggingProvider.swift b/WordPress/Classes/Utility/Logging/WPCrashLoggingProvider.swift
@@ -35,6 +35,12 @@ struct WPLoggingStack {
 }
 
 struct WPCrashLoggingDataProvider: CrashLoggingDataProvider {
+    private let contextManager: ContextManager
+
+    init(contextManager: ContextManager = .shared) {
+        self.contextManager = contextManager
+    }
+
     let sentryDSN: String = ApiCredentials.sentryDSN
 
     var userHasOptedOut: Bool {
@@ -48,12 +54,11 @@ struct WPCrashLoggingDataProvider: CrashLoggingDataProvider {
     }
 
     var currentUser: TracksUser? {
-        let context = ContextManager.sharedInstance().mainContext
-
-        guard let account = try? WPAccount.lookupDefaultWordPressComAccount(in: context) else {
-            return nil
+        return contextManager.performQuery { context -> TracksUser? in
+            guard let account = try? WPAccount.lookupDefaultWordPressComAccount(in: context) else {
+                return nil
+            }
+            return TracksUser(userID: account.userID.stringValue, email: account.email, username: account.username)
         }
-
-        return TracksUser(userID: account.userID.stringValue, email: account.email, username: account.username)
     }
 }
diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj
@@ -3563,6 +3563,7 @@
 		F41E4EEF28F247D3001880C6 /* [email protected] in Resources */ = {isa = PBXBuildFile; fileRef = F41E4EEA28F247D3001880C6 /* [email protected] */; };
 		F41E4EF028F247D3001880C6 /* [email protected] in Resources */ = {isa = PBXBuildFile; fileRef = F41E4EEB28F247D3001880C6 /* [email protected] */; };
 		F42A1D9729928B360059CC70 /* BlockedAuthor.swift in Sources */ = {isa = PBXBuildFile; fileRef = F42A1D9629928B360059CC70 /* BlockedAuthor.swift */; };
+		F4394D1F2A3AB06F003955C6 /* WPCrashLoggingDataProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4394D1E2A3AB06F003955C6 /* WPCrashLoggingDataProviderTests.swift */; };
 		F4426FD3287E08C300218003 /* SuggestionServiceMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4426FD2287E08C300218003 /* SuggestionServiceMock.swift */; };
 		F4426FD9287F02FD00218003 /* SiteSuggestionsServiceMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4426FD8287F02FD00218003 /* SiteSuggestionsServiceMock.swift */; };
 		F4426FDB287F066400218003 /* site-suggestions.json in Resources */ = {isa = PBXBuildFile; fileRef = F4426FDA287F066400218003 /* site-suggestions.json */; };
@@ -8963,6 +8964,7 @@
 		F41E4EEB28F247D3001880C6 /* [email protected] */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "[email protected]"; sourceTree = "<group>"; };
 		F42A1D9629928B360059CC70 /* BlockedAuthor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlockedAuthor.swift; sourceTree = "<group>"; };
 		F432964A287752690089C4F7 /* WordPress 144.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "WordPress 144.xcdatamodel"; sourceTree = "<group>"; };
+		F4394D1E2A3AB06F003955C6 /* WPCrashLoggingDataProviderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WPCrashLoggingDataProviderTests.swift; sourceTree = "<group>"; };
 		F4426FD2287E08C300218003 /* SuggestionServiceMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SuggestionServiceMock.swift; sourceTree = "<group>"; };
 		F4426FD8287F02FD00218003 /* SiteSuggestionsServiceMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SiteSuggestionsServiceMock.swift; sourceTree = "<group>"; };
 		F4426FDA287F066400218003 /* site-suggestions.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "site-suggestions.json"; sourceTree = "<group>"; };
@@ -13386,6 +13388,7 @@
 		852416D01A12ED2D0030700C /* Utility */ = {
 			isa = PBXGroup;
 			children = (
+				F4394D202A3B6F43003955C6 /* Crash Logging */,
 				17AF92241C46634000A99CFB /* BlogSiteVisibilityHelperTest.m */,
 				93A379EB19FFBF7900415023 /* KeychainTest.m */,
 				5948AD101AB73D19006E8882 /* WPAppAnalyticsTests.m */,
@@ -17144,6 +17147,14 @@
 			path = "white-on-green";
 			sourceTree = "<group>";
 		};
+		F4394D202A3B6F43003955C6 /* Crash Logging */ = {
+			isa = PBXGroup;
+			children = (
+				F4394D1E2A3AB06F003955C6 /* WPCrashLoggingDataProviderTests.swift */,
+			);
+			name = "Crash Logging";
+			sourceTree = "<group>";
+		};
 		F44293D428E3B39400D340AF /* App Icons */ = {
 			isa = PBXGroup;
 			children = (
@@ -23497,6 +23508,7 @@
 				08A4E12F289D2795001D9EC7 /* UserPersistentStoreTests.swift in Sources */,
 				436D55F5211632B700CEAA33 /* RegisterDomainDetailsViewModelTests.swift in Sources */,
 				E180BD4C1FB462FF00D0D781 /* CookieJarTests.swift in Sources */,
+				F4394D1F2A3AB06F003955C6 /* WPCrashLoggingDataProviderTests.swift in Sources */,
 				9813512E22F0FC2700F7425D /* FileDownloadsStatsRecordValueTests.swift in Sources */,
 				9363113F19FA996700B0C739 /* AccountServiceTests.swift in Sources */,
 				17FC0032264D728E00FCBD37 /* SharingServiceTests.swift in Sources */,

diff --git a/WordPress/WordPressTest/WPCrashLoggingDataProviderTests.swift b/WordPress/WordPressTest/WPCrashLoggingDataProviderTests.swift
@@ -0,0 +1,62 @@
+import XCTest
+import AutomatticTracks
+
+@testable import WordPress
+
+final class WPCrashLoggingDataProviderTests: XCTestCase {
+
+    // MARK: - Testing Log Error
+
+    func testReadingTrackUserInMainThread() throws {
+        // Given
+        let dataProvider = self.makeCrashLoggingDataProvider()
+
+        // When
+        let user = try XCTUnwrap(dataProvider.currentUser)
+
+        // Then
+        XCTAssertEqual(user.userID, "\(Constants.defaultAccountID)")
+        XCTAssertEqual(user.username, Constants.defaultAccountUsername)
+        XCTAssertEqual(user.email, Constants.defaultAccountEmail)
+    }
+
+    func testReadingTrackUserInBackgroundThread() async {
+        let dataProvider = self.makeCrashLoggingDataProvider()
+        await withThrowingTaskGroup(of: Void.self) { group in
+            for _ in 1...1000 {
+                group.addTask {
+                    let user = try XCTUnwrap(dataProvider.currentUser)
+                    XCTAssertEqual(user.userID, "\(Constants.defaultAccountID)")
+                    XCTAssertEqual(user.username, Constants.defaultAccountUsername)
+                    XCTAssertEqual(user.email, Constants.defaultAccountEmail)
+                }
+            }
+        }
+    }
+
+    // MARK: - Helpers
+
+    private func makeCrashLoggingDataProvider() -> WPCrashLoggingDataProvider {
+        let provider = WPCrashLoggingDataProvider(contextManager: makeCoreDataStack())
+        return provider
+    }
+
+    private func makeCoreDataStack() -> ContextManager {
+        let contextManager = ContextManager.forTesting()
+        let account = AccountBuilder(contextManager)
+            .with(id: Constants.defaultAccountID)
+            .with(email: Constants.defaultAccountEmail)
+            .with(username: Constants.defaultAccountUsername)
+            .build()
+        UserSettings.defaultDotComUUID = account.uuid
+        return contextManager
+    }
+
+    // MARK: - Constants
+
+    private enum Constants {
+        static let defaultAccountID: Int64 = 123
+        static let defaultAccountEmail: String = "[email protected]"
+        static let defaultAccountUsername: String = "foobar"
+    }
+}