Skip to content

Commit

Permalink
[Mono.Android] Prevent NRE in VS Mac when breaking on exceptions (#7103)
Browse files Browse the repository at this point in the history
Fixes: #7085
Fixes: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1554705

In a new Xamarin.Android project, if you go to your `OnCreate()`
method and add a `throw new Exception("test")`, Visual Studio for Mac
breaks on a `NullReferenceException` such as:

	[mono-rt] [ERROR] FATAL UNHANDLED EXCEPTION: System.NullReferenceException: Object reference not set to an instance of an object.
	at Android.Runtime.JNINativeWrapper._unhandled_exception (System.Exception e) [0x0000e] in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs:12
	at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V (_JniMarshal_PPL_V callback, System.IntPtr jnienv, System.IntPtr klazz, System.IntPtr p0) [0x0001d] in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs:111
	at (wrapper native-to-managed) Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(intptr,intptr,intptr)

This does not happen in VS Windows, only VS Mac.

After reviewing my changes in 32cff43, the System.Reflection.Emit
code path does a null check for `mono_unhandled_exception_method`:

	bool  filter = Debugger.IsAttached || !JNIEnv.PropagateExceptions;
	if (filter && JNIEnv.mono_unhandled_exception_method != null) {
	    ig.BeginExceptFilterBlock ();

	    ig.Emit (OpCodes.Call, JNIEnv.mono_unhandled_exception_method);
	    ig.Emit (OpCodes.Ldc_I4_1);
	    ig.BeginCatchBlock (null!);
	} else {
	    ig.BeginCatchBlock (typeof (Exception));
	}

While the new "fast" code path, does not:

	static bool _unhandled_exception (Exception e)
	{
	    if (Debugger.IsAttached || !JNIEnv.PropagateExceptions) {
	        JNIEnv.mono_unhandled_exception (e);
	        return false;
	    }
	    return true;
	}

Adding `JNIEnv.mono_unhandled_exception?.Invoke(e)` appears to solve
the problem when I build a local copy of `Mono.Android.dll` and test
it inside VS Mac.  I see my exception break properly on the line I
threw the exception.

`mono_unhandled_exception` being null appears to be something
introduced in 2aff4e7.  That commit's goal was to not lookup
`mono_unhandled_exception_method` at startup, but wait until an
exception is thrown.  Unfortunately, `mono_unhandled_exception_method`
is null at the time that the code S.R.Emitted, so we've had this
behavior for a while!

Since we are looking at reworking this entire system with "marshal
methods" as in #7004, I think we should simply add the null check for
now.  We should probably investigate the sequence of events during
startup & unhandled exceptions when the new system is in place.
  • Loading branch information
jonathanpeppers authored Jun 16, 2022
1 parent 0181212 commit 3d5231c
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public static partial class JNINativeWrapper
static bool _unhandled_exception (Exception e)
{
if (Debugger.IsAttached || !JNIEnv.PropagateExceptions) {
JNIEnv.mono_unhandled_exception (e);
JNIEnv.mono_unhandled_exception?.Invoke (e);
return false;
}
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/Mono.Android/Android.Runtime/JNINativeWrapper.g.tt
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ namespace Android.Runtime
static bool _unhandled_exception (Exception e)
{
if (Debugger.IsAttached || !JNIEnv.PropagateExceptions) {
JNIEnv.mono_unhandled_exception (e);
JNIEnv.mono_unhandled_exception?.Invoke (e);
return false;
}
return true;
Expand Down

0 comments on commit 3d5231c

Please sign in to comment.