-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[android] AppDomain.CurrentDomain.UnhandledException not working in .NET 5/6 #44526
Comments
Tagging subscribers to this area: @CoffeeFlux Issue meta data
|
I think I already fixed this for desktop, so it's strange it doesn't work on Android. There should be a failing runtime test that corresponds to this, but we still don't have issues opened for the failures so I'm not sure where to confirm that. (Related: I still think we really want to get individual issues opened for those failures up sometime soon because of things like this.) cc: @SamMonoRT |
@CoffeeFlux to double-check, I tried it again with .NET 6.0.100-alpha.1.20555.3, and changed the code to use: --Console.WriteLine ("# Unhandled Exception: sender={0}; e.IsTerminating={1}; e.ExceptionObject={2}",
++Android.Util.Log.Debug ("Foo", "# Unhandled Exception: sender={0}; e.IsTerminating={1}; e.ExceptionObject={2}", But I'm not seeing it in |
@jonathanpeppers - is this still an issue, and if so, is this really required for 6.0 ? |
We still have this test disabled: I ran the test just now, and I'm not seeing the I would think customers would want this event to work. Various crash reporting tools would use it, such as AppCenter or Raygun. I get this in the log, but the app seems to abort before the event:
|
The Android team was able to find another way via dotnet/android@c1a2ee7 Closing |
Context: c1a2ee7 Context: dotnet/runtime#44526 If c1a2ee7 is working now, we should be able to enable this test.
@steveisok The above commit deals with propagation to managed code of exceptions thrown in Java code, it doesn't make the test mentioned by @jonathanpeppers work. I've just tested it, we still don't see |
This doesn't entirely make sense to me: if there is no debugger attached, then the managed exception should be captured at the JNI boundary, re-raised in Java, and thus should eventually hit If a debugger is attached, then we're in the world of Does the |
It never does, I checked that. What I think is happening is that Mono reports the unhandled exception before it gets to the Java side and aborts. |
The log from the test contains the following interesting bits:
The assertion and the following backtrace are seen just after the Java runtime reports an unhandled exception thrown from managed code. However, before the exception propagation path kicks in, Mono aborts in static void
stub_debugger_unhandled_exception (MonoException *exc)
{
g_assert_not_reached ();
} The test doesn't use the debugger (the project it generates is built in the |
Context: dotnet/runtime#44526 (comment) Context: dotnet/runtime#44526 (comment) Context: start: https://discord.com/channels/732297728826277939/732297837953679412/869330822262587392 Context: end? https://discord.com/channels/732297728826277939/732297837953679412/869343082552893440 On .NET 6, `JNIEnv.mono_unhandled_exception` is `monodroid_debugger_unhandled_exception()`, which calls `mono_debugger_agent_unhandled_exception()`; see also e4debf7. The problem is that in our current world order of "Mono components" (0f7a0cd), if the debugger isn't used, then we get "debugger stubs" for the mono debugger agent, which turns `mono_debugger_agent_unhandled_exception()` into an [*assertion*][0]: static void stub_debugger_unhandled_exception (MonoException *exc) { g_assert_not_reached (); } The result is that when an exception is thrown, *before* the `AppDomain.UnhandledException` event can be raised, the runtime dies in a horrible flaming assertion death: E andledexceptio: * Assertion: should not be reached at /__w/1/s/src/mono/mono/component/debugger-stub.c:175 Avoid this issue by checking `Debugger.IsAttached` *before* calling `monodroid_debugger_unhandled_exception()`. Additionally, remove some obsolete comments: .NET 6 couldn't resolve `Debugger.Mono_UnhandledException()` because .NET 6 never had it, so the linker was right to warn about its absence. [0]: https://github.com/dotnet/runtime/blob/16b456426dfb5212a24bfb78bfd5d9adfcc95185/src/mono/mono/component/debugger-stub.c#L172-L176
With @jonpryor's change we now see the native
After that the runtime calls
The fatal error is reported by Mono, perhaps that's what eventually prevents the event from being raised? Perhaps the runtime is aborted before it even gets to the stage where it would raise the event? |
I'll try to fix it: |
Fixed #56380 |
Context: c1a2ee7 Context: dotnet/runtime#44526 If c1a2ee7 is working now, we should be able to enable this test.
Context: dotnet/runtime#44526 (comment) Context: dotnet/runtime#44526 (comment) Context: start: https://discord.com/channels/732297728826277939/732297837953679412/869330822262587392 Context: end? https://discord.com/channels/732297728826277939/732297837953679412/869343082552893440 On .NET 6, `JNIEnv.mono_unhandled_exception` is `monodroid_debugger_unhandled_exception()`, which calls `mono_debugger_agent_unhandled_exception()`; see also e4debf7. The problem is that in our current world order of "Mono components" (0f7a0cd), if the debugger isn't used, then we get "debugger stubs" for the mono debugger agent, which turns `mono_debugger_agent_unhandled_exception()` into an [*assertion*][0]: static void stub_debugger_unhandled_exception (MonoException *exc) { g_assert_not_reached (); } The result is that when an exception is thrown, *before* the `AppDomain.UnhandledException` event can be raised, the runtime dies in a horrible flaming assertion death: E andledexceptio: * Assertion: should not be reached at /__w/1/s/src/mono/mono/component/debugger-stub.c:175 Avoid this issue by checking `Debugger.IsAttached` *before* calling `monodroid_debugger_unhandled_exception()`. Additionally, remove some obsolete comments: .NET 6 couldn't resolve `Debugger.Mono_UnhandledException()` because .NET 6 never had it, so the linker was right to warn about its absence. [0]: https://github.com/dotnet/runtime/blob/16b456426dfb5212a24bfb78bfd5d9adfcc95185/src/mono/mono/component/debugger-stub.c#L172-L176
Context: c1a2ee7 Context: dotnet/runtime#44526 Context: dotnet/runtime#44526 (comment) Context: dotnet/runtime#44526 (comment) Context: start: https://discord.com/channels/732297728826277939/732297837953679412/869330822262587392 Context: end? https://discord.com/channels/732297728826277939/732297837953679412/869343082552893440 Context: dotnet/runtime#57304 Now that we are calling `mono_unhandled_exception()` when an unhandled exception is reported (c1a2ee7), we should be able re-enable the `InstallAndRunTests.SubscribeToAppDomainUnhandledException()` unit test on .NET 6, which was disabled in 6e3e383. What seemed like it should have been a 1-line removal ballooned a bit due to a confluence of factors: 1. Debugger component stubs, and 2. .NET 6 `Console.WriteLine()` behavior. On .NET 6, `JNIEnv.mono_unhandled_exception()` is `monodroid_debugger_unhandled_exception()`, which calls `mono_debugger_agent_unhandled_exception()`; see also e4debf7. The problem is that in our current world order of "Mono components" (0f7a0cd), if the debugger isn't used, then we get "debugger stubs" for the mono debugger agent, which turns `mono_debugger_agent_unhandled_exception()` into an [*assertion*][0]: static void stub_debugger_unhandled_exception (MonoException *exc) { g_assert_not_reached (); } The result is that when an exception is thrown, *before* the `AppDomain.UnhandledException` event can be raised, the runtime dies in a horrible flaming assertion death: E andledexceptio: * Assertion: should not be reached at /__w/1/s/src/mono/mono/component/debugger-stub.c:175 Avoid this issue by checking `Debugger.IsAttached` *before* calling `monodroid_debugger_unhandled_exception()`. Additionally, remove some obsolete comments: .NET 6 couldn't resolve `Debugger.Mono_UnhandledException()` because .NET 6 never had it, so the linker was right to warn about its absence. The problem with .NET 6 & `Console.WriteLine()` is that when format strings are used, the output may be line-wrapped in unexpected ways; see also dotnet/runtime@#57304. Additionally, the `sender` parameter value differs under .NET 6. These two issues broke our unit test; we *expected* `adb logcat` to contain: # Unhandled Exception: sender=RootDomain; e.IsTerminating=True; e.ExceptionObject=System.Exception: CRASH Under .NET 6, `adb logcat` *instead* contained: # Unhandled Exception: sender=System.Object; e.IsTerminating=True; e.ExceptionObject=System.Exception : CRASH Yes, `: CRASH` was on a separate `adb logcat` line. Fix the `SubscribeToAppDomainUnhandledException()` unit test so that `adb logcat` messages can now span multiple lines (which is sure to cause all sorts of GC garbage!), and update the expected message so that we look for the right message under legacy & .NET 6. Co-authored-by: Jonathan Pryor <[email protected]> [0]: https://github.com/dotnet/runtime/blob/16b456426dfb5212a24bfb78bfd5d9adfcc95185/src/mono/mono/component/debugger-stub.c#L172-L176
Description
I've been working on getting some integration tests working for Android in .NET 5/6, such as:
https://github.com/xamarin/xamarin-android/blob/5369da67a91c71aa646fd2642c71e519ae01cff9/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs#L63-L64
It appears that the
AppDomain.CurrentDomain.UnhandledException
event doesn't fire in an example such as:AppDomain.CurrentDomain.UnhandledException
seems to work fine in a .NET 5 console app:https://docs.microsoft.com/dotnet/api/system.appdomain.unhandledexception?view=net-5.0
Configuration
I was using a master build of Xamarin.Android, but I tried these .NET versions:
Regression?
No, Android is a new platform.
/cc @marek-safar @akoeplinger
The text was updated successfully, but these errors were encountered: