Skip to content

Commit

Permalink
[tests] enable SubscribeToAppDomainUnhandledException in .NET 6 (#6119)
Browse files Browse the repository at this point in the history
Context: c1a2ee7
Context: dotnet/runtime#44526
Context: dotnet/runtime#44526 (comment)
Context: dotnet/runtime#44526 (comment)
Context: start: https://discord.com/channels/732297728826277939/732297837953679412/869330822262587392
Context:   end? https://discord.com/channels/732297728826277939/732297837953679412/869343082552893440
Context: dotnet/runtime#57304

Now that we are calling `mono_unhandled_exception()` when an unhandled
exception is reported (c1a2ee7), we should be able re-enable the
`InstallAndRunTests.SubscribeToAppDomainUnhandledException()` unit
test on .NET 6, which was disabled in 6e3e383.

What seemed like it should have been a 1-line removal ballooned a bit
due to a confluence of factors:

 1. Debugger component stubs, and
 2. .NET 6 `Console.WriteLine()` behavior.

On .NET 6, `JNIEnv.mono_unhandled_exception()` is
`monodroid_debugger_unhandled_exception()`, which calls
`mono_debugger_agent_unhandled_exception()`; see also e4debf7.

The problem is that in our current world order of "Mono components"
(0f7a0cd), if the debugger isn't used, then we get "debugger stubs"
for the mono debugger agent, which turns
`mono_debugger_agent_unhandled_exception()` into an [*assertion*][0]:

	static void
	stub_debugger_unhandled_exception (MonoException *exc)
	{
		g_assert_not_reached ();
	}

The result is that when an exception is thrown, *before* the
`AppDomain.UnhandledException` event can be raised, the runtime dies
in a horrible flaming assertion death:

	E andledexceptio: * Assertion: should not be reached at /__w/1/s/src/mono/mono/component/debugger-stub.c:175

Avoid this issue by checking `Debugger.IsAttached` *before* calling
`monodroid_debugger_unhandled_exception()`.

Additionally, remove some obsolete comments: .NET 6 couldn't resolve
`Debugger.Mono_UnhandledException()` because .NET 6 never had it, so
the linker was right to warn about its absence.

The problem with .NET 6 & `Console.WriteLine()` is that when format
strings are used, the output may be line-wrapped in unexpected ways;
see also dotnet/runtime@#57304.  Additionally, the `sender` parameter
value differs under .NET 6.  These two issues broke our unit test;
we *expected* `adb logcat` to contain:

	# Unhandled Exception: sender=RootDomain; e.IsTerminating=True; e.ExceptionObject=System.Exception: CRASH

Under .NET 6, `adb logcat` *instead* contained:

	# Unhandled Exception: sender=System.Object; e.IsTerminating=True; e.ExceptionObject=System.Exception
	: CRASH

Yes, `: CRASH` was on a separate `adb logcat` line.

Fix the `SubscribeToAppDomainUnhandledException()` unit test so that
`adb logcat` messages can now span multiple lines (which is sure to
cause all sorts of GC garbage!), and update the expected message so
that we look for the right message under legacy & .NET 6.

Co-authored-by: Jonathan Pryor <jonpryor@vt.edu>

[0]: https://github.com/dotnet/runtime/blob/16b456426dfb5212a24bfb78bfd5d9adfcc95185/src/mono/mono/component/debugger-stub.c#L172-L176
  • Loading branch information
jonathanpeppers committed Aug 12, 2021
1 parent 4fffaf8 commit ab0ed93
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 7 deletions.
6 changes: 3 additions & 3 deletions src/Mono.Android/Android.Runtime/JNIEnv.cs
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,9 @@ internal static void PropagateUncaughtException (IntPtr env, IntPtr javaThreadPt

var javaException = JavaObject.GetObject<Java.Lang.Throwable> (env, javaExceptionPtr, JniHandleOwnership.DoNotTransfer)!;

// Disabled until Linker error surfaced in https://github.com/xamarin/xamarin-android/pull/4302#issuecomment-596400025 is resolved
//System.Diagnostics.Debugger.Mono_UnhandledException (javaException);
mono_unhandled_exception?.Invoke (javaException);
if (Debugger.IsAttached) {
mono_unhandled_exception?.Invoke (javaException);
}

try {
var jltp = javaException as JavaProxyThrowable;
Expand Down
42 changes: 38 additions & 4 deletions tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ void Button_ViewTreeObserver_GlobalLayout (object sender, EventArgs e)
}

[Test]
[Category ("DotNetIgnore")] // TODO: UnhandledException not firing: https://github.com/dotnet/runtime/issues/44526
public void SubscribeToAppDomainUnhandledException ()
{
AssertHasDevices ();
Expand All @@ -84,10 +83,45 @@ public void SubscribeToAppDomainUnhandledException ()
else
AdbStartActivity ($"{proj.PackageName}/{proj.JavaPackageName}.MainActivity");

#if NETCOREAPP
string expectedLogcatOutput = "# Unhandled Exception: sender=System.Object; e.IsTerminating=True; e.ExceptionObject=System.Exception: CRASH";
#else // NETCOREAPP
string expectedLogcatOutput = "# Unhandled Exception: sender=RootDomain; e.IsTerminating=True; e.ExceptionObject=System.Exception: CRASH";
Assert.IsTrue (MonitorAdbLogcat ((line) => {
return line.Contains (expectedLogcatOutput);
}, Path.Combine (Root, builder.ProjectDirectory, "startup-logcat.log"), 60), $"Output did not contain {expectedLogcatOutput}!");
#endif // NETCOREAPP
Assert.IsTrue (
MonitorAdbLogcat (CreateLineChecker (expectedLogcatOutput),
logcatFilePath: Path.Combine (Root, builder.ProjectDirectory, "startup-logcat.log"), timeout: 60),
$"Output did not contain {expectedLogcatOutput}!");
}


public static Func<string, bool> CreateLineChecker (string expectedLogcatOutput)
{
// On .NET 6, `adb logcat` output may be line-wrapped in unexpected ways.
// https://github.com/xamarin/xamarin-android/pull/6119#issuecomment-896246633
// Try to see if *successive* lines match expected output
var remaining = expectedLogcatOutput;
return line => {
if (line.IndexOf (remaining) >= 0) {
Reset ();
return true;
}
int count = Math.Min (line.Length, remaining.Length);
for ( ; count > 0; count--) {
var startMatch = remaining.Substring (0, count);
if (line.IndexOf (startMatch) >= 0) {
remaining = remaining.Substring (count);
return false;
}
}
Reset ();
return false;
};

void Reset ()
{
remaining = expectedLogcatOutput;
}
}

Regex ObfuscatedStackRegex = new Regex ("in <.*>:0", RegexOptions.Compiled);
Expand Down

0 comments on commit ab0ed93

Please sign in to comment.