-
-
Notifications
You must be signed in to change notification settings - Fork 396
fix(macos): Fix thread safety in NSException capture #7672
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| #import <Foundation/Foundation.h> | ||
|
|
||
| #if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK | ||
|
|
||
| # import "SentryCrash.h" | ||
| # import "SentryNSExceptionCaptureHelper.h" | ||
| # import "SentrySwift.h" | ||
| # import <stdatomic.h> | ||
|
|
||
| @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)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Klaus believes it's OK to call these methods here, but I don't trust him:
Did double-check that it's safe to call C11 atomics in this context? If you already verified this, great — just want to make sure. We could also double-check with @supervacuus. |
||
| [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 | ||
Uh oh!
There was an error while loading. Please reload this page.