[Java.Interop, Java.Interop.Dynamic] Dispose() JniPeerMembers.

Running Java.Interop.Dynamic-Tests would result in lots of
"Deleting JNI local reference handle <HANDLE> from wrong thread"
messages written to the JNI local reference log. These were coming
from all the various JniType instances involved ~everywhere:

  * JniPeerMembers.JniPeerType
  * JavaMethodInvokeInfo.ReturnType
  * JavaMethodInvokeInfo.Arguments
  * JavaMethodInvokeInfo.Method

Plus associated Jni*FieldID and Jni*MethodID values (though those
won't cause the above "Deleting..." messages.)

There are two plausible fixes:

 1. Stop using JNI Local References for everything!
    #6
    JNI Handles would still need to be used, but by making them
    *Global* references the GC can freely clean them up.

 2. Add an explicit cleanup mechanism.

We're opting for (2) for now. :-)
((1) is later.)

That said, (2) is usually done by implementing IDisposable, which
we're doing for DynamicJavaClass. Why not have JniPeerMembers and
company implement IDisposable?

Because I don't want some asshole coming through and going:

	var o = new JavaObject();
	o.JniPeerMembers.Dispose();

...which would promptly blow up fucking everything.

Instead, we're adding a *static* JniPeerMembers.Dispose() method,
which won't show up in normal JniPeerMembers code completion, and will
prevent using JniPeerMembers instances in `using` blocks.

Once JniPeerMembers has a cleanup method, extend DynamicJavaClass to
implement IDispoable. This allows our tests to cleanup after
themselves, reducing spam to the LREF log.

Author: jonpryor

src/Java.Interop.Dynamic/Java.Interop.Dynamic/DynamicJavaClass.cs

@@ -13,7 +13,7 @@
 
 namespace Java.Interop.Dynamic {
 
-	public class DynamicJavaClass : IDynamicMetaObjectProvider
+	public class DynamicJavaClass : IDynamicMetaObjectProvider, IDisposable
 	{
 		readonly    static  Func<string, JniPeerMembers>    CreatePeerMembers;
 
@@ -67,6 +67,7 @@ static DynamicJavaClass ()
 		public  string          JniClassName            {get; private set;}
 
 		JniPeerMembers          members;
+		bool                    disposed;
 
 		Dictionary<string, HashSet<string>>                 StaticFields;
 		Dictionary<string, List<JavaMethodInvokeInfo>>      StaticMethods;
@@ -80,9 +81,35 @@ public DynamicJavaClass (string jniClassName)
 			members         = CreatePeerMembers (jniClassName);
 		}
 
+		public void Dispose ()
+		{
+			Dispose (disposing: true);
+			GC.SuppressFinalize (this);
+		}
+
+		protected virtual void Dispose (bool disposing)
+		{
+			if (!disposing)
+				return;
+
+			if (disposed)
+				return;
+
+			foreach (var name in StaticMethods.Keys.ToList ()) {
+				foreach (var info in StaticMethods [name])
+					info.Dispose ();
+				StaticMethods [name]    = null;
+			}
+
+			JniPeerMembers.Dispose (members);
+			members     = null;
+
+			disposed    = true;
+		}
+
 		void LookupMethods ()
 		{
-			if (StaticMethods != null)
+			if (StaticMethods != null || disposed)
 				return;
 
 			StaticMethods   = new Dictionary<string, List<JavaMethodInvokeInfo>> ();
@@ -110,7 +137,7 @@ void LookupMethods ()
 
 		void LookupFields ()
 		{
-			if (StaticFields != null)
+			if (StaticFields != null || disposed)
 				return;
 
 			StaticFields    = new Dictionary<string, HashSet<string>> ();
@@ -141,8 +168,6 @@ void LookupFields ()
 
 		DynamicMetaObject IDynamicMetaObjectProvider.GetMetaObject (Expression parameter)
 		{
-			if (members == null)
-				throw new ObjectDisposedException (nameof (DynamicJavaClass));
 			return new MetaObject (parameter, this);
 		}
 
@@ -185,6 +210,9 @@ static List<JniType> GetJniTypes (DynamicMetaObject[] args)
 				try {
 					var at  = new JniType (vm.GetJniTypeInfoForType (a.LimitType).JniTypeReference);
 					r.Add (at);
+				} catch (JavaException e) {
+					e.Dispose ();
+					r.Add (null);
 				} catch {
 					r.Add (null);
 				}
@@ -234,30 +262,43 @@ public override DynamicMetaObject BindGetMember (GetMemberBinder binder)
 				if (overloads == null)
 					return binder.FallbackGetMember (this);
 
+				if (Value.disposed) {
+					return new DynamicMetaObject (ThrowObjectDisposedException (typeof (object)), BindingRestrictions.GetInstanceRestriction (Expression, Value));
+				}
+
 				Func<string, object>    getValue    = Value.members.StaticFields.GetValue;
 
 				var e = Expression.Call (Expression.Constant (Value.members.StaticFields), getValue.Method, Expression.Constant (overloads.First ()));
 				Debug.WriteLine ("# MetaObject.BindGetMember: e={0}", e.ToCSharpCode ());
 				return new DynamicMetaObject (e, BindingRestrictions.GetInstanceRestriction (Expression, Value));
 			}
 
+			static Expression ThrowObjectDisposedException (Type type = null)
+			{
+				return Expression.Throw (Expression.Constant (new ObjectDisposedException (nameof (DynamicJavaClass))), type);
+			}
+
 			HashSet<string> GetField (string name)
 			{
 				Value.LookupFields ();
 
 				HashSet<string> overloads;
-				if (Value.StaticFields.TryGetValue (name, out overloads))
+				if (Value.StaticFields != null && Value.StaticFields.TryGetValue (name, out overloads))
 					return overloads;
 				return null;
 			}
 
 			public override DynamicMetaObject BindInvokeMember (InvokeMemberBinder binder, DynamicMetaObject[] args)
 			{
 				Value.LookupMethods ();
-				List<JavaMethodInvokeInfo> overloads;
-				if (!Value.StaticMethods.TryGetValue (binder.Name, out overloads))
+				List<JavaMethodInvokeInfo> overloads    = null;
+				if (Value.StaticMethods != null && !Value.StaticMethods.TryGetValue (binder.Name, out overloads))
 					return binder.FallbackInvokeMember (this, args);
 
+				if (Value.disposed) {
+					return new DynamicMetaObject (ThrowObjectDisposedException (typeof (object)), BindingRestrictions.GetInstanceRestriction (Expression, Value));
+				}
+
 				foreach (var m in overloads)
 					m.LookupArguments ();
 
@@ -284,6 +325,10 @@ public override DynamicMetaObject BindSetMember (SetMemberBinder binder, Dynamic
 				if (overloads == null)
 					return binder.FallbackSetMember (this, value);
 
+				if (Value.disposed) {
+					return new DynamicMetaObject (ThrowObjectDisposedException (), BindingRestrictions.GetInstanceRestriction (Expression, Value));
+				}
+
 				Action<string, object>  setValue    = Value.members.StaticFields.SetValue;
 				var e = Expression.Block (
 						Expression.Call (Expression.Constant (Value.members.StaticFields), setValue.Method,
@@ -333,8 +378,16 @@ public void Dispose ()
 			Method.Dispose ();
 			ReturnType.Dispose ();
 			ReturnType  = null;
-			for (int i = 0; i < Arguments.Count; ++i)
+
+			if (Arguments == null)
+				return;
+
+			for (int i = 0; i < Arguments.Count; ++i) {
+				if (Arguments [i] == null)
+					continue;
 				Arguments [i].Dispose ();
+				Arguments [i] = null;
+			}
 			Arguments   = null;
 		}
 

src/Java.Interop.Dynamic/Tests/Java.Interop.Dynamic/DynamicJavaClassTests.cs

@@ -40,11 +40,27 @@ public void Constructor ()
 			Assert.Throws<ArgumentNullException> (() => new DynamicJavaClass (null));
 		}
 
+		[Test]
+		public void DisposedInstanceThrowsObjectDisposedException ()
+		{
+			dynamic Integer = new DynamicJavaClass (Integer_class);
+			Integer.Dispose ();
+			Integer.Dispose ();  // Dispose() is idempotent
+			Assert.Catch<Exception> (() => Integer.bitCount (2));
+			Assert.Catch<Exception> (() => {
+				int max = Integer.MAX_INT;
+			});
+			Assert.Catch<Exception> (() => {
+				Integer.MAX_INT = 42;
+			});
+		}
+
 		[Test]
 		public void JniClassName ()
 		{
 			dynamic Arrays  = new DynamicJavaClass (Arrays_class);
 			Assert.AreEqual (Arrays_class, Arrays.JniClassName);
+			Arrays.Dispose ();
 		}
 
 		[Test]
@@ -55,6 +71,7 @@ public void CallStaticMethod ()
 			int value       = 3;
 			int index       = Arrays.binarySearch (array, value);
 			Assert.AreEqual (2, index);
+			Arrays.Dispose ();
 		}
 
 		[Test]
@@ -63,6 +80,7 @@ public void ReadStaticMember ()
 			dynamic Integer = new DynamicJavaClass (Integer_class);
 			int max = Integer.MAX_VALUE;
 			Assert.AreEqual (int.MaxValue, max);
+			Integer.Dispose ();
 		}
 
 		[Test]
@@ -77,6 +95,7 @@ public void WriteStaticMember ()
 			Assert.AreEqual (42, max);
 			Integer.MAX_VALUE   = cur;
 			Console.WriteLine ("# done!");
+			Integer.Dispose ();
 		}
 
 		[Test]
@@ -87,6 +106,7 @@ public void FallbackPropertySet ()
 			d.MyProperty    = 42;
 			int v           = d.MyProperty;
 			Assert.AreEqual (42, v);
+			d.Dispose ();
 		}
 
 		[Test]
@@ -95,6 +115,7 @@ public void FallbackInvokeMember ()
 			dynamic d   = new MyDynamicObject ();
 			int v       = d.Method ("foo");
 			Assert.AreEqual (3, v);
+			d.Dispose ();
 		}
 	}
 }

src/Java.Interop/Java.Interop/JniPeerInstanceFields.cs

@@ -11,7 +11,18 @@ internal JniPeerInstanceFields (JniPeerMembers members)
 		}
 
 		readonly JniPeerMembers                             Members;
-		readonly Dictionary<string, JniInstanceFieldID>     InstanceFields  = new Dictionary<string, JniInstanceFieldID>();
+
+		Dictionary<string, JniInstanceFieldID>              InstanceFields  = new Dictionary<string, JniInstanceFieldID>();
+
+		internal void Dispose ()
+		{
+			if (InstanceFields == null)
+				return;
+
+			foreach (var f in InstanceFields.Values)
+				f.Dispose ();
+			InstanceFields  = null;
+		}
 
 		public JniInstanceFieldID GetFieldID (string encodedMember)
 		{

src/Java.Interop/Java.Interop/JniPeerInstanceMethods.cs

@@ -26,9 +26,27 @@ internal JniPeerInstanceMethods (JniPeerMembers members)
 		}
 
 		readonly Type                                       DeclaringType;
-		readonly JniType                                    JniPeerType;
-		readonly Dictionary<string, JniInstanceMethodID>    InstanceMethods = new Dictionary<string, JniInstanceMethodID>();
-		readonly Dictionary<Type, JniPeerInstanceMethods>   SubclassConstructors = new Dictionary<Type, JniPeerInstanceMethods> ();
+
+		JniType                                             JniPeerType;
+		Dictionary<string, JniInstanceMethodID>             InstanceMethods = new Dictionary<string, JniInstanceMethodID>();
+		Dictionary<Type, JniPeerInstanceMethods>            SubclassConstructors = new Dictionary<Type, JniPeerInstanceMethods> ();
+
+		internal void Dispose ()
+		{
+			if (JniPeerType == null)
+				return;
+
+			// Don't dispose JniPeerType; it's shared with others.
+			foreach (var m in InstanceMethods.Values)
+				m.Dispose ();
+			InstanceMethods         = null;
+
+			foreach (var p in SubclassConstructors.Values)
+				p.Dispose ();
+			SubclassConstructors    = null;
+
+			JniPeerType = null;
+		}
 
 		public JniInstanceMethodID GetConstructor (string signature)
 		{

src/Java.Interop/Java.Interop/JniPeerMembers.cs

@@ -43,10 +43,10 @@ static JniPeerMembers CreatePeerMembers (string jniPeerType)
 
 		JniType     jniPeerType;
 
-		readonly    JniPeerInstanceMethods  instanceMethods;
-		readonly    JniPeerInstanceFields   instanceFields;
-		readonly    JniPeerStaticMethods    staticMethods;
-		readonly    JniPeerStaticFields     staticFields;
+		JniPeerInstanceMethods  instanceMethods;
+		JniPeerInstanceFields   instanceFields;
+		JniPeerStaticMethods    staticMethods;
+		JniPeerStaticFields     staticFields;
 
 		public      Type        ManagedPeerType {get; private set;}
 		public      string      JniPeerTypeName {get; private set;}
@@ -59,19 +59,48 @@ public      JniType     JniPeerType {
 		}
 
 		public  JniPeerInstanceMethods  InstanceMethods {
-			get {return instanceMethods;}
+			get {return Assert (instanceMethods);}
 		}
 
 		public  JniPeerInstanceFields   InstanceFields {
-			get {return instanceFields;}
+			get {return Assert (instanceFields);}
 		}
 
 		public  JniPeerStaticMethods    StaticMethods {
-			get {return staticMethods;}
+			get {return Assert (staticMethods);}
 		}
 
 		public  JniPeerStaticFields     StaticFields {
-			get {return staticFields;}
+			get {return Assert (staticFields);}
+		}
+
+		static T Assert<T>(T value)
+			where T : class
+		{
+			if (value == null)
+				throw new ObjectDisposedException (nameof (JniPeerMembers));
+			return value;
+		}
+
+		public static void Dispose (JniPeerMembers members)
+		{
+			if (members.jniPeerType == null)
+				return;
+
+			members.instanceMethods.Dispose ();
+			members.instanceMethods = null;
+
+			members.instanceFields.Dispose ();
+			members.instanceFields  = null;
+
+			members.staticMethods.Dispose ();
+			members.staticMethods   = null;
+
+			members.staticFields.Dispose ();
+			members.staticFields    = null;
+
+			members.jniPeerType.Dispose ();
+			members.jniPeerType     = null;
 		}
 
 		internal static void AssertSelf (IJavaObject self)

src/Java.Interop/Java.Interop/JniPeerStaticFields.cs

@@ -11,7 +11,8 @@ internal JniPeerStaticFields (JniPeerMembers members)
 		}
 
 		readonly JniPeerMembers                             Members;
-		readonly Dictionary<string, JniStaticFieldID>       StaticFields  = new Dictionary<string, JniStaticFieldID>();
+
+		Dictionary<string, JniStaticFieldID>                StaticFields  = new Dictionary<string, JniStaticFieldID>();
 
 		public JniStaticFieldID GetFieldID (string encodedMember)
 		{
@@ -27,6 +28,16 @@ public JniStaticFieldID GetFieldID (string encodedMember)
 			}
 		}
 
+		internal void Dispose ()
+		{
+			if (StaticFields == null)
+				return;
+
+			foreach (var f in StaticFields.Values)
+				f.Dispose ();
+			StaticFields = null;
+		}
+
 		public object GetValue (string encodedMember)
 		{
 			var n   = JniPeerMembers.GetSignatureSeparatorIndex (encodedMember);

src/Java.Interop/Java.Interop/JniPeerStaticMethods.cs

@@ -11,7 +11,18 @@ internal JniPeerStaticMethods (JniPeerMembers members)
 		}
 
 		readonly JniPeerMembers                             Members;
-		readonly Dictionary<string, JniStaticMethodID>      StaticMethods   = new Dictionary<string, JniStaticMethodID>();
+
+		Dictionary<string, JniStaticMethodID>               StaticMethods   = new Dictionary<string, JniStaticMethodID>();
+
+		internal void Dispose ()
+		{
+			if (StaticMethods == null)
+				return;
+
+			foreach (var m in StaticMethods.Values)
+				m.Dispose ();
+			StaticMethods   = null;
+		}
 
 		public JniStaticMethodID GetMethodID (string encodedMember)
 		{