From 0f67df437f255cf3942c3a94dc35e250f7081760 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 20 Jul 2021 12:14:03 +0200 Subject: [PATCH] Use `mono_unhandled_exception` for NET6 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 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. --- 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}"); + } + } + } +}