-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
SIGSEGV when a NullReferenceException is thrown in Release mode #3861
Comments
Thank you so much for the repro, steps and the ticket. |
I don't have much to add here other than a few investigative notes/artifacts: logcat
|
Thanks for the details. I recall seeing this odd behavior where a throw new NullRef doesn't raise errors but an actual null ref does. iirc it was due to how Mono deals with signals and convert stuff into C# exceptions (or not) and since our sdk captured errors via signal handler we report stuff Mono suppresses the crash of. perhaps we need to let Mono do its thing before we take the signal handler. So instead of going first and calling the original handler we invert that |
I think it relates to this change: The underlying reason we made that changes is detailed here: I can't immediately think of an alternative way around this. When a BTW: Apologies for this making it into the 5.0.0 release. More testing should have been done on this PR before it was merged. |
The .NET runtime has no idea that another signal handler was installed after its own, similar to how the Native SDK signal handler has no idea it is running with CLR/Mono. So, the runtime can't really handle that detail because it expects to be the sole signal handler user in a .NET application (which, for most scenarios, is probably true) and uses it for the optimized path of code generation. We can only provide heuristics in finding discriminators between the crashes we must handle in the Native SDK and the ones we should ignore. In August, I created the draft PR to allow downstream tests to validate a heuristic, which we know works for CLR on Linux, also for Mono on Android. Now we know that it doesn't work, and sadly, this had to be discovered by users rather than us. This testing didn't happen in a way sufficient RE: the base case coming from the users. Since I was not involved in any downstream testing, I can only guess that most of it happened against What is visible from the logs and also my own initial Maui experiments:
To be clear, this is only from what I see in the logs. It is yet unclear why the signal handler behaves this way and whether this is generally the case with Mono, or specific to its implementation on Android. With CLR on Linux, the runtime signal handler never returns if it successfully converted a signal to a managed exception so that we can rely on that discontinuation of the signal chain as a signifier for a managed exception rather than a native crash. It is clear now that this is not the behavior for Mono on Android. Last but not least, for me to dive any deeper here, we need to clarify this topic's ownership. I can handle this topic end-to-end, if necessary, if only from an available resource or expertise perspective. However, this should be explicitly decided and accordingly prioritized against all other things, which, to my knowledge, hasn't happened yet. Having a more principled approach here might be the right choice because it also affects the |
I don't have enough context to be able to do that... I'm familiar with the dotnet backlog but not that of the NDK or Sentry, so can't say how this compares to other stuff on your plate. For .NET at least, we're getting quite a few issues raised about SIGSEGV errors on iOS/Android so wrt .NET this one would be "High Impact". I can reverse the order in which the signal handlers get chained easily enough in the .NET SDK. That will get us back to the original behaviour (so users' apps aren't crashing because of Sentry). I'm currently the only developer maintaining the .NET SDK so any assistance beyond that (addressing the original issue) would be awesome. I'm definitely happy for you to own this if you have capacity. |
It might be sensible to disable the native crash reporters on Android and iOS for Maui as a first step so as not to double-report.
I am (now) fully aware of the current resource situation. We will find a solution; I am currently not working on fixing this, and prioritization is out of my hands. We will plan how to move forward in the upcoming weekly. |
The issue reported in this ticket ( The fix is included in the 5.0.1 release. That leaves us with the original behaviour from 4.0.0, but still searching for an (alternate) resolution to the following issues on Android:
... and this, which is potentially related, on iOS: |
Original issue is this one: |
Possibly useful: C++ source code for signal handling in the dotnet runtime. It looks like the .NET runtime will invoke any previously registered signal handlers only if/when it doesn't explicitly handle the signal. |
This will only happen if the Platform Adaptation Layer (PAL) hasn't been initialized yet so that the platform default handler handles SIGSEGVs happening before or during initialization. |
I have already gone into analyzing the behavior of the CoreCLR implementation here: dotnet/android#9055 (comment). The question now is why the Mono on Android implementation behaves differently. |
We don't have the required resources available on the Native team to solve this end-to-end at the moment, so coming back to #3861 (comment), we need to recommend deactivating the native crash reporters on Android and iOS for MAUI for the time being to mitigate the issue of duplicate reports. However we're in the process of getting more resources on the downstream team(s) which means we'll hopefully be able to tackle the root cause some time in the next few months. |
Since this issue (the application crashing) has been resolved, I'll close it. I've opened a new issue summarising the remaining problem and steps we need to take to resolve in: |
Package
Sentry.Maui
.NET Flavor
.NET
.NET Version
9.0.0
OS
Android
SDK Version
35
Self-Hosted Sentry Version
No response
Steps to Reproduce
Repro app : https://github.com/tranb3r/Issues/tree/main/MauiAppSegfault
Expected Result
No crash. No SIGSEGV report by Sentry.
Actual Result
With 4.x : no crash but SIGSEGV report by Sentry.
With 5.0 : app crashes and SIGSEGV report is sent by Sentry.
Without Sentry.Maui : no crash (exception is catched).
Related issues/PRs
dotnet/android#9055
#3461
#3694
The text was updated successfully, but these errors were encountered: