[docs] Document Exception Handling semantics #4877
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Related: #4927 (comment) |
jonpryor
pushed a commit
that referenced
this pull request
Jul 23, 2021
[One .NET] Use Mono embedding API for exception debugger notification (#6106) Context: dotnet/runtime#56071 Context: #4877 Context: #4927 (comment) Context: #4927 (comment) Context: xamarin/monodroid@3e9de5a Context: xamarin/monodroid@b0f8597 Context: xamarin/monodroid@12a012e What should happen when an exception is thrown and a debugger is attached? This is in fact a loaded question: there's what Xamarin.Android (Legacy) *has* done, vs. what .NET 6 Android for .NET does, vs. what "should" happen. What "should" happen is easiest: 1. We should behave like a "normal" Desktop .NET app when a debugger is attached, AND 2. We shouldn't corrupt JVM state. Unfortunately, (1)+(2) is currently not possible, in part because Java doesn't have an equivalent to Windows' [two attempt][0] debugger notification infrastructure. See #4877 for details. What Legacy Xamarin.Android does is also detailed in #4877, and relies on the `Debugger.Mono_UnhandledException()` method in order to alert an attached debugger that there is an exception to show to the user. However, `Debugger.Mono_UnhandledException()` never made it to the `dotnet/runtime` repo. It never existed there. Thus, what .NET 6 Android for .NET *currently* does is…*nothing*. If an exception is thrown and a debugger is attached, the debugger is *not* notified. Eventually you'll get an unhandled exception, long after it was originally thrown; see commit c1a2ee7. PR dotnet/runtime#56071 added a new `mono_debugger_agent_unhandled_exception()` Mono embedding API which is equivalent to `Debugger.Mono_UnhandledException()` for use with .NET 6 + MonoVM. Update `src/Mono.Android` and `src/monodroid` so that `mono_debugger_agent_unhandled_exception()` is used to alert the debugger that an exception has been thrown at a JNI boundary. This should allow .NET 6 + Android to have equivalent exception handling semantics as legacy Xamarin.Android. [0]: https://docs.microsoft.com/en-us/windows/win32/debug/debugger-exception-handling
jonpryor
pushed a commit
that referenced
this pull request
Jul 23, 2021
[One .NET] Use Mono embedding API for exception debugger notification (#6106) Context: dotnet/runtime#56071 Context: #4877 Context: #4927 (comment) Context: #4927 (comment) Context: xamarin/monodroid@3e9de5a Context: xamarin/monodroid@b0f8597 Context: xamarin/monodroid@12a012e What should happen when an exception is thrown and a debugger is attached? This is in fact a loaded question: there's what Xamarin.Android (Legacy) *has* done, vs. what .NET 6 Android for .NET does, vs. what "should" happen. What "should" happen is easiest: 1. We should behave like a "normal" Desktop .NET app when a debugger is attached, AND 2. We shouldn't corrupt JVM state. Unfortunately, (1)+(2) is currently not possible, in part because Java doesn't have an equivalent to Windows' [two attempt][0] debugger notification infrastructure. See #4877 for details. What Legacy Xamarin.Android does is also detailed in #4877, and relies on the `Debugger.Mono_UnhandledException()` method in order to alert an attached debugger that there is an exception to show to the user. However, `Debugger.Mono_UnhandledException()` never made it to the `dotnet/runtime` repo. It never existed there. Thus, what .NET 6 Android for .NET *currently* does is…*nothing*. If an exception is thrown and a debugger is attached, the debugger is *not* notified. Eventually you'll get an unhandled exception, long after it was originally thrown; see commit c1a2ee7. PR dotnet/runtime#56071 added a new `mono_debugger_agent_unhandled_exception()` Mono embedding API which is equivalent to `Debugger.Mono_UnhandledException()` for use with .NET 6 + MonoVM. Update `src/Mono.Android` and `src/monodroid` so that `mono_debugger_agent_unhandled_exception()` is used to alert the debugger that an exception has been thrown at a JNI boundary. This should allow .NET 6 + Android to have equivalent exception handling semantics as legacy Xamarin.Android. [0]: https://docs.microsoft.com/en-us/windows/win32/debug/debugger-exception-handling
jonpryor
pushed a commit
that referenced
this pull request
Jan 28, 2022
…6657) Context: https://github.com/xamarin/xamarin-android/wiki/Blueprint#java-type-registration Context: b7a368a Context: #4877 Context: #4927 (comment) In order for Java code to call C# code, [`JNIEnv::RegisterNatives()][0] must be invoked, providing an array of `JNINativeMethod` structures, each of which contains a function pointer to invoke, kept in `JNINativeMethod::fnPtr`. Fortunately, delegates marshal as function pointers, and there is a bunch of `generator`-emitted infrastructure and coordination with Java Callable Wrappers to eventually obtain a Delegate instance to provide `JNIEnv::RegisterNatives()`. There is one deficiency in the `generator`-emitted infrastructure: it doesn't deal with C# exceptions. However, exceptions "can't" cross the JNI boundary (see b7a368a for an example of the breakage that results when exceptions do cross the boundary!), except when we *do* want exceptions to cross the JNI boundary ("improved" IDE first chance exception experience; see #4877). This "we want to catch exceptions, except when we don't" scenario has existed since the very beginning. As "the very beginning" predates [C# 4 exception filters][1], there wasn't a way for `generator` output to "selectively `catch` exceptions". We squared this circle by using `System.Reflection.Emit`: 1. During Java Callable Wrapper registration, we lookup the "marshal method getter" as provided to the `Runtime.register()` invocation, e.g. `Android.App.Activity.GetOnCreate_Landroid_os_Bundle_Handler()`. 2. `GetOnCreate_Landroid_os_Bundle_Handler()` is `generator` output, and contains a `JNINativeWrapper.CreateDelegate()` invocation: cb_onCreate_Landroid_os_Bundle_ = JNINativeWrapper.CreateDelegate ((_JniMarshal_PPL_V) n_OnCreate_Landroid_os_Bundle_); 3. `JNINativeWrapper.CreateDelegate()` uses `System.Reflection.Emit` to create a new delegate instance which *wraps* the marshal method `Activity.n_OnCreate_Landroid_os_Bundle()` in a `try`/*filtered* `catch` block and marshals the exception to Java; `JNINativeWrapper.CreateDelegate()` effectively returns: bool _run_catch_if_debugger_not_attached (Exception e) { if (Debugger.IsAttached || !JNIEnv.PropagateExceptions) { JNIEnv.mono_unhandled_exception (e); return false; } return true; } _JniMarshal_PPL_V result = (jnienv, native__this, native_savedInstanceState) => { JNIEnv.WaitForBridgeProcessing (); try { Activity.n_OnCreate_Landroid_os_Bundle_ (jnienv, native__this, native_savedInstanceState); } catch (Exception e) when (_run_catch_if_debugger_not_attached (e)) { AndroidEnvironment.UnhandledException (e); if (Debugger.IsAttached || !JNIEnv.PropagateExceptions) throw; } }; return result; Again, this was C# 2.0 at the time, so C# 4 exception filters couldn't be used, thus the need for `System.Reflection.Emit`, so that [`ILGenerator.BeginExceptionFilterBLock()`][2] could be used (the support for which is a Mono extension). After this point, use of `System.Reflection.Emit` was part of the implicit ABI between Xamarin.Android and binding assemblies. While `generator` *could* be updated to *itself* emit the `try`/`catch` block with exception filters, that would only work for binding assemblies released *after* that `generator` fix. The `System.Reflection.Emit` wrapper *can't* be skipped without breaking semantic compatibility, *or* without allowing C# exceptions to always pass through a JNI boundary, which would be Bad™. The use of `System.Refleciton.Emit` is a Known Problem™, and something we'd *like* to remove. (Hence the [`jnimarshalmethod-gen`][3] explorations…) With that background out of the way… Let us turn our attention to the `dotnet new maui` template. The default MAUI template hits `JNINativeWrapper.CreateDelegate()` 58 times during process startup, and we were wondering if we could selectively improve these particular invocations, without needing to re-think the entire "marshal method" infrastructure. *Partially specialize* `JNINativeWrapper.CreateDelegate()` for the following delegate types: * `_JniMarshal_PP_V` * `_JniMarshal_PPI_V` * `_JniMarshal_PPL_L` * `_JniMarshal_PPL_V` * `_JniMarshal_PPL_Z` * `_JniMarshal_PPII_V` * `_JniMarshal_PPLI_V` * `_JniMarshal_PPLL_V` * `_JniMarshal_PPLL_Z` * `_JniMarshal_PPIIL_V` * `_JniMarshal_PPILL_V` * `_JniMarshal_PPLIL_Z` * `_JniMarshal_PPLLL_L` * `_JniMarshal_PPLLL_Z` * `_JniMarshal_PPIIII_V` * `_JniMarshal_PPLLLL_V` * `_JniMarshal_PPLIIII_V` * `_JniMarshal_PPZIIII_V` * `_JniMarshal_PPLIIIIIIII_V` This is done via use of a T4 template, which generates `JNINativeWrapper.CreateBuiltInDelegate()`, and `JNINativeWrapper.CreateDelegate()` is updated to call `CreateBuiltInDelegate()`: partial class JNINativeWrapper { static Delegate? CreateBuiltInDelegate (Delegate dlg, Type delegateType) { switch (delegateType.Name) { case "_JniMarshal_PP_V": return … case "_JniMarshal_PPI_V": return … … } return null; } public static Delegate CreateDelegate (Delegate dlg) { … var builtin = CreateBuiltInDelegate (dlg, dlg.GetType (); if (builtin != null) return builtin; … } } This avoids use of `System.Reflection.Emit` for the specified types. Other changes: * Update `TypeManager.GetActivateHandler()` to use `_JniMarshal_PPLLLL_V` instead of `Action<IntPtr, IntPtr, IntPtr, IntPtr, IntPtr, IntPtr>`, so the fast path can be used. * Added a log message for `adb shell septprop debug.mono.log assembly`: Falling back to System.Reflection.Emit for delegate type '{delegateType}': {dlg.Method} * I was also able to remove `mono_unhandled_exception_method` from `JNINativeWrapper` as we already has this value in `JNIEnv`. ~~ Results ~~ Testing `dotnet new maui` with version: msbuild Xamarin.Android.sln -t:InstallMaui -bl -p:MauiVersion=6.0.200-preview.13.2536 A `Release` build on a Pixel 5 device, total startup time: | Startup | Average (ms) | Std Err (ms) | Std Dev (ms) | | --------- | ------------: | ------------: | ------------: | | Before | 1106.3 | 6.919 | 21.879 | | After | 1078.8 | 5.438 | 17.197 | This might save ~35ms on average? If I time the message for one call, [such as][4]: I monodroid-timing: Runtime.register: registering type `Microsoft.Maui.MauiApplication, Microsoft.Maui, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null` I monodroid-timing: Runtime.register: end time; elapsed: 0s:17::794845 The result is: | One Call | Average (ms) | Std Err (ms) | Std Dev (ms) | | --------- | ------------: | ------------: | ------------: | | Before | 23.925 | 0.050 | 0.159 | | After | 18.723 | 0.094 | 0.298 | Saving ~5.8ms for this one call. `.apk` size difference for `dotnet new android`: % apkdiff -f before.apk after.apk Size difference in bytes ([*1] apk1 only, [*2] apk2 only): + 3,390 assemblies/assemblies.blob + 54 assemblies/assemblies.x86_64.blob - 4 assemblies/assemblies.arm64_v8a.blob - 15 assemblies/assemblies.x86.blob - 65 assemblies/assemblies.armeabi_v7a.blob Summary: + 3,360 Other entries 0.03% (of 10,526,432) + 0 Dalvik executables 0.00% (of 7,816,392) + 0 Shared libraries 0.00% (of 18,414,404) + 4,096 Package size difference 0.02% (of 21,006,128) We're looking at a ~4KB size increase for this partial specialization. [0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#RegisterNatives [1]: https://docs.microsoft.com/en-us/dotnet/csharp/fundamentals/exceptions/exception-handling#catch-blocks [2]: https://docs.microsoft.com/is-is/dotnet/api/system.reflection.emit.ilgenerator.beginexceptfilterblock?view=net-6.0 [3]: http://github.com/xamarin/Java.Interop/commit/c8f3e51a6cfd78bdce89e2429efae4495481f57b [4]: https://github.com/dotnet/maui/blob/bfba62ed796d3416c4fcaa7cfbea86dc8d5e04c2/src/Compatibility/ControlGallery/src/Android/MainApplication.cs
jonpryor
pushed a commit
that referenced
this pull request
Aug 23, 2023
…8185) Context: #1198 Context: #1188 (comment) Context: #4877 Context: #4927 (comment) What happens with unhandled exceptions? throw new InvalidOperationException ("oops!"); This is a surprisingly complicated question: If this happens when a debugger is attached, the debugger will get a "first chance notification" at the `throw` site. If execution continues, odds are high that the app will abort if there is a JNI transition in the callstack. If no debugger is attached, then it depends on which thread threw the unhandled exception. If the thread which threw the unhandled exception is a .NET Thread: static void ThrowFromAnotherManagedThread() { var t = new System.Threading.Thread(() => { throw new new Java.Lang.Error ("from another thread?!"); }); t.Start (); t.Join (); } Then .NET will report the unhandled exception, *and* the app will restart: F mono-rt : [ERROR] FATAL UNHANDLED EXCEPTION: System.InvalidOperationException: oops! F mono-rt : at android_unhandled_exception.MainActivity.<>c.<ThrowFromAnotherManagedThread>b__1_0() F mono-rt : at System.Threading.Thread.StartCallback() # app restarts If the thread which threw the unhandled exception is a *Java* thread, which could be the UI thread (e.g. thrown from an `Activity.OnCreate()` override) or via a `Java.Lang.Thread` instance: static void ThrowFromAnotherJavaThread() { var t = new Java.Lang.Thread(() => { throw new InvalidOperationException ("oops!"); }); t.Start (); t.Join (); } Then .NET will report the unhandled exception, *and* the app will *not* restart (which differs from using .NET threads): E AndroidRuntime: Process: com.companyname.android_unhandled_exception, PID: 5436 E AndroidRuntime: android.runtime.JavaProxyThrowable: System.InvalidOperationException: oops! E AndroidRuntime: at android_unhandled_exception.MainActivity.<>c.<ThrowFromAnotherJavaThread>b__2_0() E AndroidRuntime: at Java.Lang.Thread.RunnableImplementor.Run() E AndroidRuntime: at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) E AndroidRuntime: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) E AndroidRuntime: at mono.java.lang.RunnableImplementor.n_run(Native Method) E AndroidRuntime: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) E AndroidRuntime: at java.lang.Thread.run(Thread.java:1012) I MonoDroid: Android.Runtime.JavaProxyThrowable: Exception_WasThrown, Android.Runtime.JavaProxyThrowable I MonoDroid: I MonoDroid: --- End of managed Android.Runtime.JavaProxyThrowable stack trace --- I MonoDroid: android.runtime.JavaProxyThrowable: System.InvalidOperationException: oops! I MonoDroid: at android_unhandled_exception.MainActivity.<>c.<ThrowFromAnotherJavaThread>b__2_0() I MonoDroid: at Java.Lang.Thread.RunnableImplementor.Run() I MonoDroid: at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) I MonoDroid: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) I MonoDroid: at mono.java.lang.RunnableImplementor.n_run(Native Method) I MonoDroid: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) I MonoDroid: at java.lang.Thread.run(Thread.java:1012) I MonoDroid: I MonoDroid: --- End of managed Android.Runtime.JavaProxyThrowable stack trace --- I MonoDroid: android.runtime.JavaProxyThrowable: System.InvalidOperationException: oops! I MonoDroid: at android_unhandled_exception.MainActivity.<>c.<ThrowFromAnotherJavaThread>b__2_0() I MonoDroid: at Java.Lang.Thread.RunnableImplementor.Run() I MonoDroid: at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) I MonoDroid: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) I MonoDroid: at mono.java.lang.RunnableImplementor.n_run(Native Method) I MonoDroid: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) I MonoDroid: at java.lang.Thread.run(Thread.java This "works", until we enter the world of crash logging for later diagnosis and fixing. The problem with our historical approach is that we would "stuff" the .NET stack trace into the "message" of the Java-side `Throwable` instance, and the "message" may not be transmitted as part of the crash logging! (This is noticeable by the different indentation levels for the `at …` lines in the crash output. Three space indents are from the `Throwable.getMessage()` output, while four space indents are from the Java-side stack trace.) We *think* that we can improve this by replacing the Java-side stack trace with a "merged" stack trace which includes both the Java-side and .NET-side stack traces. This does nothing for unhandled exceptions on .NET threads, but does alter the output from Java threads: E AndroidRuntime: FATAL EXCEPTION: Thread-3 E AndroidRuntime: Process: com.companyname.android_unhandled_exception, PID: 12321 E AndroidRuntime: android.runtime.JavaProxyThrowable: [System.InvalidOperationException]: oops! E AndroidRuntime: at android_unhandled_exception.MainActivity+<>c.<ThrowFromAnotherJavaThread>b__2_0(Unknown Source:0) E AndroidRuntime: at Java.Lang.Thread+RunnableImplementor.Run(Unknown Source:0) E AndroidRuntime: at Java.Lang.IRunnableInvoker.n_Run(Unknown Source:0) E AndroidRuntime: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(Unknown Source:0) E AndroidRuntime: at mono.java.lang.RunnableImplementor.n_run(Native Method) E AndroidRuntime: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) E AndroidRuntime: at java.lang.Thread.run(Thread.java:1012) I MonoDroid: UNHANDLED EXCEPTION: I MonoDroid: Android.Runtime.JavaProxyThrowable: Exception_WasThrown, Android.Runtime.JavaProxyThrowable I MonoDroid: I MonoDroid: --- End of managed Android.Runtime.JavaProxyThrowable stack trace --- I MonoDroid: android.runtime.JavaProxyThrowable: [System.InvalidOperationException]: oops! I MonoDroid: at android_unhandled_exception.MainActivity+<>c.<ThrowFromAnotherJavaThread>b__2_0(Unknown Source:0) I MonoDroid: at Java.Lang.Thread+RunnableImplementor.Run(Unknown Source:0) I MonoDroid: at Java.Lang.IRunnableInvoker.n_Run(Unknown Source:0) I MonoDroid: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(Unknown Source:0) I MonoDroid: at mono.java.lang.RunnableImplementor.n_run(Native Method) I MonoDroid: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) I MonoDroid: at java.lang.Thread.run(Thread.java:1012) I MonoDroid: I MonoDroid: --- End of managed Android.Runtime.JavaProxyThrowable stack trace --- I MonoDroid: android.runtime.JavaProxyThrowable: [System.InvalidOperationException]: oops! I MonoDroid: at android_unhandled_exception.MainActivity+<>c.<ThrowFromAnotherJavaThread>b__2_0(Unknown Source:0) I MonoDroid: at Java.Lang.Thread+RunnableImplementor.Run(Unknown Source:0) I MonoDroid: at Java.Lang.IRunnableInvoker.n_Run(Unknown Source:0) I MonoDroid: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(Unknown Source:0) I MonoDroid: at mono.java.lang.RunnableImplementor.n_run(Native Method) I MonoDroid: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) I MonoDroid: at java.lang.Thread.run(Thread.java:1012) Note how `at …` is always a four-space indent and always lines up. *Hopefully* this means that crash loggers can provide more useful information. TODO: * Create an "end-to-end" test which uses an actual crash logger (which one?) in order to better understand what the "end user" experience is. * The "merged" stack trace always places the managed stack trace above the Java-side stack trace. This means things will look "weird"/"wrong" if you have an *intermixed* stack trace, e.g. (Java code calls .NET code which calls Java code)+ which eventually throws from .NET.
jonathanpeppers
pushed a commit
that referenced
this pull request
Aug 23, 2023
…8185) Context: #1198 Context: #1188 (comment) Context: #4877 Context: #4927 (comment) What happens with unhandled exceptions? throw new InvalidOperationException ("oops!"); This is a surprisingly complicated question: If this happens when a debugger is attached, the debugger will get a "first chance notification" at the `throw` site. If execution continues, odds are high that the app will abort if there is a JNI transition in the callstack. If no debugger is attached, then it depends on which thread threw the unhandled exception. If the thread which threw the unhandled exception is a .NET Thread: static void ThrowFromAnotherManagedThread() { var t = new System.Threading.Thread(() => { throw new new Java.Lang.Error ("from another thread?!"); }); t.Start (); t.Join (); } Then .NET will report the unhandled exception, *and* the app will restart: F mono-rt : [ERROR] FATAL UNHANDLED EXCEPTION: System.InvalidOperationException: oops! F mono-rt : at android_unhandled_exception.MainActivity.<>c.<ThrowFromAnotherManagedThread>b__1_0() F mono-rt : at System.Threading.Thread.StartCallback() # app restarts If the thread which threw the unhandled exception is a *Java* thread, which could be the UI thread (e.g. thrown from an `Activity.OnCreate()` override) or via a `Java.Lang.Thread` instance: static void ThrowFromAnotherJavaThread() { var t = new Java.Lang.Thread(() => { throw new InvalidOperationException ("oops!"); }); t.Start (); t.Join (); } Then .NET will report the unhandled exception, *and* the app will *not* restart (which differs from using .NET threads): E AndroidRuntime: Process: com.companyname.android_unhandled_exception, PID: 5436 E AndroidRuntime: android.runtime.JavaProxyThrowable: System.InvalidOperationException: oops! E AndroidRuntime: at android_unhandled_exception.MainActivity.<>c.<ThrowFromAnotherJavaThread>b__2_0() E AndroidRuntime: at Java.Lang.Thread.RunnableImplementor.Run() E AndroidRuntime: at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) E AndroidRuntime: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) E AndroidRuntime: at mono.java.lang.RunnableImplementor.n_run(Native Method) E AndroidRuntime: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) E AndroidRuntime: at java.lang.Thread.run(Thread.java:1012) I MonoDroid: Android.Runtime.JavaProxyThrowable: Exception_WasThrown, Android.Runtime.JavaProxyThrowable I MonoDroid: I MonoDroid: --- End of managed Android.Runtime.JavaProxyThrowable stack trace --- I MonoDroid: android.runtime.JavaProxyThrowable: System.InvalidOperationException: oops! I MonoDroid: at android_unhandled_exception.MainActivity.<>c.<ThrowFromAnotherJavaThread>b__2_0() I MonoDroid: at Java.Lang.Thread.RunnableImplementor.Run() I MonoDroid: at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) I MonoDroid: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) I MonoDroid: at mono.java.lang.RunnableImplementor.n_run(Native Method) I MonoDroid: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) I MonoDroid: at java.lang.Thread.run(Thread.java:1012) I MonoDroid: I MonoDroid: --- End of managed Android.Runtime.JavaProxyThrowable stack trace --- I MonoDroid: android.runtime.JavaProxyThrowable: System.InvalidOperationException: oops! I MonoDroid: at android_unhandled_exception.MainActivity.<>c.<ThrowFromAnotherJavaThread>b__2_0() I MonoDroid: at Java.Lang.Thread.RunnableImplementor.Run() I MonoDroid: at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) I MonoDroid: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) I MonoDroid: at mono.java.lang.RunnableImplementor.n_run(Native Method) I MonoDroid: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) I MonoDroid: at java.lang.Thread.run(Thread.java This "works", until we enter the world of crash logging for later diagnosis and fixing. The problem with our historical approach is that we would "stuff" the .NET stack trace into the "message" of the Java-side `Throwable` instance, and the "message" may not be transmitted as part of the crash logging! (This is noticeable by the different indentation levels for the `at …` lines in the crash output. Three space indents are from the `Throwable.getMessage()` output, while four space indents are from the Java-side stack trace.) We *think* that we can improve this by replacing the Java-side stack trace with a "merged" stack trace which includes both the Java-side and .NET-side stack traces. This does nothing for unhandled exceptions on .NET threads, but does alter the output from Java threads: E AndroidRuntime: FATAL EXCEPTION: Thread-3 E AndroidRuntime: Process: com.companyname.android_unhandled_exception, PID: 12321 E AndroidRuntime: android.runtime.JavaProxyThrowable: [System.InvalidOperationException]: oops! E AndroidRuntime: at android_unhandled_exception.MainActivity+<>c.<ThrowFromAnotherJavaThread>b__2_0(Unknown Source:0) E AndroidRuntime: at Java.Lang.Thread+RunnableImplementor.Run(Unknown Source:0) E AndroidRuntime: at Java.Lang.IRunnableInvoker.n_Run(Unknown Source:0) E AndroidRuntime: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(Unknown Source:0) E AndroidRuntime: at mono.java.lang.RunnableImplementor.n_run(Native Method) E AndroidRuntime: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) E AndroidRuntime: at java.lang.Thread.run(Thread.java:1012) I MonoDroid: UNHANDLED EXCEPTION: I MonoDroid: Android.Runtime.JavaProxyThrowable: Exception_WasThrown, Android.Runtime.JavaProxyThrowable I MonoDroid: I MonoDroid: --- End of managed Android.Runtime.JavaProxyThrowable stack trace --- I MonoDroid: android.runtime.JavaProxyThrowable: [System.InvalidOperationException]: oops! I MonoDroid: at android_unhandled_exception.MainActivity+<>c.<ThrowFromAnotherJavaThread>b__2_0(Unknown Source:0) I MonoDroid: at Java.Lang.Thread+RunnableImplementor.Run(Unknown Source:0) I MonoDroid: at Java.Lang.IRunnableInvoker.n_Run(Unknown Source:0) I MonoDroid: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(Unknown Source:0) I MonoDroid: at mono.java.lang.RunnableImplementor.n_run(Native Method) I MonoDroid: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) I MonoDroid: at java.lang.Thread.run(Thread.java:1012) I MonoDroid: I MonoDroid: --- End of managed Android.Runtime.JavaProxyThrowable stack trace --- I MonoDroid: android.runtime.JavaProxyThrowable: [System.InvalidOperationException]: oops! I MonoDroid: at android_unhandled_exception.MainActivity+<>c.<ThrowFromAnotherJavaThread>b__2_0(Unknown Source:0) I MonoDroid: at Java.Lang.Thread+RunnableImplementor.Run(Unknown Source:0) I MonoDroid: at Java.Lang.IRunnableInvoker.n_Run(Unknown Source:0) I MonoDroid: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(Unknown Source:0) I MonoDroid: at mono.java.lang.RunnableImplementor.n_run(Native Method) I MonoDroid: at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:31) I MonoDroid: at java.lang.Thread.run(Thread.java:1012) Note how `at …` is always a four-space indent and always lines up. *Hopefully* this means that crash loggers can provide more useful information. TODO: * Create an "end-to-end" test which uses an actual crash logger (which one?) in order to better understand what the "end user" experience is. * The "merged" stack trace always places the managed stack trace above the Java-side stack trace. This means things will look "weird"/"wrong" if you have an *intermixed* stack trace, e.g. (Java code calls .NET code which calls Java code)+ which eventually throws from .NET.
jonpryor
pushed a commit
to dotnet/java-interop
that referenced
this pull request
Dec 4, 2024
…1275) Fixes: #1258 Context: c8f3e51 Context: 176240d Context: dotnet/runtime#108211 Context: dotnet/android#9306 Context: dotnet/android#9309 Context: xamarin/monodroid@3e9de5a Context: dotnet/android@8bc7a3e The [Java Native Interface][0] allows native code to be associated with a [Java `native` method declaration][1], either by way of [`Java_`-prefixed native functions][2], or via function pointers provided to [`JNIEnv::RegisterNatives()`][3]. Both `Java_`-prefixed native functions and function pointers must refer to C-callable functions with appropriate [native method arguments][4]. A *Marshal Method* is a: 1. Method or delegate which is C-callable, 2. Accepting the appropriate Java Native Method arguments, 3. Is responsible for marshaling parameter and return types, and 3. *Delegates* the call to an appropriate managed method override. We have multiple different Marshal Method implementations running around, including: * XamarinAndroid1 and XAJavaInterop1 Marshal Methods, in which the Marshal Method is an `n_`-prefixed method in (roughly-ish) the same scope as the method that would be delegated to. * `jnimarshalmethod-gen`: see 176240d * LLVM Marshal Methods, which use LLVM-IR to emit `Java_`-prefixed native functions; see dotnet/android@8bc7a3e8. Which brings us to the current XAJavaInterop1 Marshal Methods implementation. Consider the [`java.util.function.IntConsumer`][5] interface: // Java public /* partial */ interface IntConsumer { void accept(int value); } With `generator --codegen-target=XAJavaInterop1` -- used by .NET for Android -- `IntConsumer` is bound as `IIntConsumer`: namespace Java.Util.Functions { // Metadata.xml XPath interface reference: path="/api/package[@name='java.util.function']/interface[@name='IntConsumer']" [Register ("java/util/function/IntConsumer", "", "Java.Util.Functions.IIntConsumerInvoker", ApiSince = 24)] public partial interface IIntConsumer : IJavaObject, IJavaPeerable { [Register ("accept", "(I)V", "GetAccept_IHandler:Java.Util.Functions.IIntConsumerInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", ApiSince = 24)] void Accept (int value); } [Register ("java/util/function/IntConsumer", DoNotGenerateAcw=true, ApiSince = 24)] internal partial class IIntConsumerInvoker : global::Java.Lang.Object, IIntConsumer { static Delegate? cb_accept_Accept_I_V; static Delegate GetAccept_IHandler () { if (cb_accept_Accept_I_V == null) cb_accept_Accept_I_V = JNINativeWrapper.CreateDelegate (new _JniMarshal_PPI_V (n_Accept_I)); return cb_accept_Accept_I_V; } static void n_Accept_I (IntPtr jnienv, IntPtr native__this, int value) { var __this = global::Java.Lang.Object.GetObject<Java.Util.Functions.IIntConsumer> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!; __this.Accept (value); } } } The Marshal Method is `IIntConsumerInvoker.n_Accept_I()`. We also have a *Connector Method*. A Connector Method is a `static` method matching the signature of `Func<Delegate>`. The name of the connector method is mentioned in the 3rd `connector` parameter of `RegisterAttribute` on the interface method. During [Java Type Registration][6], all Connector methods for a type are looked up and invoked, and the `Delegate` instances returned from all those connector method invocations are provided to `JNIEnv::RegisterNatives()`. There are static and runtime issues with connector method and marshal method implementations until now: 1. Java Native Methods, and thus Marshal Methods, *must* conform to the C ABI. C does not support exceptions. C# *does*. What happens when `__this.Accept(value)` throws? 2. The answer to (1) is in the connector method, via the `JNINativeWrapper.CreateDelegate()` invocation. [`JNINativeWrapper.CreateDelegate()`][7] uses System.Reflection.Emit to *wrap* the Marshal Method with a try/catch block. At runtime, the intermixing of (1) and (2) will result in registering a method similar to the following with `JNIEnv::RegisterNatives()`: static void n_Accept_I (IntPtr jnienv, IntPtr native__this, int value) { JNIEnv.WaitForBridgeProcessing (); try { var __this = ava.Lang.Object.GetObject<IIntConsumer> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!; __this.Accept (value); } catch (Exception e) when (!Debugger.IsAttached) { AndroidEnvironment.UnhandledException (e); } } which presents a further two problems: 1. System.Reflection.Emit is used, which possibly slows down type registration and won't work with NativeAOT. 2. The `catch` block only executes when you're *not* debugging! Which means that if you're debugging the app, and an exception is thrown, you are now potentially unwinding the stack frame through a JNI boundary, which can *corrupt JVM state*, possibly resulting in an [app abort or crash][8]. ([***Why?!***][9]) This has been how things work since the beginning. .NET 9 introduces some features that allow us to rethink all this: * [`DebuggerDisableUserUnhandledExceptionsAttribute`][10] * [`Debugger.BreakForUserUnhandledException(Exception)`][11] > If a .NET Debugger is attached that supports the > [BreakForUserUnhandledException(Exception)][11] API, the debugger > won't break on user-unhandled exceptions when the exception is > caught by a method with this attribute, unless > [BreakForUserUnhandledException(Exception)][11] is called. Embrace .NET 9, remove the possible need for System.Reflection.Emit, and fully prevent possible JVM corruption by updating connector methods and marshal methods to instead be: namespace Java.Util.Functions { internal partial class IIntConsumerInvoker { static Delegate? cb_accept_Accept_I_V; static Delegate GetAccept_IHandler () { return cb_accept_Accept_I_V ??= new _JniMarshal_PPI_V (n_Accept_I); } [DebuggerDisableUserUnhandledExceptions] static void n_Accept_I (IntPtr jnienv, IntPtr native__this, int value) { if (!JniEnvironment.BeginMarshalMethod (jnienv, out var __envp, out var __r)) return; try { var __this = Java.Lang.Object.GetObject<IIntConsumer> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!; __this.Accept (value); } catch (global::System.Exception __e) { __r?.OnUserUnhandledException (ref __envp, __e); } finally { JniEnvironment.EndMarshalMethod (ref __envp); } } } } This removes the call to `JNINativeWrapper.CreateDelegate()` and it's implicit use of System.Reflection.Emit, properly wraps *everything* in a `try`/`catch` block so that exceptions are properly caught and marshaled back to Java if necessary, and integrates properly with expected "first chance exception" semantics. The *downside* is that this requires the "new debugger backend" to work, which at the time of this writing is only used by VSCode. As this code will only be used for .NET 10+ (2025-Nov), this is fine. [0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/jniTOC.html [1]: http://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#compiling_loading_and_linking_native_methods [2]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names [3]: http://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#RegisterNatives [4]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#native_method_arguments [5]: https://developer.android.com/reference/java/util/function/IntConsumer [6]: https://github.com/dotnet/android/wiki/Blueprint#java-type-registration [7]: https://github.com/dotnet/android/blob/65906e0b7b2f471fcfbd07e7e01b68169c25d9da/src/Mono.Android/Android.Runtime/JNINativeWrapper.cs#L29-L105 [8]: dotnet/android#8608 (comment) [9]: dotnet/android#4877 [10]: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.debuggerdisableuserunhandledexceptionsattribute?view=net-9.0 [11]: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.debugger.breakforuserunhandledexception?view=net-9.0#system-diagnostics-debugger-breakforuserunhandledexception(system-exception)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.