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

[debugger] Removing usage of mono_unhandled_exception #4927

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Jul 16, 2020

We don't need to call mono_unhandled_exception the runtime will handle with it.

Relates to mono/mono#20314.
We don't need to wait for mono/mono PR to be merged, mono PR only removes the unhandled_exception function.

@jonpryor
Copy link
Member

@thaystg, @jonathanpeppers: "semantics" question: if you have a managed1 > managed2 call stack in which managed2 throws an exception which is caught by managed1:

public void managed1 ()
{
    try {
        managed2 ();
    }
    catch (Exception e) {
        Console.WriteLine (e);
    }
}

public void managed1 ()
{
    throw new Exception ("Wee!");
}

what happens when ^^ is run within the debugger? Will we get a "first chance exception" on the throw new Exception("Wee!") line? Is a "last-chance notification" ever raised? (Presumably not?)

How does this align with what .NET & Visual Studio do (no mono in the picture)?

@jonathanpeppers
Copy link
Member

Visual Studio on Windows can break on this line:

throw new Exception ("Wee!");

But only if you have the Debug > Windows > Exception Settings boxes checked appropriately:

image

VS for Mac probably has something similar?

@jonpryor
Copy link
Member

@jonathanpeppers: which indicates the Visual Studio with .NET has a first-chance exception, which is entirely expected. :-)

I'm not at all sure how to reasonably test the "last chance notification": https://docs.microsoft.com/en-us/windows/win32/debug/debugger-exception-handling

The second attempt to notify the debugger occurs only if the system is unable to find a frame-based exception handler that handles the exception. This is known as last-chance notification.

i.e. if managed1() didn't have a try/catch block, we'd theoretically be hitting the last-chance notification scenario, but I don't know how to tell (if it's possible to tell?) the difference between the first-chance and last-chance exception.

Regardless, the question here isn't entirely about "VS with .NET", it's about "VS with Mono with @thaystg's changes". What happens there?

@thaystg
Copy link
Member Author

thaystg commented Jul 17, 2020

I've tested with my PR using VSMac and mono, and VSMac can break on this line:
throw new Exception ("Wee!");
Only if you have the Debug > Breakpoints > System.Exception checked
image

thaystg added a commit to thaystg/mono that referenced this pull request Aug 27, 2020
…bugger handle with an exception when running on android.

The biggest difference is on xamarin-android side.

Relates to dotnet/android#4927
monojenkins pushed a commit to monojenkins/runtime that referenced this pull request Aug 27, 2020
Trying to remove the usage of unhandled_exception function to make debugger handle with an exception when running on android.

The biggest difference is on xamarin-android side.
Where I removed the try catch added when generating dynamic methods while debugging and let the debugger handle the caught or uncaught exceptions.

Relates to dotnet/android#4927
@thaystg thaystg marked this pull request as ready for review August 27, 2020 19:06
@@ -51,26 +51,34 @@ public static Delegate CreateDelegate (Delegate dlg)

ig.Emit (OpCodes.Call, wait_for_bridge_processing_method!);

var label = ig.BeginExceptionBlock ();
bool filter = Debugger.IsAttached || !JNIEnv.PropagateExceptions;
if (!filter) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaystg: This presents a question: is there any way to "mimic" the "normal" .NET Unhandled Exception behaviors while still catching the exception?

Related: #4548

The problem is that when the catch block doesn't exist, in a managed > Java > managed callstack, if the rightmost frame throws (and no catch block is present around it) and execution doesn't return to Java, and we unwind the rightmost Java > managed frames, then JVM state may be corrupted.

In an ideal world, every "marshal method" would have a try{…} catch (Exception) {…} handler, so that when a managed method throws an exception it can be properly caught & propagated back to Java. The problem with this ideal is it means every exception is by definition "handled", thus "mooting" the entire concept of "first chance unhandled exceptions". (There'd still eventually be an unhandled exception, but that would be on the leftmost managed frame, after the rightmost frame had been unwound.)

Is this possible?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you telling me that removing the try catch when the debugger is attached is a problem because when we have a case like:
managed -> Java -> managed (which throws the unhandled exception), debugger will stop in the exception, but when we continue the execution we may get an error because JVM state may be corrupted? Which kind of error? shouldn't the process be finished in this case?
If I test #4548 I will reproduce the case that you are saying?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you telling me that removing the try catch when the debugger is attached is a problem because when we have a case like:
managed -> Java -> managed (which throws the unhandled exception), debugger will stop in the exception, but when we continue the execution we may get an error because JVM state may be corrupted?

Yup. Because the debugger is running, there are no try/catch blocks at Java-to-managed boundaries, and because there are try/catch blocks, if there is an exception thrown from managed code, the JVM state is corrupted, and Android aborts the process.

Desktop repro:

$ git clone https://github.com/xamarin/java.interop.git
$ cd java.interop
$ make prepare
$ cat > d <<EOF
diff --git a/tests/Java.Interop-Tests/Java.Interop/TestType.cs b/tests/Java.Interop-Tests/Java.Interop/TestType.cs
index 4640d5f1..2cd108b1 100644
--- a/tests/Java.Interop-Tests/Java.Interop/TestType.cs
+++ b/tests/Java.Interop-Tests/Java.Interop/TestType.cs
@@ -172,7 +172,8 @@ namespace Java.InteropTests
 		static Delegate GetMethodThrowsHandler ()
 		{
 			Action<IntPtr, IntPtr> h = MethodThrowsHandler;
-			return JniEnvironment.Runtime.MarshalMemberBuilder.CreateMarshalToManagedDelegate (h);
+//			return JniEnvironment.Runtime.MarshalMemberBuilder.CreateMarshalToManagedDelegate (h);
+			return h;
 		}
 
 		static void MethodThrowsHandler (IntPtr jnienv, IntPtr n_self)

EOF
$ git apply < d
$ make
$ make run-tests TESTS=bin/TestDebug/Java.Interop-Tests.dll

Desired behavior: nothing bad happens. ;-)

Actual results: horrible flaming death:

$ make run-tests TESTS=bin/TestDebug/Java.Interop-Tests.dll
r=0; \
	 	msbuild /p:Configuration=Debug  build-tools/scripts/RunNUnitTests.targets /p:TestAssembly=bin/TestDebug/Java.Interop-Tests.dll || r=1; \
	exit $r;
…
  =================================================================
  	Native Crash Reporting
  =================================================================
  Got a segv while executing native code. This usually indicates
  a fatal error in the mono runtime or one of the native libraries 
  used by your application.
  =================================================================
  
  =================================================================
  	Native stacktrace:
  =================================================================
  	0x101da3779 - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : mono_dump_native_crash_info
  	0x101d3b5be - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : mono_handle_native_crash
  	0x101d9d8f8 - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : altstack_handle_and_restore
  	0x10c849e4e - /Users/jon/android-toolchain/jdk-11/lib/server/libjvm.dylib : _ZN5Chunk9next_chopEv
  	0x10cb5ccec - /Users/jon/android-toolchain/jdk-11/lib/server/libjvm.dylib : checked_jni_GetBooleanField
  	0x10c299140 - /Users/jon/Dropbox/Developer/Java.Interop/bin/TestDebug/libjava-interop.dylib : java_interop_jnienv_get_boolean_field
  	0x1650e3841 - Unknown
  
  =================================================================
  	Telemetry Dumper:
  =================================================================
EXEC : warning : SIGSEGV handler expected:libjvm.dylib+0x5f61c3  found:0x0000000000000000 [/Users/jon/Dropbox/Developer/Java.Interop/build-tools/scripts/RunNUnitTests.targets]
  Signal Handlers:
  SIGSEGV: SIG_DFL, sa_mask[0]=00000000000000000000000000000000, sa_flags=none
…

Again, this crash is "expected", because by not catching all exceptions (which is what JniEnvironment.Runtime.MarshalMemberBuilder.CreateMarshalToManagedDelegate() does), we're propagating a Mono exception "through" a Java-to-managed boundary, and Java Does Not Like That. (Deservedly so!)

The Good And Proper solution is to catch all exceptions at all Java-to-managed boundaries, but in doing so we "kill" the normal/expected IDE unhandled exception behavior… because now all exceptions are handled.

Is there a way to have our cake and eat it, too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaystg what is the best path forward here?

Copy link
Member Author

@thaystg thaystg Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how can we do this, but I would like to try to generate a try catch with some "mark" so we would know that the debugger should stop because it is not a really handled exception, it's only an exception that we catch in our forced catch.
I will come back soon to this issue to make more tests.

@jonpryor
Copy link
Member

jonpryor commented Jul 7, 2021

Example Project:
Scratch.JMJMException.zip

Repro Steps:

  1. Download & Build Scratch.JMJMException.zip

  2. In a command-line, run adb logcat > log.txt; peruse contents for later discussion

  3. Within the IDE, Run the app with the debugger attached. Click the "Click Me!" button which fills the screen.

    What should happen is that the IDE will "break" at the throw new Exception("…") line in MainActivity.cs, with the following message written to log.txt:

    V mono-stdout: # jon: Should be in a Java > Managed [MainActivity.OnCreate] > Java [Run.tryCatchFinally] > Managed [Run] frame. Throwing an exception...
    

    When you Continue execution, the app will exit; it will no longer be running. Checking log.txt from (2) will result in viewing a lines similar to:

    F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x40 in tid 26527 (h.jmjmexception), pid 26527 (h.jmjmexception)
    I crash_dump64: obtaining output fd from tombstoned, type: kDebuggerdTombstone
    I crash_dump64: performing dump of process 26527 (target tid = 26527)
    F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
    F DEBUG   : Build fingerprint: 'google/redfin/redfin:11/RQ1A.210105.003/7005429:user/release-keys'
    F DEBUG   : Revision: 'MP1.0'
    F DEBUG   : ABI: 'arm64'
    F DEBUG   : Timestamp: 2021-07-07 14:56:02-0400
    F DEBUG   : pid: 26527, tid: 26527, name: h.jmjmexception  >>> scratch.jmjmexception <<<
    F DEBUG   : uid: 10291
    F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x40
    F DEBUG   : Cause: null pointer dereference
    F DEBUG   :     x0  0000007fc13ec9d0  x1  0000000000000001  x2  0000000000000001  x3  000000747edb4b4c
    F DEBUG   :     x4  000000747edb4b4c  x5  b4000074af42d43e  x6  0000000000000020  x7  0000000000000020
    F DEBUG   :     x8  0000000000000008  x9  0000000000000000  x10 b4000074af42d427  x11 0000000000000010
    F DEBUG   :     x12 3d2120746e696f70  x13 7274706c6c756e20  x14 0000000000000000  x15 0000000000000005
    F DEBUG   :     x16 000000747f3b3678  x17 000000771396524c  x18 0000000000000008  x19 0000007fc13ec8f0
    F DEBUG   :     x20 0000007fc13ec9d0  x21 0000000000000001  x22 00000077149d5000  x23 0000000000000000
    F DEBUG   :     x24 b40000758f3db010  x25 b4000075ef3d40c8  x26 0000007414437210  x27 0000007fc13ec9d0
    F DEBUG   :     x28 0000007fc13ece38  x29 0000007fc13ec8b0
    F DEBUG   :     lr  000000747eec66f8  sp  0000007fc13ec890  pc  000000747eec5c30  pst 0000000000001000
    F DEBUG   : backtrace:
    F DEBUG   :       #00 pc 00000000001a8c30  /apex/com.android.art/lib64/libart.so (art::ArtMethod::PrettyMethod(bool)+76) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #01 pc 00000000001a96f4  /apex/com.android.art/lib64/libart.so (art::ArtMethod::GetOatQuickMethodHeader(unsigned long)+592) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #02 pc 0000000000576718  /apex/com.android.art/lib64/libart.so (void art::StackVisitor::WalkStack<(art::StackVisitor::CountTransitions)0>(bool)+516) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #03 pc 00000000005a35d0  /apex/com.android.art/lib64/libart.so (art::Thread::GetCurrentMethod(unsigned int*, bool, bool) const+156) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #04 pc 0000000000382004  /apex/com.android.art/lib64/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+620) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #05 pc 0000000000382804  /apex/com.android.art/lib64/libart.so (art::JavaVMExt::JniAbortV(char const*, char const*, std::__va_list)+108) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #06 pc 000000000037463c  /apex/com.android.art/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::AbortF(char const*, ...)+148) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #07 pc 0000000000373120  /apex/com.android.art/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::CheckPossibleHeapValue(art::ScopedObjectAccess&, char, art::(anonymous namespace)::JniValueType)+3116) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #08 pc 0000000000371d84  /apex/com.android.art/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::Check(art::ScopedObjectAccess&, bool, char const*, art::(anonymous namespace)::JniValueType*)+612) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #09 pc 00000000003674e4  /apex/com.android.art/lib64/libart.so (art::(anonymous namespace)::CheckJNI::NewString(_JNIEnv*, unsigned short const*, int)+680) (BuildId: aeb876e57f112c8539df5053f2da77d4)
    F DEBUG   :       #10 pc 0000000000008b50  /data/app/~~xWY_FPMiBGkhLQyx_hhKpQ==/scratch.jmjmexception-K9NCUEsEY32ORi4xuwFBQw==/lib/arm64/libxa-internal-api.so (java_interop_jnienv_new_string+44) (BuildId: 0657f6418c67601c3f4fe59c63d9b239293d6c68)
    F DEBUG   :       #11 pc 000000000000bda8  <anonymous:7463644000>
    
  4. Run the app without the debugger being attached.

    What happens is that the app doesn't crash, which differs from (3). Furthermore, more # jon messages are also present in log.txt, which weren't present in (3):

    V mono-stdout: # jon: MyCatchHandler.OnCatch: t=Android.Runtime.JavaProxyThrowable: Exception of type 'Android.Runtime.JavaProxyThrowable' was thrown.
    …
    V mono-stdout: # jon: from Java finally block
    

@jonathanpeppers
Copy link
Member

jonathanpeppers commented Jul 7, 2021

Here is the same project ported to .NET 6: Scratch.JMJMException.zip

Crashes (and doesn't break) with: log.txt

This was using .NET 6.0.100-preview.6.21355.2 and Android 30.0.100-preview.6.62.

@marek-safar
Copy link
Contributor

@thaystg what are the next steps here?

@thaystg
Copy link
Member Author

thaystg commented Jul 14, 2021

As discussed with @lewing I will work on it again as soon as possible. Only finishing 2 implementations: one about debugging after hotreload on android and another about show custom displays on wasm debugger, then I will work on it again.

grendello added a commit to grendello/xamarin-android that referenced this pull request Jul 22, 2021
Context: dotnet/runtime#55904 (comment)
Context: dotnet#4927 (comment)
Context: dotnet#4927 (comment)

Xamarin.Android has been using the `AppDomain.DoUnhandledException` API
since the dawn of time to propagate uncaught Java exceptions to the
managed world.  However, said API (and AppDomains) are gone from the
NET6 MonoVM runtime and we need to switch to something else - the
`mono_unhandled_exception` native API.

This commit introduces an internal call,
`JNIEnv.monodroid_unhandled_exception`, which is used instead of the
older mechanism when targetting NET6 and which calls the native API
mentioned above.

Add a device integration test which makes sure the uncaught exceptions
are propagated as required.
jonpryor pushed a commit that referenced this pull request Jul 22, 2021
Context: dotnet/runtime#55904 (comment)
Context: #4927 (comment)
Context: #4927 (comment)
Context: xamarin/monodroid@7d62dec
Context: dotnet/runtime@15ab9f9

Xamarin.Android has been using the `AppDomain.DoUnhandledException()`
internal API since 2015 (xamarin/monodroid@7d62dec1) to propagate
uncaught Java exceptions to the managed world.

However, `AppDomain.DoUnhandledException()` was an internal API which
was *removed* from MonoVM as part of the .NET 6 effort in
dotnet/runtime@15ab9f98.

For .NET 6, instead of calling the no-longer-present
`AppDomain.DoUnhandledException()` method, call the
[`mono_unhandled_exception()` embedding API][0], which in turn raises
the `AppDomain.UnhandledException` event.

Add a new internal call, `JNIEnv.monodroid_unhandled_exception()`,
which is used on .NET 6 to invoke `mono_unhandled_exception()`.

Add an on-device integration test which ensures that the uncaught
exceptions are propagated as required.

[0]: http://docs.go-mono.com/index.aspx?link=xhtml%3adeploy%2fmono-api-exc.html
jonpryor pushed a commit that referenced this pull request Jul 22, 2021
Context: dotnet/runtime#55904 (comment)
Context: #4927 (comment)
Context: #4927 (comment)
Context: xamarin/monodroid@7d62dec
Context: dotnet/runtime@15ab9f9

Xamarin.Android has been using the `AppDomain.DoUnhandledException()`
internal API since 2015 (xamarin/monodroid@7d62dec1) to propagate
uncaught Java exceptions to the managed world.

However, `AppDomain.DoUnhandledException()` was an internal API which
was *removed* from MonoVM as part of the .NET 6 effort in
dotnet/runtime@15ab9f98.

For .NET 6, instead of calling the no-longer-present
`AppDomain.DoUnhandledException()` method, call the
[`mono_unhandled_exception()` embedding API][0], which in turn raises
the `AppDomain.UnhandledException` event.

Add a new internal call, `JNIEnv.monodroid_unhandled_exception()`,
which is used on .NET 6 to invoke `mono_unhandled_exception()`.

Add an on-device integration test which ensures that the uncaught
exceptions are propagated as required.

[0]: http://docs.go-mono.com/index.aspx?link=xhtml%3adeploy%2fmono-api-exc.html
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
Copy link
Member

@lambdageek suggests that Android use:

/*
* mono_install_ftnptr_eh_callback:
*
*   Install a callback that should be called when there is a managed exception
*   in a native-to-managed wrapper. This is mainly used by iOS to convert a
*   managed exception to a native exception, to properly unwind the native
*   stack; this native exception will then be converted back to a managed
*   exception in their managed-to-native wrapper.
*/
void
mono_install_ftnptr_eh_callback (MonoFtnPtrEHCallback callback)

@jonpryor
Copy link
Member

@lambdageek: while looking at what mono_install_ftnptr_eh_callback() does, I see:

https://github.com/dotnet/runtime/blob/21dc0040b5d6cd52501d9c502fb3ddbc23db97d7/src/mono/mono/mini/mini-exceptions.c#L2364-L2372

In particular, this worries me:

g_error ("Did not expect ftnptr_eh_callback to return.");

The (not quite documented?) requirement, then, is that the callback provided to mono_install_ftnptr_eh_callback() must not return.

This does not work for JNI stack unwind. JNI is limited to the C ABI, which doesn't have exceptions as a language or ABI feature, and thus stack unwind is cooperative, by necessity.

See also:

Thus, the way to get Java to unwind its stack is to return execution to Java.

I suppose setjmp()/longjmp() could be used to "return execution to Java", except that I don't want to be using setjmp()/longjmp(), particularly not from managed code!

Thus, I don't consider mono_install_ftnptr_eh_callback() to be a viable alternative on Android. :-(

@jonpryor
Copy link
Member

@thaystg: Re-upping and re-framing this: the problems with the current approach (described in part in this PR and in #4927) is:

  1. If a managed exception is thrown from managed code and the debugger is attached, it is possible that JVM state will be corrupted if stack unwind crosses a JNI boundary. This means there is a potential behavioral difference between app runs when a debugger is attached vs. when no debugger is attached.
  2. The way it works -- bad as it is -- involves the use of exception filters, either via Mono extensions to System.Reflection.Emit and in some "optimized" code paths:
    https://github.com/xamarin/xamarin-android/blob/8bc7a3e84f95e70fe12790ac31ecd97957771cb2/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs#L21-L26
    a. IIRC, AOT (or AOT+LLVM?) doesn't like the use of exception filters, and thus their use should be avoided?

Additionally, we would like to update generator output so that more exception handling behavior is "baked into" the code, removing the need for System.Reflection.Emit, but as part of that we'd like to know what "ABI" we should be using. We could emit exception filters, but then vargaz won't be happy with us 😉, especially given that Profiled AOT is enabled by default in .NET Android 6 (7?).

@jonpryor
Copy link
Member

@thaystg previously mentioned:

I would like to try to generate a try catch with some "mark" so we would know that the debugger should stop because it is not a really handled exception, it's only an exception that we catch in our forced catch.

I just read that -- not sure why I didn't see it before? Or don't remember having seen it before? ¯\(ツ)/¯ -- and thought of an "interesting" way to "mark" an exception: with another exception!

namespace System.Runtime.ExceptionServices {
    public partial class StackUnwinding : Exception
    {
        public StackUnwinding(Exception innerException);
    }
}

where StackUnwinding allows code to explicitly participate in native stack unwind:

void JniBoundary(IntPtr jnienv, IntPtr self)
{
    var t = new JniTransition(jnienv);
    try {
        // code which may throw an exception
    }
    catch (StackUnwinding e) {
        t.SetPendingException(e.InnerException);
    }
    finally {
        t.Dispose();
    }
}

Given a Managed1 > JniCall1 > Java > JniBoundary1 > Managed2 > JniCall2 > Java > JniBoundary2 > Managed3 callstack, and the rightmost Managed3 throws e.g. InvalidOperationException. If a debugger is attached, then a "first chance exception" will be raised.

Unhandled Exception

Next, "first pass" is performed: the entire managed callstack is scanned to see if anything catches InvalidOperationException. If nothing catches InvalidOperationException and a debugger is attached, then an "unhandled exception" is raised in the debugger, still at the initial throw site.

Finally, the "second pass" is performed to unwind the callstack, but each managed frame is unwound, right-to-left, in a *two-check" process:

  1. Execute a StackUnwinding block, if present
  2. Otherwise, unwind the stack normally, executing relevant catch blocks, which shouldn't exist in this example, and any relevant finally blocks.

This would allow us to be involved only during stack unwind back to a Java caller, without the use of exception filters, in a manner which is consistent whether or not a debugger is attached.

If there are multiple catch blocks which involve StackUnwinding and any other ExceptionType, that should be a "style-cop" warning and only the StackUnwinding catch block will be evaluated.

Handled Exception

Next, "first pass" is performed: the entire managed callstack is scanned to see if anything catches InvalidOperationException. If Managed1 catches InvalidOperationException and a debugger is attached, the debugger does not alert about an unhandled exception; the only Debugger event is the previously described "first chance exception".

For each managed frame between Managed1 and Managed2, the "second pass" is performed to unwind the callstack, in the same "two-check" process. The catch(StackUnwinding) block in JnITransition2 is hit, which will marshal the exception to Java, then return control to Java. Java will see a pending exception, execute any relevant catch/finally blocks, and should eventually return control to JniCall2, which will see the pending exception from JNI and re-throw it. This will cause Managed2 to be unwound, returning control to JniTransition1, which will catch the exception and marshal to Java, which …

Problems
A problem with this sketch is that it doesn't support cross-VM catches: if a Java.Lang.RuntimeException is thrown, there is no way to ask the Java callstacks if that type is caught as part of the "first pass". Thus, if a debugger is attached, you could have a scenario where the debugger alerts you to an "unhandled exception" (no relevant catch blocks found!), and when execution is continued, execution continues when a Java method catches & handles the exception.

This also requires "special-casing" a particular exception type, a'la ThreadAbortException. I am not sure how much appetite there is for a new "special" exception.

Alternatively, it could be a struct! Exception C#2 and later requires that catch blocks only catch Exception instances, triggering a CS0155 if you try to catch anything else. This could be changed -- IL doesn't require it -- but now we're looking at a C# language change, which is presumably a much larger "ask".

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants