-
-
Notifications
You must be signed in to change notification settings - Fork 226
Prevent Native EXC_BAD_ACCESS signal for NullRefrenceExceptions #3909
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 15 commits
c8cf715
90d41c3
37900d7
892719f
2b35173
f630cea
b8672d2
8c97561
1543cea
96845d1
341aa83
6be8cfb
69d0d07
316e75a
2d22ca3
1497b61
6c20eb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| using ObjCRuntime; | ||
|
|
||
| namespace Sentry.Cocoa; | ||
|
|
||
| internal interface IRuntime | ||
| { | ||
| internal event MarshalManagedExceptionHandler MarshalManagedException; | ||
| internal event MarshalObjectiveCExceptionHandler MarshalObjectiveCException; | ||
| } | ||
|
|
||
| internal sealed class RuntimeAdapter : IRuntime | ||
| { | ||
| public static RuntimeAdapter Instance { get; } = new(); | ||
|
|
||
| private RuntimeAdapter() | ||
| { | ||
| Runtime.MarshalManagedException += OnMarshalManagedException; | ||
| Runtime.MarshalObjectiveCException += OnMarshalObjectiveCException; | ||
| } | ||
|
|
||
| public event MarshalManagedExceptionHandler? MarshalManagedException; | ||
| public event MarshalObjectiveCExceptionHandler? MarshalObjectiveCException; | ||
|
|
||
| [SecurityCritical] | ||
| private void OnMarshalManagedException(object sender, MarshalManagedExceptionEventArgs e) => MarshalManagedException?.Invoke(this, e); | ||
|
|
||
| [SecurityCritical] | ||
| private void OnMarshalObjectiveCException(object sender, MarshalObjectiveCExceptionEventArgs e) => MarshalObjectiveCException?.Invoke(this, e); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| using ObjCRuntime; | ||
| using Sentry.Extensibility; | ||
| using Sentry.Integrations; | ||
|
|
||
| namespace Sentry.Cocoa; | ||
|
|
||
| /// <summary> | ||
| /// When AOT Compiling iOS applications, the AppDomain UnhandledExceptionHandler doesn't fire. So instead we intercept | ||
| /// the Runtime.RuntimeMarshalManagedException event. | ||
| /// </summary> | ||
| internal class RuntimeMarshalManagedExceptionIntegration : ISdkIntegration | ||
| { | ||
| private readonly IRuntime _runtime; | ||
| private IHub? _hub; | ||
| private SentryOptions? _options; | ||
|
|
||
| internal RuntimeMarshalManagedExceptionIntegration(IRuntime? runtime = null) | ||
| => _runtime = runtime ?? RuntimeAdapter.Instance; | ||
|
|
||
| public void Register(IHub hub, SentryOptions options) | ||
| { | ||
| _hub = hub; | ||
| _options = options; | ||
| _runtime.MarshalManagedException += Handle; | ||
| } | ||
|
|
||
| // Internal for testability | ||
| [SecurityCritical] | ||
| internal void Handle(object sender, MarshalManagedExceptionEventArgs e) | ||
| { | ||
| _options?.LogDebug("Runtime Marshal Managed Exception"); | ||
|
|
||
| if (e.Exception is { } ex) | ||
jamescrosswell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| ex.SetSentryMechanism( | ||
| "Runtime.MarshalManagedException", | ||
| "This exception was caught by the .NET Runtime Marshal Managed Exception global error handler. " + | ||
| "The application may have crashed as a result of this exception.", | ||
|
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. It's not certain that it did crash?
Collaborator
Author
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. I'm not sure about that no. We're using this event instead of I've asked the question here:
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. But regardless of that bool, if that fires (on your tests) the app crashes? If so, I'd indicate that's the case. But keeping it vague in here is fine, I was just wondering how this works |
||
| handled: false); | ||
|
|
||
| // Call the internal implementation, so that we still capture even if the hub has been disabled. | ||
| _hub?.CaptureExceptionInternal(ex); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh we've been missing out on a category of errors?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially, yes. Matt created this workaround to the problem (after this conversation):
sentry-dotnet/src/Sentry/Platforms/Cocoa/SentrySdk.cs
Lines 13 to 17 in c20d162
However that marshalling mode isn't supported when AOT compiling iOS apps. It causes xamarin to throw an assertion error, which is unrelated to the error from the user's code.