Skip to content
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

Mono & .NET 5 & The Debugger & Exceptions, oh my! #38465

Open
jonpryor opened this issue Jun 26, 2020 · 8 comments
Open

Mono & .NET 5 & The Debugger & Exceptions, oh my! #38465

jonpryor opened this issue Jun 26, 2020 · 8 comments
Assignees
Milestone

Comments

@jonpryor
Copy link
Member

Related: dotnet/android#4877
Related: dotnet/android#4864

Related: https://xamarinhq.slack.com/archives/C03CEGRUW/p1593198033319900
Related: https://files.slack.com/files-pri/T03C6CW9H-F0164ULFMEE/image.png

Xamarin.Android v10.x uses the mono/mono/2020-02 branch, and uses the System.Diagnostics.Debugger.Mono_UnhandledException() internal call when a debugger is attached, in order to simulate the first chance notification within the debugger.

Debugger.Mono_UnhandledException() does not exist in .NET 5 mono. Consequently, when an exception is thrown in a .NET 5 Xamarin.Android app, the debugger "first chance notification" behavior differs.

As such, when an exception is thrown and not caught, there is no "first chance" notification, nor is there a "last chance" notification. Instead, execution continues, the entire call stack is unwound, and then the exception is visible within the debugger.


This might be a Glorious Opportunity to rework how exceptions work within Xamarin.Android apps when a debugger is attached. dotnet/android#4877 describes the current state of affairs, and it's "not good." In particular, current behavior is that when a managed exception is caught while the debugger is attached, when execution continues mono will unwind stack frames, without allowing execution to return to Java code.

This basically means that if you have a Managed > Java > Managed callstack transition, the process is effectively unusable if the rightmost frame throws an exception while the process is being debugged.

(Not using a debugger results in "sane" exception handling, but means you don't have a debugger…)

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Diagnostics-coreclr untriaged New issue has not been triaged by the area owner labels Jun 26, 2020
@ghost
Copy link

ghost commented Jun 26, 2020

Tagging subscribers to this area: @tommcdon
Notify danmosemsft if you want to be subscribed.

@ghost
Copy link

ghost commented Jun 26, 2020

Tagging subscribers to this area: @thaystg
Notify danmosemsft if you want to be subscribed.

@lambdageek lambdageek added os-android runtime-mono specific to the Mono runtime labels Jun 26, 2020
@lambdageek lambdageek added this to the 6.0.0 milestone Jun 26, 2020
@lambdageek
Copy link
Member

I'm curious what the debugger situation is like on macos/ios - I recall we convert managed exceptions into objective-C exceptions and back at Managed->Native->Managed transitions - and the unwinding in native frames is handled by objC. But I don't know what the debugger behavior is like.

It would be good if Android and ios didn't diverge too much here...

@thaystg
Copy link
Member

thaystg commented Jun 26, 2020

I'm planning to take a look at this next week, to understand how iOS and Android works to understand which would be the best approach to follow on wasm and on Android using .NET 5 mono.
I will do some tests and I can write here everything that I understand. :)

@jonathanpeppers
Copy link
Member

There is also another call related to exceptions:

https://github.com/xamarin/xamarin-android/blob/4cae5f5e40896c69b7448cb78cf613cf6327c97c/src/Mono.Android/Android.Runtime/JNIEnv.cs#L251-L262

This allows the AppDomain.UnhandledException event to work, but AppDomain.DoUnhandledException does not exist in .NET 5.

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Jun 26, 2020
Context: dotnet/runtime#38465

Both of these methods are missing in .NET 5+:

* `System.Diagnostics.Debugger.Mono_UnhandledException`
* `AppDomain.CurrentDomain.DoUnhandledException`

The code path that runs during an unhandled exception relies on these
methods. Currently, an unhandled exception will throw several additional
exceptions we should fix:

    Unable to initialize UncaughtExceptionHandler. Nested exception caught: System.ArgumentNullException: Value cannot be null. (Parameter 'method')
        at System.Delegate.CreateDelegate(Type type, Object firstArgument, MethodInfo method, Boolean throwOnBindFailure, Boolean allowClosed)
        at System.Delegate.CreateDelegate(Type type, MethodInfo method, Boolean throwOnBindFailure)
        at System.Delegate.CreateDelegate(Type type, MethodInfo method)
        at Android.Runtime.JNIEnv.Initialize()
        at Android.Runtime.JNIEnv.PropagateUncaughtException(IntPtr env, IntPtr javaThreadPtr, IntPtr javaExceptionPtr)

And:

    Exception thrown while raising AppDomain.UnhandledException event: System.NullReferenceException: Object reference not set to an instance of an object
        at Android.Runtime.JNIEnv.PropagateUncaughtException(IntPtr env, IntPtr javaThreadPtr, IntPtr javaExceptionPtr)

Added the appropriate null checks in `JNIEnv.PropagateUncaughtException`
to fix the additional exceptions.
@marek-safar marek-safar removed the untriaged New issue has not been triaged by the area owner label Jun 30, 2020
jonpryor pushed a commit to dotnet/android that referenced this issue Jun 30, 2020
Context: dotnet/runtime#38465

When a thread throws an exception which isn't caught, we (eventually)
hit the "unhandled exception" codepath of
`JNIEnv.PropagateUncaughtException()` (2aff4e7 & others), which
relies on the following methods which are not present in .NET 5:

  * `System.Diagnostics.Debugger.Mono_UnhandledException()`
  * `AppDomain.DoUnhandledException()`

Unfortunately, we didn't check that the `MethodInfo`s for these
methods were `null` before invoking them, which means under .NET 5 we
would throw an exception while attempting to deal with an already
"in-flight" exception, i.e. we'd throw a *nested* exception!

	Unable to initialize UncaughtExceptionHandler. Nested exception caught: System.ArgumentNullException: Value cannot be null. (Parameter 'method')
	    at System.Delegate.CreateDelegate(Type type, Object firstArgument, MethodInfo method, Boolean throwOnBindFailure, Boolean allowClosed)
	    at System.Delegate.CreateDelegate(Type type, MethodInfo method, Boolean throwOnBindFailure)
	    at System.Delegate.CreateDelegate(Type type, MethodInfo method)
	    at Android.Runtime.JNIEnv.Initialize()
	    at Android.Runtime.JNIEnv.PropagateUncaughtException(IntPtr env, IntPtr javaThreadPtr, IntPtr javaExceptionPtr)

or:

	Exception thrown while raising AppDomain.UnhandledException event: System.NullReferenceException: Object reference not set to an instance of an object
	    at Android.Runtime.JNIEnv.PropagateUncaughtException(IntPtr env, IntPtr javaThreadPtr, IntPtr javaExceptionPtr)

Add appropriate `null` checks in `JNIEnv.PropagateUncaughtException()`
to avoid generation of nested exceptions.

TODO: Work with the dotnet/mono teams so that we can reintroduce the
functionality of `AppDomain.DoUnhandledException()`/etc..
@jonathanpeppers
Copy link
Member

Repro here: dotnet/android#4927 (comment)

@jonpryor
Copy link
Member Author

jonpryor commented Jul 7, 2021

Related: dotnet/android#4927 (comment)

.NET 6 project: https://github.com/xamarin/xamarin-android/files/6779689/Scratch.JMJMException.zip

Unhandled exception handling is broken on .NET 6 when running within VS 2022: instead of getting a first-chance exception, or any form of unhandled exception dialog, apps crash immediately if they have an unhandled exception. See this adb logcat output:

I DOTNET  : # jon: Should be in a Java > Managed [MainActivity.OnCreate] > Java [Run.tryCatchFinally] > Managed [Run] frame. Throwing an exception...
I DOTNET  :    at Scratch.JMJMException.MainActivity.<>c.<OnCreate>b__0_1() in C:\Users\jopepper\Downloads\Scratch.JMJMException\Scratch.JMJMException\MainActivity.cs:line 28
I DOTNET  :    at Java.Lang.Runnable.Run() in Mono.Android.dll:token 0x601ef04+0x0
I DOTNET  :    at Java.Lang.IRunna
I DOTNET  : bleInvoker.n_Run(IntPtr jnienv, IntPtr native__this) in Mono.Android.dll:token 0x601e900+0x0
I DOTNET  :    at Android.Runtime.DynamicMethodNameCounter.5(IntPtr , IntPtr ) in Mono.Android.dll:token 0x0+0x0
I DOTNET  :    at Java.Interop.NativeMethods.java_interop_jnienv_call_stat
I DOTNET  : ic_void_method_a(IntPtr , IntPtr& , IntPtr , IntPtr , IntPtr ) in Java.Interop.dll:token 0x0+0x0
I DOTNET  :    at Java.Interop.JniEnvironment.StaticMethods.CallStaticVoidMethod(JniObjectReference type, JniMethodInfo method, JniArgumentValue* args) in Java.Interop.dll
I DOTNET  : :token 0x6000593+0x0
I DOTNET  :    at Java.Interop.JniPeerMembers.JniStaticMethods.InvokeVoidMethod(String encodedMember, JniArgumentValue* parameters) in Java.Interop.dll:token 0x600060a+0x0
I DOTNET  :    at Testing.Run.TryCatchFinally(IRunnable p0, ICatchThrowableHandler p1, 
I DOTNET  : IRunnable p2) in C:\Users\jopepper\Downloads\Scratch.JMJMException\Scratch.Try\obj\Debug\generated\src\Testing.Run.cs:line 142
I DOTNET  :    at Scratch.JMJMException.MainActivity.<>c.<OnCreate>b__0_0(Object o, EventArgs e) in C:\Users\jopepper\Downloads\Scratch.JMJME
I DOTNET  : xception\Scratch.JMJMException\MainActivity.cs:line 24
I DOTNET  :    at Android.Views.View.IOnClickListenerImplementor.OnClick(View v) in Mono.Android.dll:token 0x6020b2f+0x0
I DOTNET  :    at Android.Views.View.IOnClickListenerInvoker.n_OnClick_Landroid_view_View_(IntPtr jnienv
I DOTNET  : , IntPtr native__this, IntPtr native_v) in Mono.Android.dll:token 0x6020b2b+0x0
I DOTNET  :    at Android.Runtime.DynamicMethodNameCounter.4(IntPtr , IntPtr , IntPtr ) in Mono.Android.dll:token 0x0+0x0
I DOTNET  : 
W monodroid: MonodroidRuntime::monodroid_pinvoke_override ("java-interop", "java_interop_jnienv_throw")
W monodroid: Found java-interop@java_interop_jnienv_throw in internal p/invoke map (0xcb5a3440)
W monodroid: MonodroidRuntime::monodroid_pinvoke_override ("java-interop", "java_interop_jnienv_throw")
W monodroid: Found java-interop@java_interop_jnienv_throw in internal p/invoke map (0xcb5a3440)
W monodroid: MonodroidRuntime::monodroid_pinvoke_override ("java-interop", "java_interop_jnienv_throw")
W monodroid: Found java-interop@java_interop_jnienv_throw in internal p/invoke map (0xcb5a3440)
--------- beginning of crash
F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x7 in tid 25815 (h.jmjmexception), pid 25815 (h.jmjmexception)
I crash_dump32: obtaining output fd from tombstoned, type: kDebuggerdTombstone
I crash_dump32: performing dump of process 25815 (target tid = 25815)
F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
F DEBUG   : Build fingerprint: 'google/sdk_gphone_x86_64/generic_x86_64:9/PSR1.180720.075/5124027:user/release-keys'
F DEBUG   : Revision: '0'
F DEBUG   : ABI: 'x86'
F DEBUG   : pid: 25815, tid: 25815, name: h.jmjmexception  >>> scratch.jmjmexception <<<
F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x7
F DEBUG   : Cause: null pointer dereference
F DEBUG   :     eax ffd85a71  ebx e5ff586c  ecx 00000000  edx 00000000
F DEBUG   :     edi ffd86288  esi 00000003
F DEBUG   :     ebp ffd86268  esp ffd86120  eip e5e35618
F DEBUG   : 
F DEBUG   : backtrace:
F DEBUG   :     #00 pc 00532618  /system/lib/libart.so (_ZN3art12StackVisitor9WalkStackILNS0_16CountTransitionsE0EEEvb+584)
F DEBUG   :     #01 pc 00548615  /system/lib/libart.so (art::Thread::DumpStack(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, bool, BacktraceMap*, bool) const+501)
F DEBUG   :     #02 pc 00542c83  /system/lib/libart.so (art::Thread::Dump(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, bool, BacktraceMap*, bool) const+83)
F DEBUG   :     #03 pc 003339d3  /system/lib/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+1283)
F DEBUG   :     #04 pc 00333ea1  /system/lib/libart.so (art::JavaVMExt::JniAbortV(char const*, char const*, char*)+113)
F DEBUG   :     #05 pc 000db967  /system/lib/libart.so (art::(anonymous namespace)::ScopedCheck::AbortF(char const*, ...)+71)
F DEBUG   :     #06 pc 000da1ae  /system/lib/libart.so (art::(anonymous namespace)::ScopedCheck::CheckPossibleHeapValue(art::ScopedObjectAccess&, char, art::(anonymous namespace)::JniValueType)+1246)
F DEBUG   :     #07 pc 000d92db  /system/lib/libart.so (art::(anonymous namespace)::ScopedCheck::Check(art::ScopedObjectAccess&, bool, char const*, art::(anonymous namespace)::JniValueType*)+827)
F DEBUG   :     #08 pc 000ca8c7  /system/lib/libart.so (art::(anonymous namespace)::CheckJNI::NewString(_JNIEnv*, unsigned short const*, int)+1175)
F DEBUG   :     #09 pc 00046a77  /data/app/scratch.jmjmexception-eDiobdrrJUpLOWxCjyoK6g==/lib/x86/libmonodroid.so (java_interop_jnienv_new_string+55)
F DEBUG   :     #10 pc 00007871  <anonymous:e58ff000>

Also (unrelatedly) odd is the line wrapping for the C# stack trace message, e.g.

I DOTNET  :    at Java.Lang.IRunna
I DOTNET  : bleInvoker.n_Run(IntPtr jnienv, IntPtr native__this) in Mono.Android.dll:token 0x601e900+0x0
I DOTNET  :    at Android.Runtime.DynamicMethodNameCounter.5(IntPtr , IntPtr ) in Mono.Android.dll:token 0x0+0x0

@jonpryor
Copy link
Member Author

jonpryor commented Jul 8, 2021

@thaystg thaystg modified the milestones: 6.0.0, 7.0.0 Jul 28, 2021
@thaystg thaystg modified the milestones: 7.0.0, Future Jul 27, 2022
@ghost ghost added untriaged New issue has not been triaged by the area owner and removed untriaged New issue has not been triaged by the area owner labels Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants