From fcf27931bd17cb2f1a38eb62bf633d44de83350f Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 22 Jul 2021 18:33:43 +0000 Subject: [PATCH] [Mono.Android] Use `mono_unhandled_exception` for NET6 (#6104) Context: https://github.com/dotnet/runtime/pull/55904#discussion_r672577807 Context: https://github.com/xamarin/xamarin-android/pull/4927#issuecomment-875864999 Context: https://github.com/xamarin/xamarin-android/pull/4927#issuecomment-875876255 Context: https://github.com/xamarin/monodroid/commit/7d62dec18ee1010fa9c96c206589827480e8d49c Context: https://github.com/dotnet/runtime/commit/15ab9f985ed45feaa619df70d288fbd0acd5c45f 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 --- src/Mono.Android/Android.Runtime/JNIEnv.cs | 15 +- src/monodroid/jni/monodroid-glue-internal.hh | 2 + src/monodroid/jni/monodroid-glue.cc | 12 ++ .../Tests/UncaughtExceptionTests.cs | 201 ++++++++++++++++++ 4 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 tests/MSBuildDeviceIntegration/Tests/UncaughtExceptionTests.cs diff --git a/src/Mono.Android/Android.Runtime/JNIEnv.cs b/src/Mono.Android/Android.Runtime/JNIEnv.cs index bbb192696be..064c73c6f72 100644 --- a/src/Mono.Android/Android.Runtime/JNIEnv.cs +++ b/src/Mono.Android/Android.Runtime/JNIEnv.cs @@ -242,7 +242,9 @@ static void ManualJavaObjectDispose (Java.Lang.Object obj) } static Action mono_unhandled_exception = null!; +#if !NETCOREAPP static Action AppDomain_DoUnhandledException = null!; +#endif // ndef NETCOREAPP static void Initialize () { @@ -253,6 +255,7 @@ static void Initialize () mono_unhandled_exception = (Action) Delegate.CreateDelegate (typeof(Action), mono_UnhandledException); } +#if !NETCOREAPP if (AppDomain_DoUnhandledException == null) { var ad_due = typeof (AppDomain) .GetMethod ("DoUnhandledException", @@ -265,8 +268,14 @@ static void Initialize () typeof (Action), ad_due); } } +#endif // ndef NETCOREAPP } +#if NETCOREAPP + [MethodImplAttribute(MethodImplOptions.InternalCall)] + extern static void monodroid_unhandled_exception (Exception javaException); +#endif // def NETCOREAPP + internal static void PropagateUncaughtException (IntPtr env, IntPtr javaThreadPtr, IntPtr javaExceptionPtr) { if (!PropagateExceptions) @@ -287,14 +296,18 @@ internal static void PropagateUncaughtException (IntPtr env, IntPtr javaThreadPt try { var jltp = javaException as JavaProxyThrowable; Exception? innerException = jltp?.InnerException; - var args = new UnhandledExceptionEventArgs (innerException ?? javaException, isTerminating: true); Logger.Log (LogLevel.Info, "MonoDroid", "UNHANDLED EXCEPTION:"); Logger.Log (LogLevel.Info, "MonoDroid", javaException.ToString ()); +#if !NETCOREAPP + var args = new UnhandledExceptionEventArgs (innerException ?? javaException, isTerminating: true); // Disabled until Linker error surfaced in https://github.com/xamarin/xamarin-android/pull/4302#issuecomment-596400025 is resolved //AppDomain.CurrentDomain.DoUnhandledException (args); AppDomain_DoUnhandledException?.Invoke (AppDomain.CurrentDomain, args); +#else // ndef NETCOREAPP + monodroid_unhandled_exception (innerException ?? javaException); +#endif // def NETCOREAPP } catch (Exception e) { Logger.Log (LogLevel.Error, "monodroid", "Exception thrown while raising AppDomain.UnhandledException event: " + e.ToString ()); } diff --git a/src/monodroid/jni/monodroid-glue-internal.hh b/src/monodroid/jni/monodroid-glue-internal.hh index 4e5ea9e74c5..f2b5a48c835 100644 --- a/src/monodroid/jni/monodroid-glue-internal.hh +++ b/src/monodroid/jni/monodroid-glue-internal.hh @@ -261,6 +261,8 @@ namespace xamarin::android::internal } #if defined (NET6) + static void monodroid_unhandled_exception (MonoObject *java_exception); + MonoClass* get_android_runtime_class (); #else // def NET6 MonoClass* get_android_runtime_class (MonoDomain *domain); diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc index e7eade011c4..923a0efd6b5 100644 --- a/src/monodroid/jni/monodroid-glue.cc +++ b/src/monodroid/jni/monodroid-glue.cc @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -1009,6 +1010,9 @@ MonodroidRuntime::init_android_runtime ( { mono_add_internal_call ("Java.Interop.TypeManager::monodroid_typemap_java_to_managed", reinterpret_cast(typemap_java_to_managed)); mono_add_internal_call ("Android.Runtime.JNIEnv::monodroid_typemap_managed_to_java", reinterpret_cast(typemap_managed_to_java)); +#if defined (NET6) + mono_add_internal_call ("Android.Runtime.JNIEnv::monodroid_unhandled_exception", reinterpret_cast(monodroid_unhandled_exception)); +#endif // def NET6 struct JnienvInitializeArgs init = {}; init.javaVm = osBridge.get_jvm (); @@ -1826,6 +1830,14 @@ MonodroidRuntime::create_and_initialize_domain (JNIEnv* env, jclass runtimeClass return domain; } +#if defined (NET6) +void +MonodroidRuntime::monodroid_unhandled_exception (MonoObject *java_exception) +{ + mono_unhandled_exception (java_exception); +} +#endif // def NET6 + MonoReflectionType* MonodroidRuntime::typemap_java_to_managed (MonoString *java_type_name) { diff --git a/tests/MSBuildDeviceIntegration/Tests/UncaughtExceptionTests.cs b/tests/MSBuildDeviceIntegration/Tests/UncaughtExceptionTests.cs new file mode 100644 index 00000000000..609ebbf6486 --- /dev/null +++ b/tests/MSBuildDeviceIntegration/Tests/UncaughtExceptionTests.cs @@ -0,0 +1,201 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text.RegularExpressions; +using NUnit.Framework; +using Xamarin.ProjectTools; + +namespace Xamarin.Android.Build.Tests +{ + [TestFixture] + [Category ("UsesDevice")] + public class UncaughtExceptionTests : DeviceTest + { + class LogcatLine + { + public string Text; + public bool Found = false; + public int SequenceNumber = -1; + public int Count = 0; + }; + + [Test] + public void EnsureUncaughtExceptionWorks () + { + AssertHasDevices (); + + var lib = new XamarinAndroidBindingProject { + ProjectName = "Scratch.Try", + AndroidClassParser = "class-parse", + }; + + lib.Imports.Add ( + new Import (() => "Directory.Build.targets") { + TextContent = () => +@" + + 1.8 + 1.8 + javac + jar + + + + + + + + + + + <_Classes>$(IntermediateOutputPath)classes + + + + '"%(Identity)"', ' ')"" /> + + + +" + }); + + lib.Sources.Add ( + new BuildItem.NoActionResource ("java\\testing\\Run.java") { + Encoding = new System.Text.UTF8Encoding (encoderShouldEmitUTF8Identifier: false), + TextContent = () => +@"package testing; + +public final class Run { + private Run() { + } + + public static interface CatchThrowableHandler { + void onCatch(Throwable t); + } + + public static final void tryCatchFinally (Runnable r, CatchThrowableHandler c, Runnable f) { + try { + r.run(); + } + catch (Throwable t) { + c.onCatch(t); + } + finally { + f.run(); + } + } +} +" + }); + + var app = new XamarinAndroidApplicationProject { + ProjectName = "Scratch.JMJMException", + }; + + app.SetDefaultTargetDevice (); + app.AddReference (lib); + + app.Sources.Remove (app.GetItem ("MainActivity.cs")); + + string mainActivityTemplate = @"using System; +using Android.App; +using Android.OS; +using Android.Runtime; +using Android.Views; +using Android.Widget; +using Testing; + +namespace Scratch.JMJMException +{ + [Register (""${JAVA_PACKAGENAME}.MainActivity""), Activity (Label = ""${PROJECT_NAME}"", MainLauncher = true, Icon = ""@drawable/icon"")] + public class MainActivity : Activity + { + protected override void OnCreate (Bundle savedInstanceState) + { + base.OnCreate(savedInstanceState); + Button b = new Button (this) { + Text = ""Click Me!"", + }; + + Testing.Run.TryCatchFinally ( + new Java.Lang.Runnable (() => { + Console.WriteLine (""#UET-1# jon: Should be in a Java > Managed [MainActivity.OnCreate] > Java [Run.tryCatchFinally] > Managed [Run] frame. Throwing an exception...""); + Console.WriteLine (new System.Diagnostics.StackTrace(fNeedFileInfo: true).ToString()); + throw new Exception (""Should be in a Java > Managed [MainActivity.OnCreate] > Java [Run.tryCatchFinally] > Managed [Run] frame. Throwing an exception...""); + }), + new MyCatchHandler (), + new Java.Lang.Runnable (() => { + Console.WriteLine ($""#UET-3# jon: from Java finally block""); + }) + ); + + SetContentView (b); + } + } + + class MyCatchHandler : Java.Lang.Object, Run.ICatchThrowableHandler + { + public void OnCatch (Java.Lang.Throwable t) + { + Console.WriteLine ($""#UET-2# jon: MyCatchHandler.OnCatch: t={t.ToString()}""); + } + } +} +"; + string mainActivity = app.ProcessSourceTemplate (mainActivityTemplate); + app.Sources.Add ( + new BuildItem.Source ("MainActivity.cs") { + TextContent = () => mainActivity + } + ); + + var expectedLogLines = new LogcatLine[] { + new LogcatLine { Text = "#UET-1#" }, + new LogcatLine { Text = "#UET-2#" }, + new LogcatLine { Text = "#UET-3#" }, + }; + + string path = Path.Combine ("temp", TestName); + using (var libBuilder = CreateDllBuilder (Path.Combine (path, lib.ProjectName))) + using (var appBuilder = CreateApkBuilder (Path.Combine (path, app.ProjectName))) { + Assert.True (libBuilder.Build (lib), "Library should have built."); + Assert.IsTrue (appBuilder.Install (app), "Install should have succeeded."); + + ClearAdbLogcat (); + + AdbStartActivity ($"{app.PackageName}/{app.JavaPackageName}.MainActivity"); + + string logcatPath = Path.Combine (Root, appBuilder.ProjectDirectory, "logcat.log"); + int sequenceCounter = 0; + MonitorAdbLogcat ( + (string line) => { + foreach (LogcatLine ll in expectedLogLines) { + if (line.IndexOf (ll.Text, StringComparison.Ordinal) < 0) { + continue; + } + ll.Found = true; + ll.Count++; + ll.SequenceNumber = sequenceCounter++; + break; + } + return false; // we must examine all the lines, and returning `true` aborts the monitoring process + }, logcatPath, 15); + } + + AssertValidLine (0, 0); + AssertValidLine (1, 1); + AssertValidLine (2, 2); + + void AssertValidLine (int idx, int expectedSequence) + { + LogcatLine ll = expectedLogLines [idx]; + Assert.IsTrue (ll.Found, $"Logcat line {idx} was not found"); + Assert.IsTrue (ll.Count == 1, $"Logcat line {idx} should have been found only once but it was found {ll.Count} times"); + Assert.IsTrue (ll.SequenceNumber == expectedSequence, $"Logcat line {idx} sequence number should be {expectedSequence} but it was {ll.SequenceNumber}"); + } + } + } +}