[java-interop, Java.Interop] Start plumbing a proper Mono GC Bridge

Context: #13

What do we want?  For cross-VM object references to work -- a
java.util.ArrayList referencing a MyJavaObjectSubclass instance, and
being the *only* reference, yet the MyJavaObjectSubclass instance
isn't collected, *because* the Java-side ArrayList is keeping it
alive.

See also commit eb69a47.

What we currently have is a terrible solution.  (Even as a terrible
solution, it's a terrible solution!)

Fortunately, we know what we need to do: import the Xamarin.Android GC
bridge implementation, which is (reasonably) well debugged and has
been used for *years*.

Update the java-interop native library, adding a new
JavaInteropGCBridge API (in <java-interop-gc-bridge.h>), and implement
that API for use on Mono.

(Currently unknown: how feasible it'll be to provide a similar API for
use with CoreCLR, which is an entirely different can of worms...)

Changes from the Xamarin.Android GC bridge are as follows:

  * It's (mostly!) scoped to an instance of JavaInteropGCBridge, as
    opposed to the Xamarin.Android implementation, which is dozens of
    global variables. (There's still a global, but now it's just
    `mono_bridge`, which is a pointer to an easier-to-reason-about
    struct.

  * Code has been reformatted for consistency, and to conform to
    https://matt.sh/howto-c , which in turn follows common C++
    practice -- declare variables at initialization, etc.

  * The JNIEnv.BridgeProcessing field is now the
    MonoRuntimeValueManager.GCBridgeProcessingIsActive field, though
    the *actual* field to use can be specified via
    java_interop_gc_bridge_set_bridge_processing_field().

  * Instead of always using Java.Lang.Object and Java.Land.Throwable
    (from Mono.Android.dll) as brided types, allow the bridged types
    to be specified via
    java_interop_gc_bridge_register_bridgeable_type().

...which is to say that there are some stylistic changes, but no
concrete algorithm changes or structural changes to the GC bridge
itself (aside from some function re-ordering).

With the JavaInteropGCBridge API in place, *integrate* it with
Java.Interop proper, which in turn is kinda complicated, as the GC
bridge is specific to Mono (+SGEN!), and will not run on .NET.

To square this circle, we make JniRuntime.JniValueManager an
*abstract* type, with a new abstract WaitForGCBridgeProcessing()
method, and concretely implement this type in
Java.Interop.MonoRuntimeValueManager (in
Java.Runtime.Environment.dll).  We then do a runtime check to see if
we're on mono; if we are, we use MonoRuntimeValueManager; otherwise,
we through an exception so whoever is setting things up can provide
their own JniRuntime.JniValueManager implementation.

Update ExportedMemberBuilder so that
JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing() is
called at entry from unmanaged to managed code, as is done with
JNIEnv.WaitForGCBridgeProcessing() in Xamarin.Android.

Notes:

  * Xamarin.Android used mono_assembly_load_full() in order to load
    Mono.Android.dll and find the appropriate types.  Trying to do
    this in java-interop caused a crash for an unexplored reason.
    To avoid the crash, mono_assembly_load_full() is *skipped*
    entirely.  Instead, a System.RuntimeTypeHandle is passed from
    managed code to java-interop to act upon the intended types.

    This in turn raises a concern, as we're passing the
    System.RuntimeTypeHandle *structure* to P/Invoke, which *works*,
    but I don't know how *stable* that is gauranteed to be.
    *Better* would be to use RuntimeTypeHandle.Value (an IntPtr), but
    for some asinine reason that property is NOT in PCL Profile7,
    meaning I can't access it from a PCL lib.

    Question: How problematic is this?

  * The JavaInteropGCBridge logging implementation records the thread
    name and thread ID of the thread creating/destroying the JNI
    handle.  This is currently done by registering a delegate from
    managed code, which uses Thread.CurrentThread.Name and
    Thread.ManagedThreadId.

    WANTED: A Mono embedding API which provides a way to get the
    thread name and thread id of a MonoThread* value.  This would
    avoid an unmanaged > managed transition during logging.

  * The GC bridge sets the static
    MonoRuntimeValueManager.GCBridgeProcessingIsActive field within
    *all* AppDomains.  This is so that Java.Interop can be used in
    multiple AppDomains...because why not?

    However, this is problematic because we're using
    mono_class_vtable() to get the MonoVTable* to set the static
    field, and mono_class_vtable() *always loads the type*, which is
    inappropriate if Java.Runtime.Environment.dll hasn't been loaded
    into the current AppDomain.

    WANTED: A Mono embedding API like mono_class_vtable() which
    returns NULL if the type hasn't already been loaded into the
    specified MonoDomain*.  That way we can set
    MonoRuntimeValueManager.GCBridgeProcessingIsActive within the
    AppDomains that have it, but not *every* AppDomain.

TODO:

While this commit *adds* a GC bridge implementation, it is not
currently *used*.  Here are some of the things that need to be done to
do that, among others:

  * Review the JniRuntime.JniValueManager API for sanity.
    Ideally it should be possible to implement "sanely" on .NET,
    though possibly *without* GC integration.  Perhaps "manual" GCs
    are supportable, e.g. a JniRuntime.ValueManager.Collect() method
    which the developer must invoke occasionally.
    (Terrible, certainly, but possibly useful for prototypes?)

  * SafeHandles must be dropped -- disable
    FEATURE_JNIENVIRONMENT_SAFEHANDLES and use
    FEATURE_JNIENVIRONMENT_JI_PINVOKES (commit add10d1).
    JavaInteropGCBridge only knows about "handle" and "handle_type"
    fields (among others); it doesn't support SafeHandles, so in order
    for Java.Interop.JavaObject to be usable with JavaInteropGCBridge,
    we need to drop SafeHandle use.

  * JavaInteropGCBridge requires that the Java Callable Wrapper
    (formerly Android Callable Wrapper) provide methods named
    monodroidAddReference(java.lang.Object) and
    monodroidClearReferences().  No Java types within Java.Interop
    currently provide these methods, meaning the *very crucial glue*
    to support cross-VM object references is *missing*.

    We should provide a variant on the Xamarin.Android
    mono.android.IGCUserPeer interface for use with Java.Interop, and
    improve JavaInteropGCBridge to use the interface instead of
    looking up the jmethodID values afresh for every encountered type.

  * We have *two* separate JNI handle loggers now: one in
    java-interop, and one in
    LoggingJniObjectReferenceManagerDecorator. We should kill the
    LoggingJniObjectReferenceManagerDecorator type.

Author: jonpryor

src/Java.Interop.Export/Java.Interop/ExportedMemberBuilder.cs

@@ -136,8 +136,11 @@ public LambdaExpression CreateMarshalToManagedExpression (MethodInfo method, Jav
 				Expression.Assign (envp, CreateJniTransition (jnienv)),
 			};
 
+			var waitForGCBridge     = typeof(JniRuntime.JniValueManager)
+				.GetRuntimeMethod (nameof (JniRuntime.JniValueManager.WaitForGCBridgeProcessing), new Type [0]);
 			var marshalBody = new List<Expression> () {
 				Expression.Assign (jvm, GetRuntime ()),
+				Expression.Call (Expression.Property (jvm, "ValueManager"), waitForGCBridge),
 			};
 
 			Expression self = null;

src/Java.Interop.Export/Tests/Java.Interop/ExportedMemberBuilderTest.cs

@@ -173,6 +173,7 @@ public void CreateMarshalFromJniMethodExpression_InstanceAction ()
 	try
 	{
 		__jvm = JniEnvironment.Runtime;
+		__jvm.ValueManager.WaitForGCBridgeProcessing();
 		__this_val = __jvm.ValueManager.GetValue<ExportTest>(__this);
 		__this_val.InstanceAction();
 	}
@@ -229,6 +230,7 @@ public void CreateMarshalFromJniMethodExpression_StaticAction ()
 	try
 	{
 		__jvm = JniEnvironment.Runtime;
+		__jvm.ValueManager.WaitForGCBridgeProcessing();
 		ExportTest.StaticAction();
 	}
 	catch (Exception __e)
@@ -261,6 +263,7 @@ public void CreateMarshalFromJniMethodExpression_StaticActionInt32String ()
 	try
 	{
 		__jvm = JniEnvironment.Runtime;
+		__jvm.ValueManager.WaitForGCBridgeProcessing();
 		v_val = Strings.ToString(v);
 		ExportTest.StaticActionInt32String(i, v_val);
 	}
@@ -297,6 +300,7 @@ public void CreateMarshalFromJniMethodExpression_StaticFuncMyLegacyColorMyColor_
 	try
 	{
 		__jvm = JniEnvironment.Runtime;
+		__jvm.ValueManager.WaitForGCBridgeProcessing();
 		color1_val = new MyLegacyColor(color1);
 		color2_val = new MyColor(color2);
 		__mret = ExportTest.StaticFuncMyLegacyColorMyColor_MyColor(color1_val, color2_val);
@@ -335,6 +339,7 @@ public void CreateMarshalFromJniMethodExpression_FuncInt64 ()
 	try
 	{
 		__jvm = JniEnvironment.Runtime;
+		__jvm.ValueManager.WaitForGCBridgeProcessing();
 		__this_val = __jvm.ValueManager.GetValue<ExportTest>(__this);
 		__mret = __this_val.FuncInt64();
 		return __mret;
@@ -374,6 +379,7 @@ public void CreateMarshalFromJniMethodExpression_FuncIJavaObject ()
 	try
 	{
 		__jvm = JniEnvironment.Runtime;
+		__jvm.ValueManager.WaitForGCBridgeProcessing();
 		__this_val = __jvm.ValueManager.GetValue<ExportTest>(__this);
 		__mret = __this_val.FuncIJavaObject();
 		if (null == __mret)
@@ -422,6 +428,7 @@ public void CreateMarshalToManagedExpression_DirectMethod ()
 	try
 	{
 		__jvm = JniEnvironment.Runtime;
+		__jvm.ValueManager.WaitForGCBridgeProcessing();
 		ExportedMemberBuilderTest.DirectInvocation(jnienv, context);
 	}
 	catch (Exception __e)

src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs

@@ -4,6 +4,8 @@
 using System.Linq.Expressions;
 using System.Reflection;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+using System.Threading;
 
 using Java.Interop.Expressions;
 
@@ -19,10 +21,15 @@ partial class CreationOptions {
 
 		partial void SetValueManager (CreationOptions options)
 		{
-			ValueManager  = SetRuntime (options.ValueManager ?? new JniValueManager ());
+			var manager     = options.ValueManager;
+			if (manager == null)
+				throw new ArgumentException (
+						"No JniValueManager specified in JniRuntime.CreationOptions.ValueManager.",
+						nameof (options));
+			ValueManager    = SetRuntime (manager);
 		}
 
-		public partial class JniValueManager : ISetRuntime, IDisposable {
+		public abstract partial class JniValueManager : ISetRuntime, IDisposable {
 
 			public      JniRuntime  Runtime { get; private set; }
 
@@ -47,11 +54,15 @@ protected virtual void Dispose (bool disposing)
 
 				foreach (var o in RegisteredInstances.Values) {
 					var t = (IDisposable) o.Target;
+					if (t == null)
+						continue;
 					t.Dispose ();
 				}
 				RegisteredInstances.Clear ();
 			}
 
+			public abstract void WaitForGCBridgeProcessing ();
+
 			Dictionary<int, WeakReference>  RegisteredInstances = new Dictionary<int, WeakReference>();
 
 			public List<WeakReference> GetSurfacedObjects ()

src/Java.Interop/Java.Interop/JniRuntime.cs

@@ -171,17 +171,17 @@ protected JniRuntime (CreationOptions options)
 			TrackIDs     = options.TrackIDs;
 			DestroyRuntimeOnDispose     = options.DestroyRuntimeOnDispose;
 
-			ObjectReferenceManager      = SetRuntime (options.ObjectReferenceManager ?? new JniObjectReferenceManager ());
-			TypeManager                 = SetRuntime (options.TypeManager ?? new JniTypeManager ());
-
-			SetValueManager (options);
-			SetExportedMemberBuilder (options);
-
 			NewObjectRequired   = options.NewObjectRequired;
 
 			InvocationPointer   = options.InvocationPointer;
 			Invoker             = CreateInvoker (InvocationPointer);
 
+			SetValueManager (options);
+			SetExportedMemberBuilder (options);
+
+			ObjectReferenceManager      = SetRuntime (options.ObjectReferenceManager ?? new JniObjectReferenceManager ());
+			TypeManager                 = SetRuntime (options.TypeManager ?? new JniTypeManager ());
+
 			if (Interlocked.CompareExchange (ref current, this, null) != null) {
 				Debug.WriteLine ("WARNING: More than one JniRuntime instance created. This is DOOMED TO FAIL.");
 			}

src/Java.Interop/Tests/Java.Interop/JniRuntime.JniValueManagerTests.cs

@@ -12,7 +12,7 @@ public class JniRuntimeJniValueManagerTests : JavaVMFixture {
 		[Test]
 		public void CreateValue ()
 		{
-			using (var vm  = new JniRuntime.JniValueManager ())
+			using (var vm  = new MyValueManager ())
 			using (var o = new JavaObject ()) {
 				vm.OnSetRuntime (JniRuntime.CurrentRuntime);
 
@@ -27,6 +27,13 @@ public void CreateValue ()
 			}
 		}
 
+		class MyValueManager : JniRuntime.JniValueManager {
+
+			public override void WaitForGCBridgeProcessing ()
+			{
+			}
+		}
+
 		[Test]
 		public void GetValue_ReturnsAlias ()
 		{

src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs

@@ -38,6 +38,9 @@ public JreRuntimeOptions ()
 					Path.GetDirectoryName (typeof (JreRuntimeOptions).Assembly.Location),
 					"java-interop.jar"),
 			};
+
+			if (ValueManager == null && Type.GetType ("Mono.Runtime", throwOnError: false) != null)
+				ValueManager    = new MonoRuntimeValueManager ();
 		}
 
 		public JreRuntimeOptions AddOption (string option)

src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs

@@ -0,0 +1,95 @@
+using System;
+using System.Runtime.InteropServices;
+
+namespace Java.Interop {
+
+	enum GCBridgeUseWeakReferenceKind {
+		Java,
+		Jni,
+	}
+
+	class MonoRuntimeValueManager : JniRuntime.JniValueManager {
+
+		#pragma warning disable 0649
+		// This field is mutated by the java-interop native lib
+		static  volatile    bool                    GCBridgeProcessingIsActive;
+		#pragma warning restore 0649
+
+		GetThreadDescriptionCb                      getThreadDescription;
+
+		public override void OnSetRuntime (JniRuntime runtime)
+		{
+			base.OnSetRuntime (runtime);
+			getThreadDescription    = x => java_interop_strdup (runtime.GetCurrentThreadDescription ());
+
+			var bridge = java_interop_gc_bridge_get_current ();
+			if (bridge != IntPtr.Zero)
+				return;
+
+			bridge  = java_interop_gc_bridge_new (runtime.InvocationPointer);
+			if (bridge == IntPtr.Zero)
+				throw new NotSupportedException ("Could not initialize JNI::Mono GC Bridge!");
+
+			try {
+				if (java_interop_gc_bridge_set_thread_description_creator (bridge, getThreadDescription, IntPtr.Zero) < 0)
+					throw new NotSupportedException ("Could not set thread description creator!");
+				if (java_interop_gc_bridge_set_bridge_processing_field (bridge, typeof (MonoRuntimeValueManager).TypeHandle, nameof (GCBridgeProcessingIsActive)) < 0)
+					throw new NotSupportedException ("Could not set bridge processing field!");
+				foreach (var t in new[]{typeof (JavaObject), typeof (JavaException)}) {
+					if (java_interop_gc_bridge_register_bridgeable_type (bridge, t.TypeHandle) < 0)
+						throw new NotSupportedException ("Could not register type " + t.FullName + "!");
+				}
+				if (java_interop_gc_bridge_set_current_once (bridge) < 0)
+					throw new NotSupportedException ("Could not set GC Bridge instance!");
+			}
+			catch (Exception) {
+				java_interop_gc_bridge_free (bridge);
+				throw;
+			}
+			if (java_interop_gc_bridge_register_hooks_once (GCBridgeUseWeakReferenceKind.Jni) < 0)
+				throw new NotSupportedException ("Could not register GC Bridge with Mono!");
+		}
+
+		public override void WaitForGCBridgeProcessing ()
+		{
+			if (!GCBridgeProcessingIsActive)
+				return;
+			java_interop_gc_bridge_wait_for_bridge_processing ();
+		}
+
+		const   string JavaInteropLib = "java-interop";
+
+		delegate    IntPtr  GetThreadDescriptionCb (IntPtr user_data);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern IntPtr java_interop_gc_bridge_get_current ();
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern int java_interop_gc_bridge_set_current_once (IntPtr bridge);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern int java_interop_gc_bridge_register_hooks_once (GCBridgeUseWeakReferenceKind weak_ref_kind);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern IntPtr java_interop_gc_bridge_new (IntPtr jvm);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern int java_interop_gc_bridge_free (IntPtr bridge);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern int java_interop_gc_bridge_set_thread_description_creator (IntPtr bridge, GetThreadDescriptionCb creator, IntPtr user_data);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern IntPtr java_interop_strdup (string value);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern int java_interop_gc_bridge_set_bridge_processing_field (IntPtr bridge, RuntimeTypeHandle type_handle, string field_name);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern int java_interop_gc_bridge_register_bridgeable_type (IntPtr bridge, RuntimeTypeHandle type_handle);
+
+		[DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)]
+		static extern void java_interop_gc_bridge_wait_for_bridge_processing ();
+	}
+}
+

src/Java.Runtime.Environment/Java.Runtime.Environment.csproj

@@ -34,6 +34,7 @@
   <ItemGroup>
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Java.Interop\JreRuntime.cs" />
+    <Compile Include="Java.Interop\MonoRuntimeValueManager.cs" />
   </ItemGroup>
   <ItemGroup>
     <Reference Include="System" />