From c471221c18dd94f8d4f6cd3f80cd1b7d472c3f05 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 7 Nov 2023 13:50:23 -0900 Subject: [PATCH] ref: move SentryCrash singleton to SentryDependencyContainer (#3247) --- SentryTestUtils/ClearTestState.swift | 2 ++ .../SentryTestUtils-ObjC-BridgingHeader.h | 1 + .../Sentry/SentryCrashExceptionApplication.m | 6 ++-- Sources/Sentry/SentryCrashIntegration.m | 2 +- Sources/Sentry/SentryCrashWrapper.m | 8 +++-- Sources/Sentry/SentryDependencyContainer.m | 13 +++++++ Sources/Sentry/SentrySDK.m | 2 +- .../HybridPublic/SentryDependencyContainer.h | 2 ++ .../Installations/SentryCrashInstallation.m | 9 ++--- .../SentryCrashMonitor_MachException.c | 12 +++---- .../Monitors/SentryCrashMonitor_NSException.m | 4 ++- Sources/SentryCrash/Recording/SentryCrash.h | 4 --- Sources/SentryCrash/Recording/SentryCrash.m | 9 ----- .../SentryCrashIntegrationTests.swift | 12 +++---- .../SentryCrash/SentryCrashReportTests.swift | 2 +- .../SentryCrashScopeObserverTests.swift | 10 ++---- ...SentryCrashInstallationReporterTests.swift | 1 + .../SentryCrashInstallationTests.m | 34 +++++++++++-------- Tests/SentryTests/SentrySDKTests.swift | 2 +- 19 files changed, 73 insertions(+), 62 deletions(-) diff --git a/SentryTestUtils/ClearTestState.swift b/SentryTestUtils/ClearTestState.swift index 7196150e305..a550449b161 100644 --- a/SentryTestUtils/ClearTestState.swift +++ b/SentryTestUtils/ClearTestState.swift @@ -48,5 +48,7 @@ class TestCleanup: NSObject { PrivateSentrySDKOnly.onAppStartMeasurementAvailable = nil SentrySDK.setAppStartMeasurement(nil) #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) + + sentrycrash_scopesync_reset() } } diff --git a/SentryTestUtils/SentryTestUtils-ObjC-BridgingHeader.h b/SentryTestUtils/SentryTestUtils-ObjC-BridgingHeader.h index 4b12beb7013..81b52766f33 100644 --- a/SentryTestUtils/SentryTestUtils-ObjC-BridgingHeader.h +++ b/SentryTestUtils/SentryTestUtils-ObjC-BridgingHeader.h @@ -42,6 +42,7 @@ #import "SentryReachability.h" #import "SentrySDK+Private.h" #import "SentrySDK+Tests.h" +#import "SentryScopeSyncC.h" #import "SentrySession.h" #import "SentrySwizzleWrapper.h" #import "SentrySystemWrapper.h" diff --git a/Sources/Sentry/SentryCrashExceptionApplication.m b/Sources/Sentry/SentryCrashExceptionApplication.m index abaaff4f414..c1da077747f 100644 --- a/Sources/Sentry/SentryCrashExceptionApplication.m +++ b/Sources/Sentry/SentryCrashExceptionApplication.m @@ -1,5 +1,6 @@ #import "SentryCrashExceptionApplication.h" #import "SentryCrash.h" +#import "SentryDependencyContainer.h" #import "SentrySDK.h" @implementation SentryCrashExceptionApplication @@ -10,8 +11,9 @@ - (void)reportException:(NSException *)exception { [[NSUserDefaults standardUserDefaults] registerDefaults:@{ @"NSApplicationCrashOnExceptions" : @YES }]; - if (nil != SentryCrash.sharedInstance.uncaughtExceptionHandler && nil != exception) { - SentryCrash.sharedInstance.uncaughtExceptionHandler(exception); + SentryCrash *crash = SentryDependencyContainer.sharedInstance.crashReporter; + if (nil != crash.uncaughtExceptionHandler && nil != exception) { + crash.uncaughtExceptionHandler(exception); } [super reportException:exception]; } diff --git a/Sources/Sentry/SentryCrashIntegration.m b/Sources/Sentry/SentryCrashIntegration.m index 5f731c39d60..84a907f5148 100644 --- a/Sources/Sentry/SentryCrashIntegration.m +++ b/Sources/Sentry/SentryCrashIntegration.m @@ -185,7 +185,7 @@ - (void)configureScope userInfo[@"release"] = self.options.releaseName; userInfo[@"dist"] = self.options.dist; - [SentryCrash.sharedInstance setUserInfo:userInfo]; + [SentryDependencyContainer.sharedInstance.crashReporter setUserInfo:userInfo]; [outerScope addObserver:self.scopeObserver]; }]; diff --git a/Sources/Sentry/SentryCrashWrapper.m b/Sources/Sentry/SentryCrashWrapper.m index 46da9fa046e..8f9da00e555 100644 --- a/Sources/Sentry/SentryCrashWrapper.m +++ b/Sources/Sentry/SentryCrashWrapper.m @@ -7,6 +7,7 @@ #import #import #import +#import #include NS_ASSUME_NONNULL_BEGIN @@ -23,7 +24,7 @@ + (instancetype)sharedInstance - (BOOL)crashedLastLaunch { - return SentryCrash.sharedInstance.crashedLastLaunch; + return SentryDependencyContainer.sharedInstance.crashReporter.crashedLastLaunch; } - (NSTimeInterval)durationFromCrashStateInitToLastCrash @@ -33,7 +34,7 @@ - (NSTimeInterval)durationFromCrashStateInitToLastCrash - (NSTimeInterval)activeDurationSinceLastCrash { - return SentryCrash.sharedInstance.activeDurationSinceLastCrash; + return SentryDependencyContainer.sharedInstance.crashReporter.activeDurationSinceLastCrash; } - (BOOL)isBeingTraced @@ -55,7 +56,8 @@ - (NSDictionary *)systemInfo { static NSDictionary *sharedInfo = nil; static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ sharedInfo = SentryCrash.sharedInstance.systemInfo; }); + dispatch_once(&onceToken, + ^{ sharedInfo = SentryDependencyContainer.sharedInstance.crashReporter.systemInfo; }); return sharedInfo; } diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m index 768771f3672..2c528d52af7 100644 --- a/Sources/Sentry/SentryDependencyContainer.m +++ b/Sources/Sentry/SentryDependencyContainer.m @@ -15,6 +15,7 @@ #import "SentryUIDeviceWrapper.h" #import #import +#import #import #import #import @@ -121,6 +122,18 @@ - (SentryCrashWrapper *)crashWrapper return _crashWrapper; } +- (SentryCrash *)crashReporter +{ + if (_crashReporter == nil) { + @synchronized(sentryDependencyContainerLock) { + if (_crashReporter == nil) { + _crashReporter = [[SentryCrash alloc] init]; + } + } + } + return _crashReporter; +} + - (SentrySysctl *)sysctlWrapper { if (_sysctlWrapper == nil) { diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index 20b57746422..aae81f632c1 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -338,7 +338,7 @@ + (void)setUser:(SentryUser *_Nullable)user + (BOOL)crashedLastRun { - return SentryCrash.sharedInstance.crashedLastLaunch; + return SentryDependencyContainer.sharedInstance.crashReporter.crashedLastLaunch; } + (void)startSession diff --git a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h index c8867893ece..e3c5a03b593 100644 --- a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h +++ b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h @@ -3,6 +3,7 @@ @class SentryANRTracker; @class SentryAppStateManager; @class SentryBinaryImageCache; +@class SentryCrash; @class SentryCrashWrapper; @class SentryCurrentDateProvider; @class SentryDebugImageProvider; @@ -54,6 +55,7 @@ SENTRY_NO_INIT @property (nonatomic, strong) SentryFileManager *fileManager; @property (nonatomic, strong) SentryAppStateManager *appStateManager; @property (nonatomic, strong) SentryCrashWrapper *crashWrapper; +@property (nonatomic, strong) SentryCrash *crashReporter; @property (nonatomic, strong) SentryThreadWrapper *threadWrapper; @property (nonatomic, strong) id random; @property (nonatomic, strong) SentrySwizzleWrapper *swizzleWrapper; diff --git a/Sources/SentryCrash/Installations/SentryCrashInstallation.m b/Sources/SentryCrash/Installations/SentryCrashInstallation.m index 31f33665703..fed15ed60fb 100644 --- a/Sources/SentryCrash/Installations/SentryCrashInstallation.m +++ b/Sources/SentryCrash/Installations/SentryCrashInstallation.m @@ -33,6 +33,7 @@ #import "SentryCrashJSONCodecObjC.h" #import "SentryCrashLogger.h" #import "SentryCrashReportFilterBasic.h" +#import "SentryDependencyContainer.h" #import /** Max number of properties that can be defined for writing to the report */ @@ -175,7 +176,7 @@ - (id)initWithRequiredProperties:(NSArray *)requiredProperties - (void)dealloc { - SentryCrash *handler = [SentryCrash sharedInstance]; + SentryCrash *handler = SentryDependencyContainer.sharedInstance.crashReporter; @synchronized(handler) { if (g_crashHandlerData == self.crashHandlerData) { g_crashHandlerData = NULL; @@ -260,7 +261,7 @@ - (void)setOnCrash:(SentryCrashReportWriteCallback)onCrash - (void)install { - SentryCrash *handler = [SentryCrash sharedInstance]; + SentryCrash *handler = SentryDependencyContainer.sharedInstance.crashReporter; @synchronized(handler) { g_crashHandlerData = self.crashHandlerData; handler.onCrash = crashCallback; @@ -270,7 +271,7 @@ - (void)install - (void)uninstall { - SentryCrash *handler = [SentryCrash sharedInstance]; + SentryCrash *handler = SentryDependencyContainer.sharedInstance.crashReporter; @synchronized(handler) { if (g_crashHandlerData == self.crashHandlerData) { g_crashHandlerData = NULL; @@ -302,7 +303,7 @@ - (void)sendAllReportsWithCompletion:(SentryCrashReportFilterCompletion)onComple sink = [SentryCrashReportFilterPipeline filterWithFilters:self.prependedFilters, sink, nil]; - SentryCrash *handler = [SentryCrash sharedInstance]; + SentryCrash *handler = SentryDependencyContainer.sharedInstance.crashReporter; handler.sink = sink; [handler sendAllReportsWithCompletion:onCompletion]; } diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c index d5c7d40af70..5a3dced3258 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c @@ -51,9 +51,9 @@ # ifdef __LP64__ # define MACH_ERROR_CODE_MASK 0xFFFFFFFFFFFFFFFF -# else +# else // !__LP64__ # define MACH_ERROR_CODE_MASK 0xFFFFFFFF -# endif +# endif // __LP64__ // ============================================================================ # pragma mark - Types - @@ -267,7 +267,7 @@ sentrycrashcm_hasReservedThreads(void) return g_primaryMachThread != 0 && g_secondaryMachThread != 0; } -#else +#else // !SentryCrashCRASH_HAS_MACH bool sentrycrashcm_isReservedThread(thread_t thread) { @@ -280,7 +280,7 @@ sentrycrashcm_hasReservedThreads(void) return false; } -#endif +#endif // SentryCrashCRASH_HAS_MACH #if SentryCrashCRASH_HAS_MACH @@ -560,7 +560,7 @@ addContextualInfoToEvent(struct SentryCrash_MonitorContext *eventContext) } } -#endif +#endif // SentryCrashCRASH_HAS_MACH SentryCrashMonitorAPI * sentrycrashcm_machexception_getAPI(void) @@ -570,7 +570,7 @@ sentrycrashcm_machexception_getAPI(void) .setEnabled = setEnabled, .isEnabled = isEnabled, .addContextualInfoToEvent = addContextualInfoToEvent -#endif +#endif // SentryCrashCRASH_HAS_MACH }; return &api; } diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_NSException.m b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_NSException.m index 7da86622775..d0afabd0754 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_NSException.m +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_NSException.m @@ -31,6 +31,7 @@ #include "SentryCrashMonitorContext.h" #import "SentryCrashStackCursor_Backtrace.h" #include "SentryCrashThread.h" +#import "SentryDependencyContainer.h" // #define SentryCrashLogger_LocalLevel TRACE #import "SentryCrashLogger.h" @@ -128,7 +129,8 @@ SentryCrashLOG_DEBUG(@"Setting new handler."); NSSetUncaughtExceptionHandler(&handleUncaughtException); - SentryCrash.sharedInstance.uncaughtExceptionHandler = &handleUncaughtException; + SentryDependencyContainer.sharedInstance.crashReporter.uncaughtExceptionHandler + = &handleUncaughtException; } else { SentryCrashLOG_DEBUG(@"Restoring original handler."); NSSetUncaughtExceptionHandler(g_previousUncaughtExceptionHandler); diff --git a/Sources/SentryCrash/Recording/SentryCrash.h b/Sources/SentryCrash/Recording/SentryCrash.h index c7188e94e15..4621e15355e 100644 --- a/Sources/SentryCrash/Recording/SentryCrash.h +++ b/Sources/SentryCrash/Recording/SentryCrash.h @@ -182,10 +182,6 @@ static NSString *const SENTRYCRASH_REPORT_ATTACHMENTS_ITEM = @"attachments"; #pragma mark - API - -/** Get the singleton instance of the crash reporter. - */ -+ (SentryCrash *)sharedInstance; - /** Install the crash reporter. * The reporter will record crashes, but will not send any crash reports unless * sink is set. diff --git a/Sources/SentryCrash/Recording/SentryCrash.m b/Sources/SentryCrash/Recording/SentryCrash.m index fe878397a97..a8d176c5a62 100644 --- a/Sources/SentryCrash/Recording/SentryCrash.m +++ b/Sources/SentryCrash/Recording/SentryCrash.m @@ -89,15 +89,6 @@ @implementation SentryCrash #pragma mark - Lifecycle - // ============================================================================ -+ (instancetype)sharedInstance -{ - static SentryCrash *sharedInstance = nil; - static dispatch_once_t onceToken; - - dispatch_once(&onceToken, ^{ sharedInstance = [[SentryCrash alloc] init]; }); - return sharedInstance; -} - - (id)init { return [self initWithBasePath:[self getBasePath]]; diff --git a/Tests/SentryTests/Integrations/SentryCrash/SentryCrashIntegrationTests.swift b/Tests/SentryTests/Integrations/SentryCrash/SentryCrashIntegrationTests.swift index 57d93bc1109..c1793d71df0 100644 --- a/Tests/SentryTests/Integrations/SentryCrash/SentryCrashIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/SentryCrash/SentryCrashIntegrationTests.swift @@ -69,7 +69,7 @@ class SentryCrashIntegrationTests: NotificationCenterTestCase { } // Test for GH-581 - func testReleaseNamePassedToSentryCrash() { + func testReleaseNamePassedToSentryCrash() throws { let releaseName = "1.0.0" let dist = "14G60" // The start of the SDK installs all integrations @@ -80,20 +80,18 @@ class SentryCrashIntegrationTests: NotificationCenterTestCase { } // To test this properly we need SentryCrash and SentryCrashIntegration installed and registered on the current hub of the SDK. - - let instance = SentryCrash.sharedInstance() - let userInfo = (instance?.userInfo ?? ["": ""]) as Dictionary + + let userInfo = try XCTUnwrap(SentryDependencyContainer.sharedInstance().crashReporter.userInfo) assertUserInfoField(userInfo: userInfo, key: "release", expected: releaseName) assertUserInfoField(userInfo: userInfo, key: "dist", expected: dist) } - func testContext_IsPassedToSentryCrash() { + func testContext_IsPassedToSentryCrash() throws { SentrySDK.start { options in options.dsn = SentryCrashIntegrationTests.dsnAsString } - let instance = SentryCrash.sharedInstance() - let userInfo = (instance?.userInfo ?? ["": ""]) as Dictionary + let userInfo = try XCTUnwrap(SentryDependencyContainer.sharedInstance().crashReporter.userInfo) let context = userInfo["context"] as? [String: Any] assertContext(context: context) diff --git a/Tests/SentryTests/Integrations/SentryCrash/SentryCrashReportTests.swift b/Tests/SentryTests/Integrations/SentryCrash/SentryCrashReportTests.swift index d04655189f0..ca1ed7bbade 100644 --- a/Tests/SentryTests/Integrations/SentryCrash/SentryCrashReportTests.swift +++ b/Tests/SentryTests/Integrations/SentryCrash/SentryCrashReportTests.swift @@ -78,7 +78,7 @@ class SentryCrashReportTests: XCTestCase { * UserInfo is picked up by the crash report when writing a new report. */ private func serializeToCrashReport(scope: Scope) { - SentryCrash.sharedInstance().userInfo = scope.serialize() + SentryDependencyContainer.sharedInstance().crashReporter.userInfo = scope.serialize() } private func deleteTestDir() { diff --git a/Tests/SentryTests/Integrations/SentryCrash/SentryCrashScopeObserverTests.swift b/Tests/SentryTests/Integrations/SentryCrash/SentryCrashScopeObserverTests.swift index 7e670a61bbb..3424e534f05 100644 --- a/Tests/SentryTests/Integrations/SentryCrash/SentryCrashScopeObserverTests.swift +++ b/Tests/SentryTests/Integrations/SentryCrash/SentryCrashScopeObserverTests.swift @@ -1,3 +1,4 @@ +import SentryTestUtils import XCTest class SentryCrashScopeObserverTests: XCTestCase { @@ -17,16 +18,9 @@ class SentryCrashScopeObserverTests: XCTestCase { private let fixture = Fixture() - override func setUp() { - super.setUp() - sentrycrash_scopesync_reset() - SentryCrash.sharedInstance().userInfo = nil - } - override func tearDown() { super.tearDown() - sentrycrash_scopesync_reset() - SentryCrash.sharedInstance().userInfo = nil + clearTestState() } func testUser() { diff --git a/Tests/SentryTests/SentryCrash/SentryCrashInstallationReporterTests.swift b/Tests/SentryTests/SentryCrash/SentryCrashInstallationReporterTests.swift index 0aafcfdbc71..f1a25f7c92f 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashInstallationReporterTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryCrashInstallationReporterTests.swift @@ -21,6 +21,7 @@ class SentryCrashInstallationReporterTests: XCTestCase { super.tearDown() sentrycrash_deleteAllReports() clearTestState() + sut.uninstall() } func testReportIsSentAndDeleted() throws { diff --git a/Tests/SentryTests/SentryCrash/SentryCrashInstallationTests.m b/Tests/SentryTests/SentryCrash/SentryCrashInstallationTests.m index 503c56db03b..a37d986d66c 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashInstallationTests.m +++ b/Tests/SentryTests/SentryCrash/SentryCrashInstallationTests.m @@ -40,14 +40,7 @@ - (void)tearDown [SentryDependencyContainer reset]; } -- (SentryCrashTestInstallation *)getSut -{ - SentryCrashTestInstallation *installation = - [[SentryCrashTestInstallation alloc] initForTesting]; - self.notificationCenter = [[TestNSNotificationCenterWrapper alloc] init]; - [[SentryCrash sharedInstance] setSentryNSNotificationCenterWrapper:self.notificationCenter]; - return installation; -} +#pragma mark - Tests - (void)testUninstall { @@ -55,7 +48,8 @@ - (void)testUninstall [installation install]; - SentryCrashMonitorType monitorsAfterInstall = [SentryCrash sharedInstance].monitoring; + SentryCrashMonitorType monitorsAfterInstall + = SentryDependencyContainer.sharedInstance.crashReporter.monitoring; [installation uninstall]; @@ -80,8 +74,8 @@ - (void)testUninstall_Install [installation install]; - SentryCrash *sentryCrash = [SentryCrash sharedInstance]; - SentryCrashMonitorType monitorsAfterInstall = sentryCrash.monitoring; + SentryCrashMonitorType monitorsAfterInstall + = SentryDependencyContainer.sharedInstance.crashReporter.monitoring; CrashHandlerData *crashHandlerDataAfterInstall = [installation g_crashHandlerData]; // To ensure multiple calls in a row work @@ -104,14 +98,26 @@ - (void)testUninstall_Install #if SentryCrashCRASH_HAS_UIAPPLICATION XCTAssertEqual(55, self.notificationCenter.removeObserverWithNameInvocationsCount); -#endif +#endif // SentryCrashCRASH_HAS_UIAPPLICATION +} + +#pragma mark - Private + +- (SentryCrashTestInstallation *)getSut +{ + SentryCrashTestInstallation *installation = + [[SentryCrashTestInstallation alloc] initForTesting]; + self.notificationCenter = [[TestNSNotificationCenterWrapper alloc] init]; + [SentryDependencyContainer.sharedInstance.crashReporter + setSentryNSNotificationCenterWrapper:self.notificationCenter]; + return installation; } - (void)assertReinstalled:(SentryCrashTestInstallation *)installation monitorsAfterInstall:(SentryCrashMonitorType)monitorsAfterInstall crashHandlerDataAfterInstall:(CrashHandlerData *)crashHandlerDataAfterInstall { - SentryCrash *sentryCrash = [SentryCrash sharedInstance]; + SentryCrash *sentryCrash = SentryDependencyContainer.sharedInstance.crashReporter; XCTAssertNotEqual(NULL, [installation g_crashHandlerData]); XCTAssertEqual(monitorsAfterInstall, sentryCrash.monitoring); XCTAssertEqual(monitorsAfterInstall, sentrycrashcm_getActiveMonitors()); @@ -126,7 +132,7 @@ - (void)assertReinstalled:(SentryCrashTestInstallation *)installation - (void)assertUninstalled:(SentryCrashTestInstallation *)installation monitorsAfterInstall:(SentryCrashMonitorType)monitorsAfterInstall { - SentryCrash *sentryCrash = [SentryCrash sharedInstance]; + SentryCrash *sentryCrash = SentryDependencyContainer.sharedInstance.crashReporter; XCTAssertEqual(NULL, [installation g_crashHandlerData]); XCTAssertEqual(SentryCrashMonitorTypeNone, sentryCrash.monitoring); XCTAssertEqual(SentryCrashMonitorTypeNone, sentrycrashcm_getActiveMonitors()); diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index f57ce01a629..e88425bfff0 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -187,7 +187,7 @@ class SentrySDKTests: XCTestCase { } func testCrashedLastRun() { - XCTAssertEqual(SentryCrash.sharedInstance().crashedLastLaunch, SentrySDK.crashedLastRun) + XCTAssertEqual(SentryDependencyContainer.sharedInstance().crashReporter.crashedLastLaunch, SentrySDK.crashedLastRun) } func testCaptureCrashEvent() {