[Java.Interop] Add JniManagedPeerStates, IJavaPeerable.JniManagedPeerState

We want to retrofit Xamarin.Android to use Java.Interop. This in turn
requires that Java.Interop be able to do everything that
Xamarin.Android does, with the added complication that Java.Interop
will be used in Xamarin.Android before major parts are complete (e.g.
Activation [8c83f64] and Issue #11 [0]).

Fortunately we can look at how Xamarin.Android is architected to see
what we might need to provide in advance before it's actually used,
which leads us to Xamarin.Android's internal
Java.Interop.IJavaObjectEx interface:

	interface IJavaObjectEx {
		IntPtr      KeyHandle         {get; set;}
		bool        IsProxy           {get; set;}
		bool        NeedsActivation   {get; set;}
		IntPtr      ToLocalJniHandle ();
	}

Of those members, IJavaObjectEx.KeyHandle is already exposed as
IJavaPeerable.JniIdentityHashCode, and
IJavaObjectEx.ToLocalJniHandle() shouldn't be needed (it was added to
incorrectly address a multithreading-related bug).

That leaves IJavaObjectEx.IsProxy and IJavaObjectEx.NeedsActivation,
which are both involved in Java-side activation.

In the interest of avoiding API breaks in the future, we need to
support those constructs in Java.Interop *now*, even if they won't be
fully utilized until later.

Additionally, those names suck for "public" names -- what do they
*mean*, sans context? -- and, while things have been reasonably stable
here for the past several years, I'm not entirely certain that more
such states won't need to be added in the future, so we need to
support IJavaObjectEx.IsProxy and IJavaObjectEx.NeedsActivation
semantics in an "extensible" manner?

The solution? Yet another [Flags] enum!

	[Flags]
	public enum JniManagedPeerStates {
		None,
		Activatable         = (1 << 0),
		Replaceable         = (1 << 1),
	}

The use of a [Flags] enum allows us to add additional states in the
future, should we need to do so.

JniManagedPeerStates.Activatable is IJavaObjectEx.NeedsActivation, and
means that IJavaPeerable.PeerReference was set *before* the
constructor was invoked. (Setting IJavaPeerable.PeerReference before
the constructor executes is not yet done in Java.Interop.)
It means that a future "proper" constructor invocation is assumed to
be forthcoming, as during Java activation, IF the Java constructor
virtually invokes a method which is overridden in managed code, a
managed peer will need to be constructed so that the method override
can be invoked, and "later" the "real" constructor will be invoked.

	https://developer.xamarin.com/guides/android/under_the_hood/architecture/#Java_Activation

 1. `new Peer(...)` is invoked from Java.

 2. A super class constructor of NativePeer virtually invokes a method
    overridden by Peer in managed code.

 3. The Marshal Method is executed, which needs to delegate the method
    to *something*, and thus creates a new managed Peer instance
    through the activation constructor.
    This created managed peer instance will have the
    JniManagedPeerStates.Activatable state set.

 4. The managed override is executed and returns back to Java.

 5. Once all super constructors have finished, the Peer constructor
    executes com.xamarin.java_interop.ManagedPeer.runConstructor(),

 6. ManagedPeer.runConstructor() invokes the appropriate corresponding
    constructor on the instance created in (3). If the
    JniManagedPeerStates.Activatable state *isn't* set, then the
    ManagedPeer.runConstructor() call would be *ignored*.
    This also means that *two constructors* will be invoked on *one
    instrance*.

The JniManagedPeerStates.Activatable state needs to be set to sanely
prevent invoking constructors more than is intended on a given
peer instance.

JniManagedPeerStates.Replaceable is IJavaObjectEx.IsProxy, and means
that the Peer instance was created through the activation constructor.
It additionally means that if two managed instances are created around
the same Java instance, the non-Replaceable instance will be the one
returned by JniRuntime.JniValueManager.PeekObject().

Normally, JniManagedPeerStates.Replaceable shouldn't be needed, but
there is one environment where it is:

Android devices which are pre-Honeycomb (API-11). On those devices,
JNIEnv.AllocObject() cannot be used (8c4248b), so something very
similar but not quite like the above Activation case can happen when
constructing instances *from managed code*.

In "normal" use -- JNIEnv::AllocObject() works! -- managed
construction is:

 1. `new Peer()` invokes managed constructor.

 2. Managed constructor calls
    JniPeerMembers.InstanceMethods.StartCreateInstance(), which uses
    JNIEnv::AllocObject() to create a Java instance *without executing
    the Java instance constructor*.

 3. JniRuntime.JniValueManager.Construct() adds the Java instance from
    (2) to the mapping table.

 4. The constructor calls
    JniPeerMembers.InstanceMethods.FinishCreateInstance(), which
    invokes the Java instance constructor, and if a Java instance
    constructor virtually invokes a method overridden in managed code,
    the marshal method will find the instance created in (1).

When JNIEnv::AllocObject() doesn't work, the above falls down:

 1. `new Peer()` invokes managed constructor.

 2. Managed constructor calls
    JniPeerMembers.InstanceMethods.StartCreateInstance(), which uses
    JNIEnv::NewObject() to create a Java instance *and executes
    the Java instance constructor*.

 3. If a Java constructor virtually invokes a method overridden in
    managed code, the marshal method will be invoked and won't find an
    already-existing peer for the Java instance, and thus will create
    one via JniRuntime.JniValueManager.CreatePeer().

 4. JniRuntime.JniValueManager.CreatePeer() will set the
    JniManagedPeerStates.Replaceable state on the instance created in
    (3).

    Note: at this point there are *two* managed peers which want to
    "own" the Java instance: the instance created in (1) which is
    still being constructed (!), and the instance created in (3).

 5. The overriding managed method returns control to the Java
    constructor, which eventually completes execution and returns
    execution to the Peer managed constructor in (1).

 6. The Peer constructor invokes
    JniRuntime.JniValueManager.Construct() to add the instance from
    (1) to the instance mapping table, a mapping which *already
    exists* because of (3).

There needs to be a way for (6) to replace the mapping created in (3)
with the Peer instance in (1), and JniManagedPeerStates.Replaceable is
how that is tracked.

(Aside: supporting platforms that have broken JNIEnv::AllocObject()
implementations is a GIANT PAIN IN THE ASS.)

To use JniManagedPeerStates, update IJavaPeerable to add the following
members:

  partial interface IJavaPeerable {
    JniManagedPeerStates     JniManagedPeerState {get;}
    void                     SetJniManagedPeerState (JniManagedPeerStates value);
  }

IJavaPeerable.JniManagedPeerState is the current state of the managed
peer. IJavaPeerable.SetJniManagedPeerState() allows updating the
current state of the managed peer, permitting the state to be tracked
cross activation constructor calls.

[0]: dotnet/java-interop#11

Author: jonpryor

src/Java.Interop/Documentation/Java.Interop/IJavaPeerable.xml

@@ -195,6 +195,19 @@ partial class ExampleBinding : IJavaPeerable {
     </remarks>
     <seealso cref="M:Java.Interop.IJavaPeerable.SetJniIdentityHashCode" />
   </member>>
+  <member name="P:JniManagedPeerState">
+    <summary>State of the managed peer.</summary>
+    <remarks>
+      <para>
+        A <see cref="T:Java.Interop.JniManagedPeerStates" /> value providing
+        the state of the current Managed Peer instance.
+        <format type="text/html">
+          (<a href="https://developer.xamarin.com/guides/android/advanced_topics/garbage_collection/#Cross-VM_Object_Collections"
+          >Definitions</a>)
+        </format>
+      </para>
+    </remarks>
+  </member>
   <member name="P:JniPeerMembers">
     <summary>
       Member access and invocation support.
@@ -285,6 +298,20 @@ partial class ExampleBinding {
     </remarks>
     <seealso cref="P:Java.Interop.IJavaPeerable.JniIdentityHashCode" />
   </member>
+  <member name="P:SetJniManagedPeerState">
+    <summary>Set the state of the managed peer.</summary>
+    <param name="value">
+      A <see cref="T:Java.Interop.JniManagedPeerStates" /> value providing
+      the state of the current Managed Peer instance.
+    </param>
+    <remarks>
+      <para>
+        This should only be called from a
+        <see cref="T:Java.Interop.JniRuntime+JniValueManager" /> instance.
+      </para>
+    </remarks>
+    <seealso cref="P:Java.Interop.IJavaPeerable.JniManagedPeerState" />
+  </member>
   <member name="M:SetPeerReference">
     <summary>Set the value returned by <c>PeerReference</c>.</summary>
     <param name="value">

src/Java.Interop/Documentation/Java.Interop/JniManagedPeerStates.xml

@@ -0,0 +1,34 @@
+<?xml version="1.0"?>
+<docs>
+  <member name="T:JniManagedPeerStates">
+    <summary>
+      Managed peer states.
+    </summary>
+  </member>
+  <member name="F:Activatable">
+    <summary>
+      Managed peer is eligeable for activation.
+    </summary>
+    <remarks>
+      <para>
+        A managed peer is <i>activatable</i> if it the native peer was
+        constructed first and a forthcoming constructor invocation is
+        assumed to be forthcoming.
+      </para>
+    </remarks>
+  </member>
+  <member name="F:Replaceable">
+    <summary>
+      Managed peer is replaceable.
+    </summary>
+    <remarks>
+      <para>
+        A managed peer is <i>replaceable</i> if it was created through
+        <see cref="M:Java.Interop.JniRuntime+JniValueManager.CreatePeer" />.
+        Being "replaceable" means that if another peer instance is created
+        around the same Java instance, the newly created instance will replace
+        the replaceable peer in the Java-to-managed instance mapping.
+      </para>
+    </remarks>
+  </member>
+</docs>

src/Java.Interop/Java.Interop.csproj

@@ -76,6 +76,7 @@
     <Compile Include="Java.Interop\JniEnvironment.Strings.cs" />
     <Compile Include="Java.Interop\JniEnvironment.Types.cs" />
     <Compile Include="Java.Interop\JniFieldInfo.cs" />
+    <Compile Include="Java.Interop\JniManagedPeerStates.cs" />
     <Compile Include="Java.Interop\JniMethodInfo.cs" />
     <Compile Include="Java.Interop\JniObjectReference.cs" />
     <Compile Include="Java.Interop\JniPeerMembers.cs" />
@@ -184,5 +185,6 @@
       <LastGenOutput>JniPeerMembers.JniMethods.cs</LastGenOutput>
     </None>
     <None Include="Documentation\Java.Interop\IJavaPeerable.xml" />
+    <None Include="Documentation\Java.Interop\JniManagedPeerStates.xml" />
   </ItemGroup>
 </Project>

src/Java.Interop/Java.Interop/IJavaPeerable.cs

@@ -19,6 +19,12 @@ public interface IJavaPeerable : IDisposable
 		/// <include file="../Documentation/Java.Interop/IJavaPeerable.xml" path="/docs/member[@name='P:JniPeerMembers']/*" />
 		JniPeerMembers          JniPeerMembers {get;}
 
+		/// <include file="../Documentation/Java.Interop/IJavaPeerable.xml" path="/docs/member[@name='P:JniManagedPeerState']/*" />
+		JniManagedPeerStates    JniManagedPeerState {get;}
+
+		/// <include file="../Documentation/Java.Interop/IJavaPeerable.xml" path="/docs/member[@name='M:SetJniManagedPeerState']/*" />
+		void                    SetJniManagedPeerState (JniManagedPeerStates value);
+
 		// Lifetime management
 		/// <include file="../Documentation/Java.Interop/IJavaPeerable.xml" path="/docs/member[@name='M:UnregisterFromRuntime']/*" />
 		void    UnregisterFromRuntime ();

src/Java.Interop/Java.Interop/JavaException.cs

@@ -11,6 +11,8 @@ unsafe public class JavaException : Exception, IJavaPeerable
 		int     identity;
 		string  javaStackTrace;
 
+		JniManagedPeerStates     state;
+
 #if FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES
 		JniObjectReference  reference;
 #endif  // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES
@@ -232,6 +234,10 @@ unsafe string _GetJavaStack (JniObjectReference handle)
 			}
 		}
 
+		JniManagedPeerStates IJavaPeerable.JniManagedPeerState {
+			get {return state;}
+		}
+
 		void IJavaPeerable.Disposed ()
 		{
 			Dispose (disposing: true);
@@ -247,6 +253,11 @@ void IJavaPeerable.SetJniIdentityHashCode (int value)
 			identity    = value;
 		}
 
+		void IJavaPeerable.SetJniManagedPeerState (JniManagedPeerStates value)
+		{
+			state   = value;
+		}
+
 		void IJavaPeerable.SetPeerReference (JniObjectReference reference)
 		{
 			SetPeerReference (ref reference, JniObjectReferenceOptions.Copy);

src/Java.Interop/Java.Interop/JavaObject.cs

@@ -9,6 +9,8 @@ unsafe public class JavaObject : IJavaPeerable
 
 		int     keyHandle;
 
+		JniManagedPeerStates     state;
+
 #if FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES
 		JniObjectReference  reference;
 #endif  // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES
@@ -44,6 +46,10 @@ public int JniIdentityHashCode {
 			get {return keyHandle;}
 		}
 
+		public JniManagedPeerStates JniManagedPeerState {
+			get {return state;}
+		}
+
 		// Note: JniPeerMembers is invoked virtually from the constructor;
 		// it MUST be valid before the derived constructor executes!
 		// The pattern MUST be followed.
@@ -136,6 +142,10 @@ public override unsafe string ToString ()
 			return JniEnvironment.Strings.ToString (ref lref, JniObjectReferenceOptions.CopyAndDispose);
 		}
 
+		JniManagedPeerStates IJavaPeerable.JniManagedPeerState {
+			get {return state;}
+		}
+
 		void IJavaPeerable.Disposed ()
 		{
 			Dispose (disposing: true);
@@ -151,6 +161,11 @@ void IJavaPeerable.SetJniIdentityHashCode (int value)
 			keyHandle   = value;
 		}
 
+		void IJavaPeerable.SetJniManagedPeerState (JniManagedPeerStates value)
+		{
+			state   = value;
+		}
+
 		void IJavaPeerable.SetPeerReference (JniObjectReference reference)
 		{
 			SetPeerReference (ref reference, JniObjectReferenceOptions.Copy);

src/Java.Interop/Java.Interop/JniManagedPeerStates.cs

@@ -0,0 +1,14 @@
+using System;
+
+namespace Java.Interop
+{
+	/// <include file="../Documentation/Java.Interop/JniManagedPeerStates.xml" path="/docs/member[@name='T:JniManagedPeerStates']/*" />
+	[Flags]
+	public enum JniManagedPeerStates {
+		None,
+		/// <include file="../Documentation/Java.Interop/JniManagedPeerStates.xml" path="/docs/member[@name='F:Activatable']/*" />
+		Activatable         = (1 << 0),
+		/// <include file="../Documentation/Java.Interop/JniManagedPeerStates.xml" path="/docs/member[@name='F:Replaceable']/*" />
+		Replaceable         = (1 << 1),
+	}
+}

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

@@ -65,6 +65,14 @@ public void Construct (IJavaPeerable value, ref JniObjectReference reference, Jn
 				if (value == null)
 					throw new ArgumentNullException (nameof (value));
 
+				var activationRef   = value.PeerReference;
+				if (activationRef.IsValid) {
+					value.SetJniManagedPeerState (value.JniManagedPeerState | JniManagedPeerStates.Activatable);
+					JniObjectReference.Dispose (ref reference, options);
+					reference   = activationRef;
+					options     = JniObjectReferenceOptions.Copy;
+				}
+
 				if (!reference.IsValid)
 					throw new ArgumentException ("handle is invalid.", nameof (reference));
 
@@ -229,7 +237,9 @@ public virtual IJavaPeerable CreatePeer (ref JniObjectReference reference, JniOb
 					transfer,
 				};
 				try {
-					return (IJavaPeerable) ctor.Invoke (acts);
+					var peer    = (IJavaPeerable) ctor.Invoke (acts);
+					peer.SetJniManagedPeerState (peer.JniManagedPeerState | JniManagedPeerStates.Replaceable);
+					return peer;
 				} finally {
 					reference   = (JniObjectReference) acts [0];
 				}

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

@@ -109,7 +109,7 @@ public override void Add (IJavaPeerable value)
 			lock (RegisteredInstances) {
 				WeakReference<IJavaPeerable>    existing;
 				IJavaPeerable     target;
-				if (RegisteredInstances.TryGetValue (key, out existing) && existing.TryGetTarget (out target))
+				if (RegisteredInstances.TryGetValue (key, out existing) && existing.TryGetTarget (out target) && !Replaceable (target))
 					Runtime.ObjectReferenceManager.WriteGlobalReferenceLine (
 							"Warning: Not registering PeerReference={0} IdentityHashCode=0x{1} Instance={2} Instance.Type={3} Java.Type={4}; " +
 							"keeping previously registered PeerReference={5} Instance={6} Instance.Type={7} Java.Type={8}.",
@@ -127,6 +127,13 @@ public override void Add (IJavaPeerable value)
 			}
 		}
 
+		static bool Replaceable (IJavaPeerable peer)
+		{
+			if (peer == null)
+				return true;
+			return (peer.JniManagedPeerState & JniManagedPeerStates.Replaceable) == JniManagedPeerStates.Replaceable;
+		}
+
 		public override void Remove (IJavaPeerable value)
 		{
 			int key = value.JniIdentityHashCode;

xa-gendarme-ignore.txt

@@ -109,6 +109,7 @@ R: Gendarme.Rules.Design.FlagsShouldNotDefineAZeroValueRule
 # > **AVOID** using flag enum values of zero unless the value represents "all flags are cleared" and is named appropriately, as prescribed by the next guideline
 # "Invalid" implies "all flags are cleared" to *me*...
 # > **DO** name the zero value of flag enums None. For a flag enum, the value must always mean "all flags are cleared."
+T: Java.Interop.JniManagedPeerStates
 T: Java.Interop.JniObjectReferenceOptions