[Mono.Android] Java.Interop Unification!

Context: https://github.com/xamarin/monodroid/commit/e318861ed8eb20a71852378ddd558409d6b1c234
Context: 130905e
Context: de04316
Context: #9636
Context: dotnet/java-interop#1293

In the beginning there was Mono for Android, which had a set of
`Mono.Android.dll` assemblies (one per supported API level), each of
which contained "duplicated" binding logic: each API level had its
own `Java.Lang.Object`, `Android.Runtime.JNIEnv`, etc.

dotnet/java-interop started, in part, as a way to "split out" the
core integration logic, so that it *wouldn't* need to be duplicated
across every assembly.  As part of this, it introduced its own core
abstractions, notably `Java.Interop.IJavaPeerable` and
`Java.Interop.JavaObject`.

When dotnet/java-interop was first introduced into Xamarin.Android,
with xamarin/monodroid@e318861e, the integration was incomplete.
Integration continued with 130905e, allowing unit tests within
`Java.Interop-Tests.dll` to run within Xamarin.Android and
construction of instances of e.g. `JavaInt32Array`, but one large
piece of integration remained:

Moving GC bridge code *out* of `Java.Lang.Object`, and instead
relying on `Java.Interop.JavaObject`, turning this:

	namespace Java.Lang {
	    public partial class Object : System.Object, IJavaPeerable /* … */ {
	    }
	}

into this:

	namespace Java.Lang {
	    public partial class Object : Java.Interop.JavaObject, IJavaPeerable /* … */ {
	    }
	}

*Why*?  In part because @jonpryor has wanted to do this for literal
years at this point, but also in part because of #9636
and related efforts to use Native AOT, which involves avoiding /
bypassing `DllImportAttribute` invocations (for now, everything
touched by Native AOT becomes a single `.so` binary, which we don't
know the name of).  Avoiding P/Invoke means *embracing* and extending
existing Java.Interop constructs (e.g. de04316).

In addition to altering the base types of `Java.Lang.Object` and
`Java.Lang.Throwable`:

  * Remove `handle` and related fields from `Java.Lang.Object` and
    `Java.Lang.Throwable`.

  * Update `PreserveLists/Mono.Android.xml` so that the removed
    fields are note preserved.

  * Rename `JNIenvInit.AndroidValueManager` to
    `JNIEnvInit.ValueManager`, and change its type to
    `JniRuntime.JniValueManager`.  This is to help "force" usage of
    `JnIRuntime.JniValueManager` in more places, as we can't
    currently use `AndroidValueManager` in Native AOT (P/Invokes!).

  * Cleanup: Remove `JNIEnv.Exit()` and related code.  These were
    used by the Android Designer, which is no longer supported.

  * Update (`internal`) interface `IJavaObjectEx` to remove
    constructs present on `IJavaPeerable.`

  * Update `ExceptionTest.CompareStackTraces()` to use
    `System.Diagnostics.StackTrace(ex, fNeedFileInfo:true)`
    so that when the `debug.mono.debug` system property is set, the
    `ExceptionTest.InnerExceptionIsSet()` unit test doesn't fail.
    Also, increase assertion message verbosity.

Author: jonpryor

.gitmodules

@@ -13,7 +13,7 @@
 [submodule "external/Java.Interop"]
     path = external/Java.Interop
     url = https://github.com/dotnet/java-interop
-    branch = main
+    branch = dev/jonp/jonp-overridable-Throwable-ctor-arg
 [submodule "external/libunwind"]
     path = external/libunwind
     url = https://github.com/libunwind/libunwind.git

src/Microsoft.Android.Sdk.ILLink/PreserveLists/Mono.Android.xml

@@ -36,15 +36,7 @@
 		-->
 		<type fullname="Java.Interop.TypeManager*JavaTypeManager" />
 		<type fullname="Java.Lang.Object">
-			<field name="handle" />
-			<field name="handle_type" />
-			<field name="refs_added" />
 			<method name="SetHandleOnDeserialized" />
 		</type>
-		<type fullname="Java.Lang.Throwable">
-			<field name="handle" />
-			<field name="handle_type" />
-			<field name="refs_added" />
-		</type>
 	</assembly>
 </linker>

src/Mono.Android/Android.Runtime/AndroidRuntime.cs

@@ -56,29 +56,36 @@ public override string GetCurrentManagedThreadStackTrace (int skipFrames, bool f
 		{
 			if (!reference.IsValid)
 				return null;
-			var peeked      = JNIEnvInit.AndroidValueManager?.PeekPeer (reference);
+			var peeked      = JNIEnvInit.ValueManager?.PeekPeer (reference);
 			var peekedExc   = peeked as Exception;
 			if (peekedExc == null) {
 				var throwable = Java.Lang.Object.GetObject<Java.Lang.Throwable> (reference.Handle, JniHandleOwnership.DoNotTransfer);
 				JniObjectReference.Dispose (ref reference, options);
 				return throwable;
 			}
 			JniObjectReference.Dispose (ref reference, options);
-			var unwrapped = JNIEnvInit.AndroidValueManager?.UnboxException (peeked!);
+			var unwrapped = UnboxException (peeked!);
 			if (unwrapped != null) {
 				return unwrapped;
 			}
 			return peekedExc;
 		}
 
+		Exception? UnboxException (IJavaPeerable value)
+		{
+			if (JNIEnvInit.ValueManager is AndroidValueManager vm) {
+				return vm.UnboxException (value);
+			}
+			return null;
+		}
+
 		public override void RaisePendingException (Exception pendingException)
 		{
-			var je  = pendingException as JavaProxyThrowable;
+			var je  = pendingException as JavaException;
 			if (je == null) {
 				je  = JavaProxyThrowable.Create (pendingException);
 			}
-			var r = new JniObjectReference (je.Handle);
-			JniEnvironment.Exceptions.Throw (r);
+			JniEnvironment.Exceptions.Throw (je.PeerReference);
 		}
 	}
 
@@ -470,6 +477,7 @@ static bool CallRegisterMethodByIndex (JniNativeMethodRegistrationArguments argu
 			}
 		}
 
+		[Obsolete ("Use RegisterNativeMembers(JniType, Type, ReadOnlySpan<char>) instead.")]
 		public override void RegisterNativeMembers (
 				JniType nativeClass,
 				[DynamicallyAccessedMembers (MethodsAndPrivateNested)]

src/Mono.Android/Android.Runtime/JNIEnv.cs

@@ -76,49 +76,6 @@ internal static bool ShouldWrapJavaException (Java.Lang.Throwable? t, [CallerMem
 			return wrap;
 		}
 
-		[DllImport ("libc")]
-		static extern int gettid ();
-
-		internal static void Exit ()
-		{
-			/* Manually dispose surfaced objects and close the current JniEnvironment to
-			 * avoid ObjectDisposedException thrown on finalizer threads after shutdown
-			 */
-			foreach (var surfacedObject in Java.Interop.Runtime.GetSurfacedObjects ()) {
-				try {
-					var obj = surfacedObject.Target as IDisposable;
-					if (obj != null)
-						obj.Dispose ();
-					continue;
-				} catch (Exception e) {
-					RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"Couldn't dispose object: {e}");
-				}
-				/* If calling Dispose failed, the assumption is that user-code in
-				 * the Dispose(bool) overload is to blame for it. In that case we
-				 * fallback to manual deletion of the surfaced object.
-				 */
-				var jobj = surfacedObject.Target as Java.Lang.Object;
-				if (jobj != null)
-					ManualJavaObjectDispose (jobj);
-			}
-			JniEnvironment.Runtime.Dispose ();
-		}
-
-		/* FIXME: This reproduces the minimal steps in Java.Lang.Object.Dispose
-		 * that needs to be executed so that we don't leak any GREF and prevent
-		 * code execution into an appdomain that we are disposing via a finalizer.
-		 * Ideally it should be done via another more generic mechanism, likely
-		 * from the Java.Interop.Runtime API.
-		 */
-		static void ManualJavaObjectDispose (Java.Lang.Object obj)
-		{
-			var peer = obj.PeerReference;
-			var handle = peer.Handle;
-			var keyHandle = ((IJavaObjectEx)obj).KeyHandle;
-			Java.Lang.Object.Dispose (obj, ref handle, keyHandle, (JObjectRefType)peer.Type);
-			GC.SuppressFinalize (obj);
-		}
-
 		internal static void PropagateUncaughtException (IntPtr env, IntPtr javaThreadPtr, IntPtr javaExceptionPtr)
 		{
 			if (!JNIEnvInit.PropagateExceptions)

src/Mono.Android/Android.Runtime/JNIEnvInit.cs

@@ -35,7 +35,7 @@ internal struct JnienvInitializeArgs {
 		}
 #pragma warning restore 0649
 
-		internal static AndroidValueManager? AndroidValueManager;
+		internal static JniRuntime.JniValueManager? ValueManager;
 		internal static bool IsRunningOnDesktop;
 		internal static bool jniRemappingInUse;
 		internal static bool MarshalMethodsEnabled;
@@ -94,7 +94,7 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args)
 
 			BoundExceptionType = (BoundExceptionType)args->ioExceptionType;
 			androidRuntime = new AndroidRuntime (args->env, args->javaVm, args->grefLoader, args->Loader_loadClass, args->jniAddNativeMethodRegistrationAttributePresent != 0);
-			AndroidValueManager = (AndroidValueManager) androidRuntime.ValueManager;
+			ValueManager = androidRuntime.ValueManager;
 
 			IsRunningOnDesktop = args->isRunningOnDesktop == 1;
 

src/Mono.Android/Android.Runtime/JavaProxyThrowable.cs

@@ -116,6 +116,7 @@ public static JavaProxyThrowable Create (Exception innerException)
 
 		void TranslateStackTrace ()
 		{
+			Console.WriteLine ("# jonp: JavaProxyThrowable.TranslateStackTrace: InnerException={0} {1}", InnerException?.GetType(), InnerException);
 			// FIXME: https://github.com/xamarin/xamarin-android/issues/8724
 			// StackFrame.GetMethod() will return null under NativeAOT;
 			// However, you can still get useful information from StackFrame.ToString():

src/Mono.Android/Java.Interop/IJavaObjectEx.cs

@@ -3,9 +3,6 @@
 namespace Java.Interop {
 
 	interface IJavaObjectEx {
-		IntPtr      KeyHandle         {get; set;}
-		bool        IsProxy           {get; set;}
-		bool        NeedsActivation   {get; set;}
 		IntPtr      ToLocalJniHandle ();
 	}
 }

src/Mono.Android/Java.Interop/Runtime.cs

@@ -11,7 +11,7 @@ public static class Runtime {
 		[Obsolete ("Please use Java.Interop.JniEnvironment.Runtime.ValueManager.GetSurfacedPeers()")]
 		public static List<WeakReference> GetSurfacedObjects ()
 		{
-			var peers = JNIEnvInit.AndroidValueManager!.GetSurfacedPeers ();
+			var peers = JNIEnvInit.ValueManager!.GetSurfacedPeers ();
 			var r = new List<WeakReference> (peers.Count);
 			foreach (var p in peers) {
 				if (p.SurfacedPeer.TryGetTarget (out var target))

src/Mono.Android/Java.Interop/TypeManager.cs

@@ -133,9 +133,10 @@ static Type[] GetParameterTypes (string? signature)
 		static void n_Activate (IntPtr jnienv, IntPtr jclass, IntPtr typename_ptr, IntPtr signature_ptr, IntPtr jobject, IntPtr parameters_ptr)
 		{
 			var o   = Java.Lang.Object.PeekObject (jobject);
-			var ex  = o as IJavaObjectEx;
+			var ex  = o as IJavaPeerable;
 			if (ex != null) {
-				if (!ex.NeedsActivation && !ex.IsProxy)
+				var state = ex.JniManagedPeerState;
+				if (!state.HasFlag (JniManagedPeerStates.Activatable) && !state.HasFlag (JniManagedPeerStates.Replaceable))
 					return;
 			}
 			if (!ActivationEnabled) {
@@ -171,10 +172,8 @@ internal static void Activate (IntPtr jobject, ConstructorInfo cinfo, object? []
 		{
 			try {
 				var newobj = RuntimeHelpers.GetUninitializedObject (cinfo.DeclaringType!);
-				if (newobj is Java.Lang.Object o) {
-					o.handle = jobject;
-				} else if (newobj is Java.Lang.Throwable throwable) {
-					throwable.handle = jobject;
+				if (newobj is IJavaPeerable peer) {
+					peer.SetPeerReference (new JniObjectReference (jobject));
 				} else {
 					throw new InvalidOperationException ($"Unsupported type: '{newobj}'");
 				}
@@ -232,7 +231,7 @@ static Exception CreateJavaLocationException ()
 			return null;
 		}
 
-		internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership transfer)
+		internal static IJavaPeerable? CreateInstance (IntPtr handle, JniHandleOwnership transfer)
 		{
 			return CreateInstance (handle, transfer, null);
 		}