Bump to xamarin/Java.Interop/d16-8@a8079616

Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1193891
Fixes: #4989

Changes: dotnet/java-interop@b4c2a67...a807961

  * dotnet/java-interop@a8079616: [Java.Interop] Dispose, *then* Remove (#708)

~~~

Also cherry-picks d4e0738, which *should* have been done as part of
commit 62a5709, but was overlooked:

What happens if you dispose an instance *while disposing the instance*?

	// C#
	public class MyDisposableObject : Java.Lang.Object {
	    bool _isDisposed;

	    protected override void Dispose (bool disposing)
	    {
	        if (_isDisposed) {
	            return;
	        }
	        _isDisposed = true;
	        if (this.Handle != IntPtr.Zero)
	            this.Dispose ();
	        base.Dispose (disposing);
	    }
	}

	// …
	void MyTestMethod ()
	{
	    var value = new MyDisposableObject ();
	    value.Dispose ();
	    value.Dispose ();
	}

Here, `MyDisposableObject.Dispose(bool)` calls `Object.Dispose()`
when `Object.Handle` is valid.  This "feels" admittedly unusual, but
`IDisposable.Dispose()` is *supposed to be* Idempotent: it can be
called multiple times with no ill effects.  Shouldn't this be the same?

Unfortunately, it *isn't* the same; it crashes, hard:

	  =================================================================
	        Native stacktrace:
	  =================================================================
	        0x10245bc49 - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : mono_dump_native_crash_info
	        0x1023f3d35 - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : mono_handle_native_crash
	        0x10245b20f - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : sigabrt_signal_handler
	        0x7fff6983d5fd - /usr/lib/system/libsystem_platform.dylib : _sigtramp
	        0x700009b716b0 - Unknown
	        0x7fff69713808 - /usr/lib/system/libsystem_c.dylib : abort
	        0x10658377a - …/lib/server/libjvm.dylib : _ZN2os5abortEbPvPKv
	        0x10637ea5f - …/lib/server/libjvm.dylib : _ZN8jniCheck15validate_handleEP10JavaThreadP8_jobject
	        0x10637eb08 - …/lib/server/libjvm.dylib : _ZN8jniCheck15validate_objectEP10JavaThreadP8_jobject
	        0x1063818a1 - …/lib/server/libjvm.dylib : checked_jni_GetObjectClass
	        0x10594cfa8 - …/Java.Interop/bin/TestDebug/libjava-interop.dylib : java_interop_jnienv_get_object_class
	        0x107a1c636 - Unknown
	        0x107aa2c73 - Unknown

	…
	  =================================================================
	        Managed Stacktrace:
	  =================================================================
	          at <unknown> <0xffffffff>
	          at Java.Interop.NativeMethods:java_interop_jnienv_get_object_class <0x000a5>
	          at Types:GetObjectClass <0x0010a>
	          at Types:GetJniTypeNameFromInstance <0x000a2>
	          at JniValueManager:DisposePeer <0x002c2>
	          at JniValueManager:DisposePeer <0x000f2>
	          at Java.Interop.JavaObject:Dispose <0x000ea>
	          at Java.InteropTests.JavaObjectTest:NestedDisposeInvocations <0x00072>
	          at System.Object:runtime_invoke_void__this__ <0x000b0>
	          at <unknown> <0xffffffff>
	          at System.Reflection.RuntimeMethodInfo:InternalInvoke <0x000b8>
	          at System.Reflection.RuntimeMethodInfo:Invoke <0x00152>
	          at System.Reflection.MethodBase:Invoke <0x00058>

Ouch.

The cause of the crash isn't the "successive" `.Dispose()` invocations
within `MyTestMethod()`, but rather the *nested* `.Dispose()`
invocation within `MyDisposableObject.Dispose()`.

Runtime execution is thus:

 1. `JavaObject.Dispose()`
 2. `JniRuntime.JniValueManager.DisposePeer(this)`
 3. `var h = this.PeerReference`
 4. `JniRuntime.JniValueManager.DisposePeer(h, this)`
 5. `JavaObject.Disposed()`
 6. `MyDisposableObject.Dispose(disposing:true)`
 7. `JavaObject.Dispose()`              // back to (1)?
 8. `JniRuntime.JniValueManager.DisposePeer(this)`
 9. `var h = this.PeerReference`        // *second* ref to `h`
10. `JniRuntime.JniValueManager.DisposePeer(h, this)`, which passes
    `h` to e.g. `JniEnvironment.Types.GetJniTypeNameFromInstance()`,
    thus requiring that `h` be a valid JNI reference, and also calls
    `JniObjectReference.Dispose()`, invalidating `h`.
11. "Unwinding" (4), call `Types.GetJniTypeNameFromInstance()` with
    the `h` from (3).

The problem *appears to be* the recursive `Dispose()` invocation on
(7), but the *actual* problem is step (3): by holding a cached/"old"
value of `this.PeerReference` -- and then later using that *same* value
in `JniRuntime.JniValueManager.DisposePeer()`-- when the nested
`JavaObject.Dispose()` invocation continues execution,
`this.PeerReference` will be *invalidated*, but the copy of the handle
from (3) will still be used!  This causes the JVM to very loudly abort.

The fix is to defer the "caching" present in (3): instead of storing
the `PeerReference` value "immediately" -- and disposing the same
value "later" -- don't store the value until *after*
`IJavaPeerable.Disposed()` is called.  This gives the
`Dispose(disposing:true)` method a chance to execute *before*
retaining any cached references to `PeerReference` -- which may in
turn invalidate `PeerReference`! -- thus ensuring that we only attempt
to dispose *valid* JNI handles.

Commit dotnet/java-interop@4141d848 contains fixes to
`JniRuntime.JniValueManager.DisposePeer()`, and add appropriate tests.

Author: jonpryor

external/Java.Interop

@@ -558,10 +558,12 @@ public override void RemovePeer (IJavaPeerable value)
 		{
 			if (value == null)
 				throw new ArgumentNullException (nameof (value));
-			if (!value.PeerReference.IsValid)
-				throw new ArgumentException ("Must have a valid JNI object reference!", nameof (value));
 
 			var reference       = value.PeerReference;
+			if (!reference.IsValid) {
+				// Likely an idempotent DIspose(); ignore.
+				return;
+			}
 			var hash            = JNIEnv.IdentityHash! (reference.Handle);
 
 			RemovePeer (value, hash);

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

@@ -67,6 +67,14 @@ public void JnienvCreateInstance_RegistersMultipleInstances ()
 				Assert.AreSame (adapter, registered);
 			}
 		}
+
+		[Test]
+		public void NestedDisposeInvocations ()
+		{
+			var value = new MyDisposableObject ();
+			value.Dispose ();
+			value.Dispose ();
+		}
 	}
 
 	/*
@@ -157,4 +165,23 @@ public override void SetSelection (int position)
 			throw new NotImplementedException();
 		}
 	}
+
+	public class MyDisposableObject : Java.Lang.Object
+	{
+		bool _isDisposed;
+		public MyDisposableObject ()
+		{
+		}
+
+		protected override void Dispose (bool disposing)
+		{
+			if (_isDisposed) {
+				return;
+			}
+			_isDisposed = true;
+			if (this.Handle != IntPtr.Zero)
+				this.Dispose ();
+			base.Dispose (disposing);
+		}
+	}
 }