-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
[iOS / Native AOT] Bad stack trace due to unsupported marshalling behavior #3280
Comments
Hm, I'm not sure I follow everything that's going on here but it seems like that assertion is also dependent on
I can't see anything from Sentry in that stack trace though. Do you know what kind of exception was originally thrown (that resulted in this stack trace)? |
I don' think it is dependent on I don't know what exception was originally thrown (that's what I'm trying to find out), @rolfbjarne recommended that I should also subscribe to the |
OK thanks, that makes more sense. If we can find a way to throw an exception that reproduces this behaviour then we should be able to work out a solution 👍🏻 |
@jamescrosswell you can reproduce this behavior / stack trace by simply throwing an exception in the sentry-dotnet/src/Sentry/Platforms/Cocoa/SentrySdk.cs Lines 14 to 17 in bea3fcf
|
We could definitely put these on the Cocoa specific native options - like we do with the LogCat options for Android. |
Upping the priority on this as it crashes the process. |
@bitsandfoxes what do we want to do about this?
Realistically, we can't test for things like this until/unless we create a developer profile and an app in the Apple store that we start down the QA route. Having done that once personally for a Flutter app, I suspect it's going to take about 3-5 days to get this all setup via CI so that we can easily deploy new versions of our app (e.g. Sentry.Samples.MAUI) to the iOS store... there are various encryption/deployment keys to manage that we'll need to sync between CI and our local machines etc. I don't think we've got the bandwidth to do this right now. Perhaps once we've sorted out what we're going to do about net9.0 (and the device tests) we could circle back to this. |
Would it not be possible to allow an optional/temporary way to disable the changing of marshalling bahavior, as I suggested above? |
I was able to run a Native AOT compiled app locally on a physical device using |
That helps... it means we don't have to push it all the way through to test flight. Just a bit of fluffing around setting up encryption keys, developer profiles and configuring an app in the apple dev portal. So it sounds a little less painful, when we do have the bandwidth to look into this. |
When I create a blank iOS app (
In your screenshot, these lines still seem to be present: I also cannot reproduce the stack trace at the moment when using my own logging, the top of the stack trace always looks like the one below, even when setting the ExceptionMode to
Wish I could provide more help, but I'm also no expert in the matter, unfortunately. |
Thanks @tipa, I'm running against 9.0.1 runtime so I think my stack trace corresponds to this version of the runtime.m (line 1478), which is ultimately triggered here (line 2484). That makes sense since Bryce was setting the ExceptionMode to Using After much fiddling around and digging into the internals of Xamarin, I think I might have a PR that fixes this: However, I haven't managed to test this on the CoreCLR. No matter what I try, on my physical iOS device, it detects that it's running on MonoVM 😕 I'm running That could indicate that the logic I'm using to detect whether CoreCLR is being used is incorrect... but I'm reasonably confident this should work: sentry-dotnet/src/Sentry.Bindings.Cocoa/buildTransitive/Sentry.Bindings.Cocoa.targets Lines 58 to 61 in 0087cf3
If I put together a pre-release including this fix, would you be able to test it? If this doesn't work, then I think we could just add an option to let you override the |
I used a similar command when testing, I think I didn't specify the -f and -r options, but that shouldn't make much of a difference.
Based on the command above, I assume you were running this project? I don't know if you made any adjustments to the file prior to running it, but maybe the condition here isn't met? But that's also just a wild guess, because it looks right... sentry-dotnet/samples/Sentry.Samples.Maui/Sentry.Samples.Maui.csproj Lines 48 to 50 in 3e4af30
I can try as long as it doesn't take too long. |
OK, I think I've finally got it working. The MSBuild condition I had configured wasn't appropriate. I've tweaked this and now when I run on a physical device with this command, I get AOT Compilation and
And when running on a simulator with this command, I get
I believe this is finally what we wanted... @tipa I'll put together a pre-release and let you know when it's available so that you can double check my work. Thank you so much for your patience with this. |
@tipa this should finally have been resolved in PR#3909 that was included in the 5.1.0 release. That PR removes all/any code modifying the If you have a chance to test with 5.1.0 that'd be great (and I can leave this issue open for a few more days). If not, I understand and I'll close this issue immediately. |
Hm, it is and it isn't. The fix in the 5.1.0 release suppresses EXC_BAD_ACCESS exceptions from the Cocoa SDK (those would typically be translated to and captured as managed NullReferenceExceptions). Your code is creating EXC_CRASH errors rather than EXC_BAD_ACCESS errors, so the Sentry SDK doesn't suppress these. It looks like you just did a I think we just have to accept duplicate exceptions in this particular scenario then (until we can find a long term / root cause solution to the problem described in #3902. Although you're getting two events for exceptions that crash the app, at least one of them should have a stack trace that helps find the code that it's coming from. |
Yes, the original issue indeed appears to be fixed |
Package
Sentry
.NET Flavor
.NET
.NET Version
8.0.2
OS
iOS
SDK Version
4.2.1
Self-Hosted Sentry Version
No response
Steps to Reproduce
Throw an exception in the
AppDelegate.FinishedLaunching
method in an NativeAot-compiled app.To run a Native AOT compiled app locally on a physical device use
dotnet publish /t:Run /p:_DeviceName=DEVICEID
whereDEVICEID
can be queried usingxcrun xctrace list devices
.There might be more prerequisites, like the managed exception to be marshalled to native code and back to managed.
Expected Result
Crash with useful strack trace including managed C# function from where it originated
Actual Result
I believe this is caused because of this event handler that changes the default marshalling behavior of exceptions:
sentry-dotnet/src/Sentry/Platforms/Cocoa/SentrySdk.cs
Lines 14 to 17 in bea3fcf
The code was introduced after this discussion in the Xamarin repo: dotnet/macios#15252
According to @rolfbjarne from the .NET iOS/macOS team, CoreCLR (which is what is used on iOS when using Native AOT as well as on macOS) doesn't support
MarshalManagedExceptionMode.UnwindNativeCode
and therefore, this assertion is crashing the process:https://github.com/xamarin/xamarin-macios/blob/907081f787315704a01c940cf28b46b47db23df0/runtime/runtime.m#L2362-L2364
https://github.com/xamarin/xamarin-macios/blob/907081f787315704a01c940cf28b46b47db23df0/runtime/runtime.m#L2452-L2455
References
The text was updated successfully, but these errors were encountered: