From 0814e17ad0a33bcc3788fd23ac45fb90f9baf93f Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Wed, 11 Mar 2026 11:47:03 +0100 Subject: [PATCH 1/2] fix(macos): Fix thread safety in NSException capture Replace static BOOL flag with C11 atomic to prevent race conditions when multiple threads throw exceptions simultaneously. The previous implementation could allow duplicate exception captures when _crashOnException: was called from within reportException:. Key changes: - Add SentryNSExceptionCaptureHelper with atomic flag - Swizzle both reportException: and _crashOnException: methods - Remove SentryUseNSExceptionCallstackWrapper (no longer needed) - Simplify exception handling flow The atomic flag ensures thread-safe deduplication: when reportException: captures an exception and calls [super reportException:], which internally dispatches to _crashOnException:, the second call is skipped to avoid duplicate reports. --- Sentry.xcodeproj/project.pbxproj | 24 ++-- Sources/Sentry/SentryClient.m | 9 -- .../Sentry/SentryCrashExceptionApplication.m | 14 ++- .../SentryCrashExceptionApplicationHelper.m | 32 ------ .../Sentry/SentryNSExceptionCaptureHelper.m | 47 ++++++++ Sources/Sentry/SentrySDKInternal.m | 17 --- Sources/Sentry/SentryUncaughtNSExceptions.m | 67 ++++++++--- .../SentryUseNSExceptionCallstackWrapper.m | 74 ------------ .../SentryCrashExceptionApplicationHelper.h | 9 -- .../include/SentryNSExceptionCaptureHelper.h | 19 ++++ Sources/Sentry/include/SentryPrivate.h | 2 +- Sources/Sentry/include/SentrySDK+Private.h | 12 -- .../include/SentryUncaughtNSExceptions.h | 4 +- .../SentryUseNSExceptionCallstackWrapper.h | 24 ---- .../SentryCrash/SentryCrashIntegration.swift | 1 + ...yUseNSExceptionCallstackWrapperTests.swift | 63 ----------- .../SentryUncaughtNSExceptionsTests.swift | 84 +++++++++++--- Tests/SentryTests/SentryClientTests.swift | 103 +++++++---------- ...SentryCrashExceptionApplicationTests.swift | 106 +++++++++++------- .../SentryTests/SentrySDKInternalTests.swift | 23 +--- .../SentryTests/SentryTests-Bridging-Header.h | 1 - 21 files changed, 321 insertions(+), 414 deletions(-) delete mode 100644 Sources/Sentry/SentryCrashExceptionApplicationHelper.m create mode 100644 Sources/Sentry/SentryNSExceptionCaptureHelper.m delete mode 100644 Sources/Sentry/SentryUseNSExceptionCallstackWrapper.m delete mode 100644 Sources/Sentry/include/SentryCrashExceptionApplicationHelper.h create mode 100644 Sources/Sentry/include/SentryNSExceptionCaptureHelper.h delete mode 100644 Sources/Sentry/include/SentryUseNSExceptionCallstackWrapper.h delete mode 100644 Tests/SentryTests/Helper/SentryUseNSExceptionCallstackWrapperTests.swift diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index cb547207e23..e585b3862b1 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -453,15 +453,13 @@ F44858132E03579D0013E63B /* SentryCrashDynamicLinker+Test.h in Headers */ = {isa = PBXBuildFile; fileRef = F44858122E0357940013E63B /* SentryCrashDynamicLinker+Test.h */; }; F44D2B5C2E6B7E8700FF31FA /* SentryProfilingScreenFramesHelper.h in Headers */ = {isa = PBXBuildFile; fileRef = F44D2B5A2E6B7E8700FF31FA /* SentryProfilingScreenFramesHelper.h */; }; F44D2B5D2E6B7E8700FF31FA /* SentryProfilingScreenFramesHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = F44D2B5B2E6B7E8700FF31FA /* SentryProfilingScreenFramesHelper.m */; }; - F452437E2DE60B71003E8F50 /* SentryUseNSExceptionCallstackWrapper.m in Sources */ = {isa = PBXBuildFile; fileRef = F452437D2DE60B71003E8F50 /* SentryUseNSExceptionCallstackWrapper.m */; }; - F452438C2DE65BC0003E8F50 /* SentryUseNSExceptionCallstackWrapper.h in Headers */ = {isa = PBXBuildFile; fileRef = F452438B2DE65BC0003E8F50 /* SentryUseNSExceptionCallstackWrapper.h */; }; F459B01D2F2AB1480096AA8B /* SentryAppStartTrackerHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = F459B01C2F2AB1480096AA8B /* SentryAppStartTrackerHelper.m */; }; F459B0242F2AB1560096AA8B /* SentryAppStartTrackerHelper.h in Headers */ = {isa = PBXBuildFile; fileRef = F459B0232F2AB1560096AA8B /* SentryAppStartTrackerHelper.h */; }; F485E2832F293E1A00B66F52 /* SentryUIViewControllerSwizzlingHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = F485E2822F293E1A00B66F52 /* SentryUIViewControllerSwizzlingHelper.m */; }; F485E28A2F293E2900B66F52 /* SentryUIViewControllerSwizzlingHelper.h in Headers */ = {isa = PBXBuildFile; fileRef = F485E2892F293E2900B66F52 /* SentryUIViewControllerSwizzlingHelper.h */; }; F48F21F52EF1F05000E33FC1 /* SentryAppStartMeasurement+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = F48F21F42EF1F04C00E33FC1 /* SentryAppStartMeasurement+Private.h */; }; - F49D419C2DEA30C300D9244E /* SentryCrashExceptionApplicationHelper.h in Headers */ = {isa = PBXBuildFile; fileRef = F49D419B2DEA30B800D9244E /* SentryCrashExceptionApplicationHelper.h */; }; - F49D419E2DEA3D0600D9244E /* SentryCrashExceptionApplicationHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = F49D419D2DEA3D0300D9244E /* SentryCrashExceptionApplicationHelper.m */; }; + F49D419C2DEA30C300D9244E /* SentryNSExceptionCaptureHelper.h in Headers */ = {isa = PBXBuildFile; fileRef = F49D419B2DEA30B800D9244E /* SentryNSExceptionCaptureHelper.h */; }; + F49D419E2DEA3D0600D9244E /* SentryNSExceptionCaptureHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = F49D419D2DEA3D0300D9244E /* SentryNSExceptionCaptureHelper.m */; }; F4F33D4C2F1F35BA00753FE2 /* SentryNSDataSwizzlingHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = F4F33D4A2F1F35BA00753FE2 /* SentryNSDataSwizzlingHelper.m */; }; F4F33D4D2F1F35BA00753FE2 /* SentryNSFileManagerSwizzlingHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = F4F33D4B2F1F35BA00753FE2 /* SentryNSFileManagerSwizzlingHelper.m */; }; FA24E02A2EBBF6F900EFD92E /* SentryOptionsObjC.h in Headers */ = {isa = PBXBuildFile; fileRef = FA24E0292EBBF6F500EFD92E /* SentryOptionsObjC.h */; }; @@ -1115,15 +1113,13 @@ F44858122E0357940013E63B /* SentryCrashDynamicLinker+Test.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "SentryCrashDynamicLinker+Test.h"; sourceTree = ""; }; F44D2B5A2E6B7E8700FF31FA /* SentryProfilingScreenFramesHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SentryProfilingScreenFramesHelper.h; sourceTree = ""; }; F44D2B5B2E6B7E8700FF31FA /* SentryProfilingScreenFramesHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryProfilingScreenFramesHelper.m; sourceTree = ""; }; - F452437D2DE60B71003E8F50 /* SentryUseNSExceptionCallstackWrapper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryUseNSExceptionCallstackWrapper.m; sourceTree = ""; }; - F452438B2DE65BC0003E8F50 /* SentryUseNSExceptionCallstackWrapper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryUseNSExceptionCallstackWrapper.h; path = include/SentryUseNSExceptionCallstackWrapper.h; sourceTree = ""; }; F459B01C2F2AB1480096AA8B /* SentryAppStartTrackerHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryAppStartTrackerHelper.m; sourceTree = ""; }; F459B0232F2AB1560096AA8B /* SentryAppStartTrackerHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryAppStartTrackerHelper.h; path = include/SentryAppStartTrackerHelper.h; sourceTree = ""; }; F485E2822F293E1A00B66F52 /* SentryUIViewControllerSwizzlingHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryUIViewControllerSwizzlingHelper.m; sourceTree = ""; }; F485E2892F293E2900B66F52 /* SentryUIViewControllerSwizzlingHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryUIViewControllerSwizzlingHelper.h; path = include/SentryUIViewControllerSwizzlingHelper.h; sourceTree = ""; }; F48F21F42EF1F04C00E33FC1 /* SentryAppStartMeasurement+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "SentryAppStartMeasurement+Private.h"; path = "include/SentryAppStartMeasurement+Private.h"; sourceTree = ""; }; - F49D419B2DEA30B800D9244E /* SentryCrashExceptionApplicationHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryCrashExceptionApplicationHelper.h; path = include/SentryCrashExceptionApplicationHelper.h; sourceTree = ""; }; - F49D419D2DEA3D0300D9244E /* SentryCrashExceptionApplicationHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryCrashExceptionApplicationHelper.m; sourceTree = ""; }; + F49D419B2DEA30B800D9244E /* SentryNSExceptionCaptureHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryNSExceptionCaptureHelper.h; path = include/SentryNSExceptionCaptureHelper.h; sourceTree = ""; }; + F49D419D2DEA3D0300D9244E /* SentryNSExceptionCaptureHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryNSExceptionCaptureHelper.m; sourceTree = ""; }; F4F33D4A2F1F35BA00753FE2 /* SentryNSDataSwizzlingHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryNSDataSwizzlingHelper.m; sourceTree = ""; }; F4F33D4B2F1F35BA00753FE2 /* SentryNSFileManagerSwizzlingHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryNSFileManagerSwizzlingHelper.m; sourceTree = ""; }; F4F33D4E2F1F35C500753FE2 /* SentryNSDataSwizzlingHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryNSDataSwizzlingHelper.h; path = include/SentryNSDataSwizzlingHelper.h; sourceTree = ""; }; @@ -1401,8 +1397,8 @@ children = ( 632F434E1F581D5400A18A36 /* SentryCrashExceptionApplication.h */, 632F434F1F581D5400A18A36 /* SentryCrashExceptionApplication.m */, - F49D419B2DEA30B800D9244E /* SentryCrashExceptionApplicationHelper.h */, - F49D419D2DEA3D0300D9244E /* SentryCrashExceptionApplicationHelper.m */, + F49D419B2DEA30B800D9244E /* SentryNSExceptionCaptureHelper.h */, + F49D419D2DEA3D0300D9244E /* SentryNSExceptionCaptureHelper.m */, 6344DDB21EC309E000D9160D /* SentryCrashReportSink.h */, 6344DDB31EC309E000D9160D /* SentryCrashReportSink.m */, 6344DDB71EC3115C00D9160D /* SentryCrashReportConverter.h */, @@ -1505,8 +1501,6 @@ 844EDC75294144DB00C86F34 /* SentrySystemWrapper.mm */, FAB359972E05D7E90083D5E3 /* SentryEventSwiftHelper.h */, FAB359992E05D8080083D5E3 /* SentryEventSwiftHelper.m */, - F452438B2DE65BC0003E8F50 /* SentryUseNSExceptionCallstackWrapper.h */, - F452437D2DE60B71003E8F50 /* SentryUseNSExceptionCallstackWrapper.m */, ); name = Helper; sourceTree = ""; @@ -2366,7 +2360,7 @@ 84AF45A629A7FFA500FBB177 /* SentryProfiledTracerConcurrency.h in Headers */, 8EAE980B261E9F530073B6B3 /* SentryPerformanceTracker.h in Headers */, 63FE718520DA4C1100CDBAE8 /* SentryCrashC.h in Headers */, - F49D419C2DEA30C300D9244E /* SentryCrashExceptionApplicationHelper.h in Headers */, + F49D419C2DEA30C300D9244E /* SentryNSExceptionCaptureHelper.h in Headers */, A8AFFCCD29069C3E00967CD7 /* SentryHttpStatusCodeRange.h in Headers */, FAAB95DE2EA1EB470030A2DB /* SentryDeviceContextKeys.h in Headers */, D83D079B2B7F9D1C00CC9674 /* SentryMsgPackSerializer.h in Headers */, @@ -2486,7 +2480,6 @@ 63FE716F20DA4C1100CDBAE8 /* SentryCrashCPU_Apple.h in Headers */, 639FCFA81EBC80CC00778193 /* SentryFrame.h in Headers */, D8BFE37229A3782F002E73F3 /* SentryTimeToDisplayTracker.h in Headers */, - F452438C2DE65BC0003E8F50 /* SentryUseNSExceptionCallstackWrapper.h in Headers */, 63FE716D20DA4C1100CDBAE8 /* SentryCrashSysCtl.h in Headers */, 639889BB1EDED18400EA7442 /* SentrySwizzle.h in Headers */, FA7206DF2E0B37850072FDD4 /* SentryProfileCollector.h in Headers */, @@ -2900,7 +2893,7 @@ 63FE712F20DA4C1100CDBAE8 /* SentryCrashSysCtl.c in Sources */, 7B3B473825D6CC7E00D01640 /* SentryNSError.m in Sources */, 621AE74D2C626C510012E730 /* SentryANRTrackerV2.m in Sources */, - F49D419E2DEA3D0600D9244E /* SentryCrashExceptionApplicationHelper.m in Sources */, + F49D419E2DEA3D0600D9244E /* SentryNSExceptionCaptureHelper.m in Sources */, D8ACE3C82762187200F5A213 /* SentryFileIOTrackerHelper.m in Sources */, D8B088B729C9E3FF00213258 /* SentryTracerConfiguration.m in Sources */, FA7206E12E0B37C80072FDD4 /* SentryProfileCollector.mm in Sources */, @@ -3000,7 +2993,6 @@ 62C316832B1F2EA1000D7031 /* SentryDelayedFramesTracker.m in Sources */, D8BFE37329A3782F002E73F3 /* SentryTimeToDisplayTracker.m in Sources */, 6334314320AD9AE40077E581 /* SentryMechanism.m in Sources */, - F452437E2DE60B71003E8F50 /* SentryUseNSExceptionCallstackWrapper.m in Sources */, 63FE70D320DA4C1000CDBAE8 /* SentryCrashMonitor_AppState.c in Sources */, 639FCFA51EBC809A00778193 /* SentryStacktrace.m in Sources */, 84A898542E163072009A551E /* SentryProfileConfiguration.m in Sources */, diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index cbf76edae51..41081d2c41c 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -30,7 +30,6 @@ #import "SentryTransport.h" #import "SentryTransportAdapter.h" #import "SentryTransportFactory.h" -#import "SentryUseNSExceptionCallstackWrapper.h" #import "SentryUser.h" #if SENTRY_HAS_UIKIT @@ -184,14 +183,6 @@ - (SentryEvent *)buildExceptionEvent:(NSException *)exception event.exceptions = @[ sentryException ]; -#if TARGET_OS_OSX - // When a exception class is SentryUseNSExceptionCallstackWrapper, we should use the thread from - // it - if ([exception isKindOfClass:[SentryUseNSExceptionCallstackWrapper class]]) { - event.threads = [(SentryUseNSExceptionCallstackWrapper *)exception buildThreads]; - } -#endif - [self setUserInfo:exception.userInfo withEvent:event]; return event; } diff --git a/Sources/Sentry/SentryCrashExceptionApplication.m b/Sources/Sentry/SentryCrashExceptionApplication.m index 3e66e0428b0..edc37e6b952 100644 --- a/Sources/Sentry/SentryCrashExceptionApplication.m +++ b/Sources/Sentry/SentryCrashExceptionApplication.m @@ -3,8 +3,14 @@ #if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK # import "SentryCrashExceptionApplication.h" -# import "SentryCrashExceptionApplicationHelper.h" +# import "SentryNSExceptionCaptureHelper.h" # import "SentryUncaughtNSExceptions.h" +# import + +// Private AppKit method called on the application instance during CATransaction flush. +@interface NSApplication (SentryCrashOnException) +- (void)_crashOnException:(NSException *)exception; +@end @implementation SentryCrashExceptionApplication @@ -13,13 +19,15 @@ - (void)reportException:(NSException *)exception [SentryUncaughtNSExceptions configureCrashOnExceptions]; // We cannot test an NSApplication because you create more than one at a time, so we use a // helper to hold the logic. - [SentryCrashExceptionApplicationHelper reportException:exception]; + [SentryNSExceptionCaptureHelper reportException:exception]; [super reportException:exception]; + [SentryNSExceptionCaptureHelper reportExceptionDidFinish]; } - (void)_crashOnException:(NSException *)exception { - [SentryCrashExceptionApplicationHelper _crashOnException:exception]; + [SentryNSExceptionCaptureHelper crashOnException:exception]; + [super _crashOnException:exception]; } @end diff --git a/Sources/Sentry/SentryCrashExceptionApplicationHelper.m b/Sources/Sentry/SentryCrashExceptionApplicationHelper.m deleted file mode 100644 index 669dff2fcb7..00000000000 --- a/Sources/Sentry/SentryCrashExceptionApplicationHelper.m +++ /dev/null @@ -1,32 +0,0 @@ -#import - -#if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK - -# import "SentryCrash.h" -# import "SentryCrashExceptionApplication.h" -# import "SentryCrashExceptionApplicationHelper.h" -# import "SentrySDK+Private.h" -# import "SentrySDKInternal.h" -# import "SentrySwift.h" - -@implementation SentryCrashExceptionApplicationHelper - -+ (void)reportException:(NSException *)exception -{ - SentryCrashSwift *crash = SentryDependencyContainer.sharedInstance.crashReporter; - if (nil != crash.uncaughtExceptionHandler && nil != exception) { - crash.uncaughtExceptionHandler(exception); - } -} - -+ (void)_crashOnException:(NSException *)exception -{ - [SentrySDKInternal captureCrashOnException:exception]; -# if !(SENTRY_TEST || SENTRY_TEST_CI) - abort(); -# endif -} - -@end - -#endif // TARGET_OS_OSX diff --git a/Sources/Sentry/SentryNSExceptionCaptureHelper.m b/Sources/Sentry/SentryNSExceptionCaptureHelper.m new file mode 100644 index 00000000000..1cc717a9d9b --- /dev/null +++ b/Sources/Sentry/SentryNSExceptionCaptureHelper.m @@ -0,0 +1,47 @@ +#import + +#if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK + +# import "SentryCrash.h" +# import "SentryNSExceptionCaptureHelper.h" +# import "SentrySwift.h" +# import + +@implementation SentryNSExceptionCaptureHelper + +// Thread-safe flag to prevent duplicate exception captures when +// _crashOnException: is called from within reportException: +static _Atomic BOOL _insideReportException = NO; + ++ (void)reportException:(NSException *)exception +{ + atomic_store(&_insideReportException, YES); + [self captureException:exception]; +} + ++ (void)reportExceptionDidFinish +{ + atomic_store(&_insideReportException, NO); +} + ++ (void)crashOnException:(NSException *)exception +{ + // When called from within reportException: (i.e., [super reportException:] internally + // dispatches to _crashOnException: when NSApplicationCrashOnExceptions is YES), + // the exception was already captured, so skip to avoid duplicate reports. + if (!atomic_load(&_insideReportException)) { + [self captureException:exception]; + } +} + ++ (void)captureException:(NSException *)exception +{ + SentryCrashSwift *crash = SentryDependencyContainer.sharedInstance.crashReporter; + if (nil != crash.uncaughtExceptionHandler && nil != exception) { + crash.uncaughtExceptionHandler(exception); + } +} + +@end + +#endif // TARGET_OS_OSX diff --git a/Sources/Sentry/SentrySDKInternal.m b/Sources/Sentry/SentrySDKInternal.m index a22468649e7..fdfe6a32b3b 100644 --- a/Sources/Sentry/SentrySDKInternal.m +++ b/Sources/Sentry/SentrySDKInternal.m @@ -17,7 +17,6 @@ #import "SentrySpanInternal.h" #import "SentrySwift.h" #import "SentryTransactionContext.h" -#import "SentryUseNSExceptionCallstackWrapper.h" #if TARGET_OS_OSX # import "SentryCrashExceptionApplication.h" @@ -358,22 +357,6 @@ + (SentryId *)captureException:(NSException *)exception withScope:(SentryScope * return [SentrySDKInternal.currentHub captureException:exception withScope:scope]; } -#if TARGET_OS_OSX - -+ (SentryId *)captureCrashOnException:(NSException *)exception -{ - SentryUseNSExceptionCallstackWrapper *wrappedException = - [[SentryUseNSExceptionCallstackWrapper alloc] - initWithName:exception.name - reason:exception.reason - userInfo:exception.userInfo - callStackReturnAddresses:exception.callStackReturnAddresses]; - return [SentrySDKInternal captureException:wrappedException - withScope:SentrySDKInternal.currentHub.scope]; -} - -#endif // TARGET_OS_OSX - + (SentryId *)captureMessage:(NSString *)message { return [SentrySDKInternal captureMessage:message withScope:SentrySDKInternal.currentHub.scope]; diff --git a/Sources/Sentry/SentryUncaughtNSExceptions.m b/Sources/Sentry/SentryUncaughtNSExceptions.m index 3d5dc8e59f5..f37e1d28cd1 100644 --- a/Sources/Sentry/SentryUncaughtNSExceptions.m +++ b/Sources/Sentry/SentryUncaughtNSExceptions.m @@ -2,9 +2,10 @@ #if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK -# import "SentryCrash.h" +# import + # import "SentryInternalDefines.h" -# import "SentrySwift.h" +# import "SentryNSExceptionCaptureHelper.h" # import "SentrySwizzle.h" # import "SentryUncaughtNSExceptions.h" # import @@ -26,26 +27,64 @@ + (void)swizzleNSApplicationReportException SEL selector = NSSelectorFromString(@"reportException:"); SentrySwizzleInstanceMethod(NSApplication, selector, SentrySWReturnType(void), SentrySWArguments(NSException * exception), SentrySWReplacement({ - [SentryUncaughtNSExceptions capture:exception]; - return SentrySWCallOriginal(exception); + [SentryNSExceptionCaptureHelper reportException:exception]; + SentrySWCallOriginal(exception); + [SentryNSExceptionCaptureHelper reportExceptionDidFinish]; }), SentrySwizzleModeOncePerClassAndSuperclasses, (void *)selector); # pragma clang diagnostic pop } -+ (void)capture:(nullable NSException *)exception ++ (void)swizzleNSApplicationCrashOnException { - SentryCrashSwift *crash = SentryDependencyContainer.sharedInstance.crashReporter; - - if (crash.uncaughtExceptionHandler == nil) { - return; - } + // SentrySwizzleClassMethod has no built-in deduplication, so guard against repeated SDK starts. + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wshadow" +# if SENTRY_TEST || SENTRY_TEST_CI +# pragma clang diagnostic ignored "-Wunused-variable" +# endif + // AppKit calls _crashOnException: when an exception is caught during CATransaction flush + // or view layout, bypassing reportException: entirely. Depending on macOS version, AppKit + // may call the class method +[NSApplication _crashOnException:] or the instance method + // -[NSApp _crashOnException:], so we swizzle both. + // + // SentryNSExceptionCaptureHelper handles deduplication: when _crashOnException: is + // called from within reportException: (e.g., when NSApplicationCrashOnExceptions is YES), + // the exception is only captured once. + SEL selector = NSSelectorFromString(@"_crashOnException:"); - if (exception == nil) { - return; - } + // _crashOnException: is a private AppKit method that may not exist on all macOS versions. + // Only swizzle if the method is actually present on the class/instance. + if (class_getClassMethod([NSApplication class], selector)) { + SentrySwizzleClassMethod(NSApplication, selector, SentrySWReturnType(void), + SentrySWArguments(NSException * exception), SentrySWReplacement({ + [SentryNSExceptionCaptureHelper crashOnException:exception]; +# if SENTRY_TEST || SENTRY_TEST_CI + // Don't call the original in tests as it would abort() the process. + swizzleInfo.originalCalled = YES; +# else + return SentrySWCallOriginal(exception); +# endif + })); + } - crash.uncaughtExceptionHandler(SENTRY_UNWRAP_NULLABLE(NSException, exception)); + if (class_getInstanceMethod([NSApplication class], selector)) { + SentrySwizzleInstanceMethod(NSApplication, selector, SentrySWReturnType(void), + SentrySWArguments(NSException * exception), SentrySWReplacement({ + [SentryNSExceptionCaptureHelper crashOnException:exception]; +# if SENTRY_TEST || SENTRY_TEST_CI + // Don't call the original in tests as it would abort() the process. + swizzleInfo.originalCalled = YES; +# else + return SentrySWCallOriginal(exception); +# endif + }), + SentrySwizzleModeOncePerClassAndSuperclasses, (void *)selector); + } +# pragma clang diagnostic pop + }); } @end diff --git a/Sources/Sentry/SentryUseNSExceptionCallstackWrapper.m b/Sources/Sentry/SentryUseNSExceptionCallstackWrapper.m deleted file mode 100644 index a37ce6d2f88..00000000000 --- a/Sources/Sentry/SentryUseNSExceptionCallstackWrapper.m +++ /dev/null @@ -1,74 +0,0 @@ -#import "SentryUseNSExceptionCallstackWrapper.h" -#import "SentryCrashStackEntryMapper.h" -#import "SentrySDK+Private.h" -#import "SentryStacktraceBuilder.h" -#import "SentrySwift.h" -#import "SentryThread.h" - -#if TARGET_OS_OSX - -@interface SentryUseNSExceptionCallstackWrapper () - -@property (nonatomic, strong) NSArray *returnAddressesArray; - -@end - -@implementation SentryUseNSExceptionCallstackWrapper - -- (instancetype)initWithName:(NSExceptionName)aName - reason:(NSString *_Nullable)aReason - userInfo:(NSDictionary *_Nullable)aUserInfo - callStackReturnAddresses:(NSArray *)callStackReturnAddresses -{ - if (self = [super initWithName:aName reason:aReason userInfo:aUserInfo]) { - self.returnAddressesArray = callStackReturnAddresses; - } - return self; -} - -- (NSArray *)buildThreads -{ - SentryThread *sentryThread = [[SentryThread alloc] initWithThreadId:@0]; - sentryThread.name = @"NSException Thread"; - sentryThread.crashed = @YES; - // This data might not be real, but we cannot collect other threads - sentryThread.current = @YES; - sentryThread.isMain = @YES; - - SentryCrashStackEntryMapper *crashStackToEntryMapper = [self buildCrashStackToEntryMapper]; - NSMutableArray *frames = [NSMutableArray array]; - - // Iterate over all the addresses and create a SentryFrame - [self.returnAddressesArray - enumerateObjectsUsingBlock:^(NSNumber *_Nonnull obj, NSUInteger idx, BOOL *_Nonnull stop) { - SentryCrashStackCursor stackCursor; - stackCursor.stackEntry.address = [obj unsignedLongValue]; - stackCursor.stackEntry.imageName = nil; - stackCursor.stackEntry.imageAddress = 0; - stackCursor.stackEntry.symbolAddress = 0; - stackCursor.stackEntry.symbolName = nil; - - [frames addObject:[crashStackToEntryMapper - sentryCrashStackEntryToSentryFrame:stackCursor.stackEntry]]; - }]; - - sentryThread.stacktrace = [SentryStacktraceBuilder buildStacktraceFromFrames:frames]; - - return @[ sentryThread ]; -} - -- (SentryCrashStackEntryMapper *)buildCrashStackToEntryMapper -{ - SentryOptions *options = SentrySDK.startOption; - - SentryInAppLogic *inAppLogic = - [[SentryInAppLogic alloc] initWithInAppIncludes:options.inAppIncludes]; - SentryCrashStackEntryMapper *crashStackEntryMapper = - [[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic]; - - return crashStackEntryMapper; -} - -@end - -#endif // TARGET_OS_OSX diff --git a/Sources/Sentry/include/SentryCrashExceptionApplicationHelper.h b/Sources/Sentry/include/SentryCrashExceptionApplicationHelper.h deleted file mode 100644 index b89745b06fa..00000000000 --- a/Sources/Sentry/include/SentryCrashExceptionApplicationHelper.h +++ /dev/null @@ -1,9 +0,0 @@ -#import - -#if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK - -@interface SentryCrashExceptionApplicationHelper : NSObject -+ (void)reportException:(NSException *)exception; -+ (void)_crashOnException:(NSException *)exception; -@end -#endif diff --git a/Sources/Sentry/include/SentryNSExceptionCaptureHelper.h b/Sources/Sentry/include/SentryNSExceptionCaptureHelper.h new file mode 100644 index 00000000000..432d3c11182 --- /dev/null +++ b/Sources/Sentry/include/SentryNSExceptionCaptureHelper.h @@ -0,0 +1,19 @@ +#import + +#if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK + +@interface SentryNSExceptionCaptureHelper : NSObject + +/// Captures the exception and marks that we are inside reportException:. +/// Call this from -[NSApplication reportException:] before calling super. ++ (void)reportException:(NSException *)exception; + +/// Called after [super reportException:] returns. ++ (void)reportExceptionDidFinish; + +/// Captures the exception only if not already captured by reportException:. +/// Call this from -[NSApplication _crashOnException:]. ++ (void)crashOnException:(NSException *)exception; + +@end +#endif diff --git a/Sources/Sentry/include/SentryPrivate.h b/Sources/Sentry/include/SentryPrivate.h index 3928674fa60..932724b90d4 100644 --- a/Sources/Sentry/include/SentryPrivate.h +++ b/Sources/Sentry/include/SentryPrivate.h @@ -12,7 +12,6 @@ #import "SentryCrashBinaryImageCache.h" #import "SentryCrashC.h" #import "SentryCrashDynamicLinker.h" -#import "SentryCrashExceptionApplicationHelper.h" #import "SentryCrashUUIDConversion.h" #import "SentryDataCategoryMapper.h" #import "SentryDiscardReasonMapper.h" @@ -24,6 +23,7 @@ #import "SentryHub+SwiftPrivate.h" #import "SentryNSDataSwizzlingHelper.h" #import "SentryNSDataUtils.h" +#import "SentryNSExceptionCaptureHelper.h" #import "SentryNSFileManagerSwizzlingHelper.h" #import "SentryNetworkTracker.h" #import "SentrySDK+Private.h" diff --git a/Sources/Sentry/include/SentrySDK+Private.h b/Sources/Sentry/include/SentrySDK+Private.h index a1ec3f860b8..750fc1f7426 100644 --- a/Sources/Sentry/include/SentrySDK+Private.h +++ b/Sources/Sentry/include/SentrySDK+Private.h @@ -60,18 +60,6 @@ NS_ASSUME_NONNULL_BEGIN */ + (void)captureEnvelope:(SentryEnvelope *)envelope; -#if TARGET_OS_OSX -/** - * Captures an exception event and sends it to Sentry using the stacktrace from the exception. - * @param exception The exception to send to Sentry. - * @return The @c SentryId of the event or @c SentryId.empty if the event is not sent. - * - */ -+ (SentryId *)captureCrashOnException:(NSException *)exception - NS_SWIFT_NAME(captureCrashOn(exception:)); - -#endif // TARGET_OS_OSX - #if SENTRY_HAS_UIKIT /** Only needed for testing. We can't use `SENTRY_TEST || SENTRY_TEST_CI` because we call this from diff --git a/Sources/Sentry/include/SentryUncaughtNSExceptions.h b/Sources/Sentry/include/SentryUncaughtNSExceptions.h index e6daec28cdd..efd94cdefa6 100644 --- a/Sources/Sentry/include/SentryUncaughtNSExceptions.h +++ b/Sources/Sentry/include/SentryUncaughtNSExceptions.h @@ -2,8 +2,6 @@ #if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK -# import "SentryDefines.h" - NS_ASSUME_NONNULL_BEGIN @interface SentryUncaughtNSExceptions : NSObject @@ -18,7 +16,7 @@ NS_ASSUME_NONNULL_BEGIN + (void)swizzleNSApplicationReportException; -+ (void)capture:(nullable NSException *)exception; ++ (void)swizzleNSApplicationCrashOnException; @end diff --git a/Sources/Sentry/include/SentryUseNSExceptionCallstackWrapper.h b/Sources/Sentry/include/SentryUseNSExceptionCallstackWrapper.h deleted file mode 100644 index 4ea8b2cd539..00000000000 --- a/Sources/Sentry/include/SentryUseNSExceptionCallstackWrapper.h +++ /dev/null @@ -1,24 +0,0 @@ -#import - -NS_ASSUME_NONNULL_BEGIN - -#if TARGET_OS_OSX - -@class SentryThread; - -/** - * This is a helper class to identify exceptions that should use the stacktrace within - */ -@interface SentryUseNSExceptionCallstackWrapper : NSException - -- (instancetype)initWithName:(NSExceptionName)aName - reason:(NSString *_Nullable)aReason - userInfo:(NSDictionary *_Nullable)aUserInfo - callStackReturnAddresses:(NSArray *)callStackReturnAddresses; -- (NSArray *)buildThreads; - -@end - -#endif // TARGET_OS_OSX - -NS_ASSUME_NONNULL_END diff --git a/Sources/Swift/Integrations/SentryCrash/SentryCrashIntegration.swift b/Sources/Swift/Integrations/SentryCrash/SentryCrashIntegration.swift index 171d8fa202c..c2135f2a124 100644 --- a/Sources/Swift/Integrations/SentryCrash/SentryCrashIntegration.swift +++ b/Sources/Swift/Integrations/SentryCrash/SentryCrashIntegration.swift @@ -133,6 +133,7 @@ final class SentryCrashIntegration: NSOb if enableReportingUncaughtExceptions { SentryUncaughtNSExceptions.configureCrashOnExceptions() SentryUncaughtNSExceptions.swizzleNSApplicationReportException() + SentryUncaughtNSExceptions.swizzleNSApplicationCrashOnException() } #endif diff --git a/Tests/SentryTests/Helper/SentryUseNSExceptionCallstackWrapperTests.swift b/Tests/SentryTests/Helper/SentryUseNSExceptionCallstackWrapperTests.swift deleted file mode 100644 index 3adadde81e2..00000000000 --- a/Tests/SentryTests/Helper/SentryUseNSExceptionCallstackWrapperTests.swift +++ /dev/null @@ -1,63 +0,0 @@ -// -// SentryUseNSExceptionCallstackWrapperTests.swift -// SentryTests -// -// Created by Itay Brenner on 30/5/25. -// Copyright © 2025 Sentry. All rights reserved. -// - -@testable import Sentry -import XCTest - -#if os(macOS) - -class SentryUseNSExceptionCallstackWrapperTests: XCTestCase { - func testInitWithException() { - let name = "TestException" - let reason = "Test Reason" - let userInfo = ["key": "value"] - - let wrapper = SentryUseNSExceptionCallstackWrapper(name: NSExceptionName(name), reason: reason, userInfo: userInfo, callStackReturnAddresses: []) - - // Make sure the name, reason and userInfo stay the same - XCTAssertEqual(wrapper.name, NSExceptionName(name)) - XCTAssertEqual(wrapper.reason, reason) - XCTAssertEqual(wrapper.userInfo as? [String: String], userInfo) - } - - func testBuildThreads() { - let addresses = [0x1234, 0x5678, 0x9ABC] - let wrapper = SentryUseNSExceptionCallstackWrapper(name: NSExceptionName(rawValue: "Exception Name"), reason: "Exception Reason", userInfo: [:], callStackReturnAddresses: addresses.map { NSNumber(value: $0) }) - - let threads = wrapper.buildThreads() - - // Verify thread properties - XCTAssertEqual(threads.count, 1) - let thread = threads[0] - XCTAssertEqual(thread.threadId, 0) - XCTAssertEqual(thread.name, "NSException Thread") - XCTAssertEqual(thread.crashed, true) - XCTAssertEqual(thread.current, true) - XCTAssertEqual(thread.isMain, true) - - XCTAssertNotNil(thread.stacktrace) - XCTAssertNotNil(thread.stacktrace?.frames) - - // Addresses are in reverse order - XCTAssertNotNil(thread.stacktrace?.frames[0].instructionAddress, sentry_formatHexAddressUInt64(UInt64(addresses[2]))) - XCTAssertNotNil(thread.stacktrace?.frames[1].instructionAddress, sentry_formatHexAddressUInt64(UInt64(addresses[1]))) - XCTAssertNotNil(thread.stacktrace?.frames[2].instructionAddress, sentry_formatHexAddressUInt64(UInt64(addresses[0]))) - } - - func testBuildThreadsWithEmptyCallStack() { - let wrapper = SentryUseNSExceptionCallstackWrapper(name: NSExceptionName(rawValue: "Exception Name"), reason: "Exception Reason", userInfo: [:], callStackReturnAddresses: []) - - let threads = wrapper.buildThreads() - - XCTAssertEqual(threads.count, 1) - let thread = threads[0] - XCTAssertNotNil(thread.stacktrace) - XCTAssertEqual(thread.stacktrace?.frames.count, 0) - } -} -#endif // os(macOS) diff --git a/Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift b/Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift index 05e86ff0f63..3d749ef9806 100644 --- a/Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift +++ b/Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift @@ -29,38 +29,88 @@ final class SentryUncaughtNSExceptionsTests: XCTestCase { // We have to set the flat to false, cause otherwise we would crash UserDefaults.standard.set(false, forKey: "NSApplicationCrashOnExceptions") NSApplication.shared.reportException(uncaughtInternalInconsistencyException) + + XCTAssertTrue(wasUncaughtExceptionHandlerCalled) } - func testCapture_ForwardsException() throws { + func testSwizzleNSApplicationCrashOnException_classMethod() throws { + let selector = NSSelectorFromString("_crashOnException:") + try XCTSkipUnless( + NSApplication.responds(to: selector), + "_crashOnException: class method not available on this macOS version" + ) + let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter - + defer { crashReporter.uncaughtExceptionHandler = nil wasUncaughtExceptionHandlerCalled = false } + crashReporter.uncaughtExceptionHandler = uncaughtExceptionHandler - - SentryUncaughtNSExceptions.capture(uncaughtInternalInconsistencyException) + + SentryUncaughtNSExceptions.swizzleNSApplicationCrashOnException() + + // Call the class method +[NSApplication _crashOnException:]. + // In tests, SentrySWCallOriginal is skipped so this won't abort(). + NSApplication.perform(selector, with: uncaughtInternalInconsistencyException) + + XCTAssertTrue(wasUncaughtExceptionHandlerCalled) } - - func testCapture_NoUncaughtExceptionHandler() throws { - defer { wasUncaughtExceptionHandlerCalled = false } - - SentryUncaughtNSExceptions.capture(uncaughtInternalInconsistencyException) - - XCTAssertFalse(wasUncaughtExceptionHandlerCalled) + + func testSwizzleNSApplicationCrashOnException_calledMultipleTimes_classMethodCapturesOnce() throws { + let selector = NSSelectorFromString("_crashOnException:") + try XCTSkipUnless( + NSApplication.responds(to: selector), + "_crashOnException: class method not available on this macOS version" + ) + + let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter + + defer { + crashReporter.uncaughtExceptionHandler = nil + wasUncaughtExceptionHandlerCalled = false + uncaughtExceptionHandlerCallCount = 0 + } + + uncaughtExceptionHandlerCallCount = 0 + crashReporter.uncaughtExceptionHandler = uncaughtExceptionHandler + + // Simulate multiple SDK starts — the class method swizzle should not stack. + SentryUncaughtNSExceptions.swizzleNSApplicationCrashOnException() + SentryUncaughtNSExceptions.swizzleNSApplicationCrashOnException() + + NSApplication.perform(selector, with: uncaughtInternalInconsistencyException) + + XCTAssertTrue(wasUncaughtExceptionHandlerCalled) + XCTAssertEqual(uncaughtExceptionHandlerCallCount, 1, "Handler should fire exactly once, not stack on repeated SDK starts") } - - func testCapture_ExceptionIsNil() throws { + + func testSwizzleNSApplicationCrashOnException_instanceMethod() throws { + let selector = NSSelectorFromString("_crashOnException:") + try XCTSkipUnless( + NSApplication.shared.responds(to: selector), + "_crashOnException: instance method not available on this macOS version" + ) + let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter - + defer { crashReporter.uncaughtExceptionHandler = nil wasUncaughtExceptionHandlerCalled = false } - SentryUncaughtNSExceptions.capture(nil) - XCTAssertFalse(wasUncaughtExceptionHandlerCalled) + + crashReporter.uncaughtExceptionHandler = uncaughtExceptionHandler + + SentryUncaughtNSExceptions.swizzleNSApplicationCrashOnException() + + // Call the instance method -[NSApp _crashOnException:]. + // In tests, SentrySWCallOriginal is skipped so this won't abort(). + NSApplication.shared.perform(selector, with: uncaughtInternalInconsistencyException) + + XCTAssertTrue(wasUncaughtExceptionHandlerCalled) } + #endif // os(macOS) } @@ -68,10 +118,12 @@ final class SentryUncaughtNSExceptionsTests: XCTestCase { // We need to declare this on the file level because otherwise we get the error: // A C function pointer cannot be formed from a closure that captures context. var wasUncaughtExceptionHandlerCalled = false +var uncaughtExceptionHandlerCallCount = 0 func uncaughtExceptionHandler(exception: NSException) { XCTAssertEqual(uncaughtInternalInconsistencyException.name, exception.name) XCTAssertEqual(uncaughtInternalInconsistencyException.reason, exception.reason) wasUncaughtExceptionHandlerCalled = true + uncaughtExceptionHandlerCallCount += 1 } let uncaughtInternalInconsistencyException = NSException(name: .internalInconsistencyException, reason: "reason", userInfo: nil) diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index 71533984ad7..9f726188927 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -24,9 +24,10 @@ extension SentryClientInternal { } // swiftlint:disable file_length +// swiftlint:disable type_body_length // We are aware that the client has a lot of logic and we should maybe // move some of it to other classes. -class SentryClientTests: XCTestCase { +final class SentryClientTests: XCTestCase { private static let dsn = TestConstants.dsnAsString(username: "SentryClientTest") @@ -57,7 +58,7 @@ class SentryClientTests: XCTestCase { let processWrapper = MockSentryProcessInfo() let extraContentProvider: SentryExtraContextProvider let locale = Locale(identifier: "en_US") - let timezone = TimeZone(identifier: "Europe/Vienna")! + let timezone = TimeZone(identifier: "Europe/Vienna") ?? TimeZone(secondsFromGMT: 3_600) ?? .current let queue = DispatchQueue(label: "SentryHubTests", qos: .utility, attributes: [.concurrent]) let dispatchQueue = TestSentryDispatchQueueWrapper() @@ -757,14 +758,16 @@ class SentryClientTests: XCTestCase { } func testCaptureErrorWithComplexUserInfo() throws { - let url = URL(string: "https://github.com/getsentry")! + let url = try XCTUnwrap(URL(string: "https://github.com/getsentry")) let error = NSError(domain: "domain", code: 0, userInfo: ["url": url]) let eventId = fixture.getSut().capture(error: error, scope: fixture.scope) eventId.assertIsNotEmpty() let actual = try lastSentEventWithAttachment() - XCTAssertEqual(url.absoluteString, actual.context!["user info"]!["url"] as? String) + let context = try XCTUnwrap(actual.context) + let userInfo = try XCTUnwrap(context["user info"]) + XCTAssertEqual(url.absoluteString, userInfo["url"] as? String) } func testCaptureErrorWithNestedUnderlyingErrors() throws { @@ -1206,7 +1209,9 @@ class SentryClientTests: XCTestCase { eventId.assertIsNotEmpty() let actual = try lastSentEventWithAttachment() - XCTAssertEqual(expectedValue, actual.context!["user info"]!["key"] as? String) + let context = try XCTUnwrap(actual.context) + let userInfo = try XCTUnwrap(context["user info"]) + XCTAssertEqual(expectedValue, userInfo["key"] as? String) } func testScopeIsNotNil() throws { @@ -2111,7 +2116,7 @@ class SentryClientTests: XCTestCase { options.onCrashedLastRun = { crashEvent in callbackExpectation.fulfill() XCTAssertEqual(event.eventId, crashEvent.eventId) - captureCrash!() + captureCrash?() } }) captureCrash = { client.captureFatalEvent(event, with: self.fixture.scope) } @@ -2185,12 +2190,12 @@ class SentryClientTests: XCTestCase { XCTAssertEqual(scope.attachments.first?.attachmentType, .viewHierarchy) } - func testCaptureEvent_withAdditionalEnvelopeItem() { + func testCaptureEvent_withAdditionalEnvelopeItem() throws { let event = Event(level: SentryLevel.warning) event.message = fixture.message - + let attachment = "{}" - let data = attachment.data(using: .utf8)! + let data = try XCTUnwrap(attachment.data(using: .utf8)) let itemHeader = SentryEnvelopeItemHeader(type: "attachment", length: UInt(data.count)) let item = SentryEnvelopeItem(header: itemHeader, data: data) @@ -2233,79 +2238,79 @@ class SentryClientTests: XCTestCase { } } - func testCaptureReplayEvent() { + func testCaptureReplayEvent() throws { let sut = fixture.getSut() let replayEvent = SentryReplayEvent(eventId: SentryId(), replayStartTimestamp: Date(), replayType: .session, segmentId: 2) let replayRecording = SentryReplayRecording(segmentId: 2, size: 200, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 930, width: 390, extraEvents: []) - + //Not a video url, but its ok for test the envelope - let movieUrl = Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json") - - sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl!, with: Scope()) + let movieUrl = try XCTUnwrap(Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json")) + + sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl, with: Scope()) let envelope = fixture.transport.sentEnvelopes.first XCTAssertEqual(try XCTUnwrap(envelope?.items.first).header.type, SentryEnvelopeItemTypes.replayVideo) } - func testCaptureReplayEvent_WrongEventFromEventProcessor() { + func testCaptureReplayEvent_WrongEventFromEventProcessor() throws { let sut = fixture.getSut() sut.options.beforeSend = { _ in return Event() } - + let replayEvent = SentryReplayEvent(eventId: SentryId(), replayStartTimestamp: Date(), replayType: .session, segmentId: 2) let replayRecording = SentryReplayRecording(segmentId: 3, size: 200, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 930, width: 390, extraEvents: []) - - let movieUrl = Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json") - sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl!, with: Scope()) - + + let movieUrl = try XCTUnwrap(Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json")) + sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl, with: Scope()) + //Nothing should be captured because beforeSend returned a non ReplayEvent XCTAssertEqual(self.fixture.transport.sentEnvelopes.count, 0) } - func testCaptureReplayEvent_DontCaptureNilEvent() { + func testCaptureReplayEvent_DontCaptureNilEvent() throws { let sut = fixture.getSut() sut.options.beforeSend = { _ in return nil } - + let replayEvent = SentryReplayEvent(eventId: SentryId(), replayStartTimestamp: Date(), replayType: .session, segmentId: 2) let replayRecording = SentryReplayRecording(segmentId: 3, size: 200, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 930, width: 390, extraEvents: []) - - let movieUrl = Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json") - sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl!, with: Scope()) - + + let movieUrl = try XCTUnwrap(Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json")) + sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl, with: Scope()) + //Nothing should be captured because beforeSend returned nil XCTAssertEqual(self.fixture.transport.sentEnvelopes.count, 0) } - func testCaptureReplayEvent_InvalidFile() { + func testCaptureReplayEvent_InvalidFile() throws { let sut = fixture.getSut() sut.options.beforeSend = { _ in return nil } - + let replayEvent = SentryReplayEvent(eventId: SentryId(), replayStartTimestamp: Date(), replayType: .session, segmentId: 2) let replayRecording = SentryReplayRecording(segmentId: 3, size: 200, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 930, width: 390, extraEvents: []) - - let movieUrl = URL(string: "NoFile")! + + let movieUrl = try XCTUnwrap(URL(string: "NoFile")) sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl, with: Scope()) //Nothing should be captured because beforeSend returned nil XCTAssertEqual(self.fixture.transport.sentEnvelopes.count, 0) } - func testCaptureReplayEvent_noBradcrumbsThreadsDebugMeta() { + func testCaptureReplayEvent_noBradcrumbsThreadsDebugMeta() throws { let sut = fixture.getSut() let replayEvent = SentryReplayEvent(eventId: SentryId(), replayStartTimestamp: Date(), replayType: .session, segmentId: 2) let replayRecording = SentryReplayRecording(segmentId: 2, size: 200, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 930, width: 390, extraEvents: []) - + //Not a video url, but its ok for test the envelope - let movieUrl = Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json") - + let movieUrl = try XCTUnwrap(Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json")) + let scope = Scope() scope.addBreadcrumb(Breadcrumb(level: .debug, category: "Test Breadcrumb")) - - sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl!, with: scope) + + sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl, with: scope) XCTAssertNil(replayEvent.breadcrumbs) XCTAssertNil(replayEvent.threads) @@ -2579,32 +2584,6 @@ class SentryClientTests: XCTestCase { XCTAssertEqual(testProcessor.addLogInvocations.count, 0, "Log should be dropped when enableLogs is false") } - func testCaptureSentryWrappedException() throws { -#if os(macOS) - let exception = NSException(name: NSExceptionName("exception"), reason: "reason", userInfo: nil) - // If we don't raise the exception, it won't have the callStack data - let raisedException = ExceptionCatcher.try { - exception.raise() - } - let raisedExceptionUnwrapped = raisedException! - let sentryException = SentryUseNSExceptionCallstackWrapper(name: raisedExceptionUnwrapped.name, reason: raisedExceptionUnwrapped.reason, userInfo: raisedExceptionUnwrapped.userInfo, callStackReturnAddresses: raisedExceptionUnwrapped.callStackReturnAddresses) - let eventId = fixture.getSut().capture(exception: sentryException, scope: fixture.scope) - - eventId.assertIsNotEmpty() - let actual = try lastSentEventWithAttachment() - XCTAssertEqual(actual.threads?.count, 1) - XCTAssertEqual(actual.threads?[0].name, "NSException Thread") - XCTAssertEqual(actual.threads?[0].threadId, 0) - XCTAssertEqual(actual.threads?[0].crashed, true) - XCTAssertEqual(actual.threads?[0].current, true) - XCTAssertEqual(actual.threads?[0].isMain, true) - // Make sure the stacktrace is not empty - XCTAssertGreaterThan(actual.threads?[0].stacktrace?.frames.count ?? 0, 1) - - #else - throw XCTSkip("Test is disabled for this OS version") - #endif // os(macOS) - } } private extension SentryClientTests { diff --git a/Tests/SentryTests/SentryCrashExceptionApplicationTests.swift b/Tests/SentryTests/SentryCrashExceptionApplicationTests.swift index 7bedd4075fa..c7911bf5497 100644 --- a/Tests/SentryTests/SentryCrashExceptionApplicationTests.swift +++ b/Tests/SentryTests/SentryCrashExceptionApplicationTests.swift @@ -4,57 +4,83 @@ import XCTest #if os(macOS) -class SentryCrashExceptionApplicationHelperTests: XCTestCase { - +private var exceptionHandlerCallCount = 0 +private func countingExceptionHandler(exception: NSException) { + exceptionHandlerCallCount += 1 +} + +class SentryNSExceptionCaptureHelperTests: XCTestCase { + override func tearDown() { super.tearDown() - + + SentryNSExceptionCaptureHelper.reportExceptionDidFinish() + let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter + crashReporter.uncaughtExceptionHandler = nil + exceptionHandlerCallCount = 0 + clearTestState() resetUserDefaults() UserDefaults.standard.removeObject(forKey: "NSApplicationCrashOnExceptions") } - - func testCrashOnException() throws { - // Arrange - let client = TestClient(options: Options()) - let hub = TestHub(client: client, andScope: nil) - SentrySDKInternal.setCurrentHub(hub) - let exception = NSException(name: NSExceptionName("TestException"), reason: "Test Reason", userInfo: nil) - - // Act - SentryCrashExceptionApplicationHelper._crash(on: exception) - - // Assert - let testClient = try XCTUnwrap(SentrySDKInternal.currentHub().getClient() as? TestClient) - XCTAssertEqual(1, testClient.captureExceptionWithScopeInvocations.count) - XCTAssertEqual(exception.name, testClient.captureExceptionWithScopeInvocations.first?.exception.name) - XCTAssertEqual(exception.reason, testClient.captureExceptionWithScopeInvocations.first?.exception.reason) - } - - func testReportExceptionWithUncaughtExceptionHandler() { - // Arrange + + func testReportExceptionCallsHandler() { let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter - defer { - crashReporter.uncaughtExceptionHandler = nil - wasUncaughtExceptionHandlerCalled = false - UserDefaults.standard.removeObject(forKey: "NSApplicationCrashOnExceptions") - } - crashReporter.uncaughtExceptionHandler = uncaughtExceptionHandler - - // Act - SentryCrashExceptionApplicationHelper.report(uncaughtInternalInconsistencyException) - - // Assert - XCTAssertTrue(wasUncaughtExceptionHandlerCalled) + crashReporter.uncaughtExceptionHandler = countingExceptionHandler + + SentryNSExceptionCaptureHelper.report(uncaughtInternalInconsistencyException) + SentryNSExceptionCaptureHelper.reportExceptionDidFinish() + + XCTAssertEqual(exceptionHandlerCallCount, 1) } - + func testReportExceptionWithoutUncaughtExceptionHandler() { let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter crashReporter.uncaughtExceptionHandler = nil - - SentryCrashExceptionApplicationHelper.report(uncaughtInternalInconsistencyException) - - XCTAssertFalse(wasUncaughtExceptionHandlerCalled) + + SentryNSExceptionCaptureHelper.report(uncaughtInternalInconsistencyException) + SentryNSExceptionCaptureHelper.reportExceptionDidFinish() + + XCTAssertEqual(exceptionHandlerCallCount, 0) + } + + func testCrashOnException_directCall_capturesException() { + let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter + crashReporter.uncaughtExceptionHandler = countingExceptionHandler + + SentryNSExceptionCaptureHelper.crash(on: uncaughtInternalInconsistencyException) + + XCTAssertEqual(exceptionHandlerCallCount, 1) + } + + func testCrashOnException_calledDuringReportException_doesNotDuplicate() { + let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter + crashReporter.uncaughtExceptionHandler = countingExceptionHandler + + // Simulate the flow: reportException: captures, then [super reportException:] + // internally calls _crashOnException: (when NSApplicationCrashOnExceptions is YES). + SentryNSExceptionCaptureHelper.report(uncaughtInternalInconsistencyException) + + // This simulates _crashOnException: being called by [super reportException:] + SentryNSExceptionCaptureHelper.crash(on: uncaughtInternalInconsistencyException) + + SentryNSExceptionCaptureHelper.reportExceptionDidFinish() + + XCTAssertEqual(exceptionHandlerCallCount, 1) + } + + func testCrashOnException_afterReportExceptionFinishes_capturesNormally() { + let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter + crashReporter.uncaughtExceptionHandler = countingExceptionHandler + + // First: a full reportException: cycle + SentryNSExceptionCaptureHelper.report(uncaughtInternalInconsistencyException) + SentryNSExceptionCaptureHelper.reportExceptionDidFinish() + + // Then: a direct _crashOnException: call (separate AppKit code path) + SentryNSExceptionCaptureHelper.crash(on: uncaughtInternalInconsistencyException) + + XCTAssertEqual(exceptionHandlerCallCount, 2) } } diff --git a/Tests/SentryTests/SentrySDKInternalTests.swift b/Tests/SentryTests/SentrySDKInternalTests.swift index c2d90f32740..c520135e789 100644 --- a/Tests/SentryTests/SentrySDKInternalTests.swift +++ b/Tests/SentryTests/SentrySDKInternalTests.swift @@ -3,6 +3,7 @@ import XCTest // swiftlint:disable file_length +// swiftlint:disable type_body_length class SentrySDKInternalTests: XCTestCase { private static let dsnAsString = TestConstants.dsnAsString(username: "SentrySDKTests") @@ -56,7 +57,7 @@ class SentrySDKInternalTests: XCTestCase { scope = Scope() scope.setTag(value: "value", key: "key") - client = TestClient(options: options)! + client = try XCTUnwrap(TestClient(options: options)) hub = SentryHubInternal(client: client, andScope: scope, andCrashWrapper: TestSentryCrashWrapper(processInfoWrapper: ProcessInfo.processInfo), andDispatchQueue: SentryDispatchQueueWrapper()) feedback = SentryFeedback(message: "Again really?", name: "Tim Apple", email: "tim@apple.com") @@ -449,7 +450,7 @@ class SentrySDKInternalTests: XCTestCase { } #if SENTRY_HAS_UIKIT - func testClose_StopsAppStateManager() { + func testClose_StopsAppStateManager() throws { SentrySDK.start { options in options.dsn = SentrySDKTests.dsnAsString options.tracesSampleRate = 1 @@ -472,7 +473,8 @@ class SentrySDKInternalTests: XCTestCase { XCTAssertEqual(appStateManager.startCount, 0) let stateAfterStop = fixture.fileManager.readAppState() - XCTAssertFalse(stateAfterStop!.isSDKRunning) + let state = try XCTUnwrap(stateAfterStop) + XCTAssertFalse(state.isSDKRunning) } #endif @@ -943,21 +945,6 @@ class SentrySDKInternalTests: XCTestCase { XCTAssertEqual(result["tag2"], "value2") } #endif // os(iOS) || os(tvOS) - -#if os(macOS) - func testCaptureCrashOnException() { - givenSdkWithHub() - - SentrySDKInternal.captureCrashOn(exception: fixture.exception) - - let client = fixture.client - XCTAssertEqual(1, client.captureExceptionWithScopeInvocations.count) - XCTAssertNotEqual(fixture.exception, client.captureExceptionWithScopeInvocations.first?.exception) - XCTAssertEqual(fixture.exception.name, client.captureExceptionWithScopeInvocations.first?.exception.name) - XCTAssertEqual(fixture.exception.reason, client.captureExceptionWithScopeInvocations.first?.exception.reason) - XCTAssertEqual(fixture.scope, client.captureExceptionWithScopeInvocations.first?.scope) - } -#endif // os(macOS) } private extension SentrySDKInternalTests { diff --git a/Tests/SentryTests/SentryTests-Bridging-Header.h b/Tests/SentryTests/SentryTests-Bridging-Header.h index 19c04ff069d..a2971b71c63 100644 --- a/Tests/SentryTests/SentryTests-Bridging-Header.h +++ b/Tests/SentryTests/SentryTests-Bridging-Header.h @@ -146,7 +146,6 @@ #import "SentryTransportAdapter.h" #import "SentryTransportFactory.h" #import "SentryUncaughtNSExceptions.h" -#import "SentryUseNSExceptionCallstackWrapper.h" #import "SentryWatchdogTerminationBreadcrumbProcessor.h" #import "SentryWeakMap.h" #import "TestSentrySpan.h" From 3e4252e4184803619d7956be0aa5489c840bf420 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Wed, 11 Mar 2026 11:49:25 +0100 Subject: [PATCH 2/2] docs: Add changelog entry for NSException thread safety fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ea9ef14ab6..19d8b2886a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ### Fixes +- Fix thread safety in macOS NSException capture to prevent duplicate exception reports (#7672) - Capture transactions that finish during the launch profiling window instead of silently discarding them (#7602) ## 9.6.0