[Java.Interop] JNIEnv::NewObject and Replaceable instances (#1323)

jonpryor · web-flow · commit d3d3a1bf8200 · 2025-04-15T10:50:50.000-04:00
Context: 3043d89 Context: dec35f5 Context: dotnet/android#9862 Context: dotnet/android#9862 (comment) Context: dotnet/android#10004 Context: https://github.com/xamarin/monodroid/commit/326509e56d4e582c53bbe5dfe6d5c741a27f1af5 Context: https://github.com/xamarin/monodroid/commit/940136ebf1318a7c57a855e2728ce2703c0240af Ever get the feeling that everything is inextricably related? JNI has two pattens for create an instance of a Java type: 1. [`JNIEnv::NewObject(jclass clazz, jmethodID methodID, const jvalue* args)`][0] 2. [`JNIEnv::AllocObject(jclass clazz)`][1] + [`JNIEnv::CallNonvirtualVoidMethod(jobject obj, jclass clazz, jmethodID methodID, const jvalue* args)`][2] In both patterns: * `clazz` is the `java.lang.Class` of the type to create. * `methodID` is the constructor to execute * `args` are the constructor arguments. In .NET terms: * `JNIEnv::NewObject()` is equivalent to using `System.Reflection.ConstructorInfo.Invoke(object?[]?)`, while * `JNIEnv::AllocObject()` + `JNIEnv::CallNonvirtualVoidMethod()` is equivalent to using `System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(Type)` + `System.Reflection.MethodBase.Invoke(object?, object?[]?)`. Why prefer one over the other? When hand-writing your JNI code, `JNIEnv::NewObject()` is easier. This is less of a concern when a code generator is used. The *real* reason to *avoid* `JNIEnv::NewObject()` whenever possible is the [Java Activation][3] scenario, summarized as the "are you sure you want to do this?" [^1] scenario of invoking a virtual method from the constructor: class Base { public Base() { VirtualMethod(); } public virtual void VirtualMethod() {} } class Derived : Base { public override void VirtualMethod() {} } Java and C# are identical here: when a constructor invokes a virtual method, the most derived method implementation is used, which will occur *before* the constructor of the derived type has *started* execution. (With lots of quibbling about field initializers…) Thus, assume you have a Java `CallVirtualFromConstructorBase` type, which has its constructor Do The Wrong Thing™ and invoke a virtual method from the constructor, and that method is overridden in C#? // Java public class CallVirtualFromConstructorBase { public CallVirtualFromConstructorBase(int value) { calledFromConstructor(value); } public void calledFromConstructor(int value) { } } // C# public class CallVirtualFromConstructorDerived : CallVirtualFromConstructorBase { public CallVirtualFromConstructorDerived(int value) : base (value) { } public override void CalledFromConstructor(int value) { } } What happens with: var p = new CallVirtualFromConstructorDerived(42); The answer depends on whether or not `JNIEnv::NewObject()` is used. If `JNIEnv::NewObject()` is *not* used (the default!) 1. `CallVirtualFromConstructorDerived(int)` constructor begins execution, immediately calls `base(value)`. 2. `CallVirtualFromConstructorBase(int)` constructor runs, uses `JNIEnv::AllocObject()` to *create* (but not construct!) Java `CallVirtualFromConstructorDerived` instance. 3. `JavaObject.Construct(ref JniObjectReference, JniObjectReferenceOptions)` invoked, creating a mapping between the C# instance created in (1) and the Java instance created in (2). 4. `CallVirtualFromConstructorBase(int)` C# constructor calls `JniPeerMembers.InstanceMethods.FinishGenericCreateInstance()`, which eventually invokes `JNIEnv::CallNonvirtualVoidMethod()` with the Java `CallVirtualFromConstructorDerived(int)` ctor. 5. Java `CallVirtualFromConstructorDerived(int)` constructor invokes Java `CallVirtualFromConstructorBase(int)` constructor, which invokes `CallVirtualFromConstructorDerived.calledFromConstructor()`. 6. Marshal method (356485e) for `CallVirtualFromConstructorBase.CalledFromConstructor()` invoked, *immediately* calls `JniRuntime.JniValueManager.GetPeer()` (e288589) to obtain an instance upon which to invoke `.CalledFromConstructor()`, finds the instance mapping from (3), invokes `CallVirtualFromConstructorDerived.CalledFromConstructor()` override. 7. Marshal Method for `CalledFromConstructor()` returns, Java `CallVirtualFromConstructorBase(int)` constructor finishes, Java `CallVirtualFromConstructorDerived(int)` constructor finishes, `JNIEnv::CallNonvirtualVoidMethod()` finishes. 8. `CallVirtualFromConstructorDerived` instance finishes construction. If `JNIEnv::NewObject()` is used: 1. `CallVirtualFromConstructorDerived(int)` constructor begins execution, immediately calls `base(value)`. Note that this is the first created `CallVirtualFromConstructorDerived` instance, but it hasn't been registered yet. 2. `CallVirtualFromConstructorBase(int)` constructor runs, uses `JNIEnv::NewObject()` to construct Java `CallVirtualFromConstructorDerived` instance. 3. `JNIEnv::NewObject()` invokes Java `CallVirtualFromConstructorDerived(int)` constructor, which invokes `CallVirtualFromConstructorBase(int)` constructor, which invokes `CallVirtualFromConstructorDerived.calledFromConstructor()`. 4. Marshal method (356485e) for `CallVirtualFromConstructorBase.CalledFromConstructor()` invoked, *immediately* calls `JniRuntime.JniValueManager.GetPeer()` (e288589) to obtain an instance upon which to invoke `.CalledFromConstructor()`. Here is where things go "off the rails" compared to the `JNIEnv::AllocObject()` code path: There is no such instance -- we're still in the middle of constructing it! -- so we look for an "activation constructor". 5. `CallVirtualFromConstructorDerived(ref JniObjectReference, JniObjectReferenceOptions)` activation constructor executed. This is the *second* `CallVirtualFromConstructorDerived` instance created, and registers a mapping from the Java instance that we started constructing in (3) to what we'll call the "activation intermediary". The activation intermediary instance is marked as "Replaceable". 6. `CallVirtualFromConstructorDerived.CalledFromConstructor()` method override invoked on the activation intermediary. 7. Marshal Method for `CalledFromConstructor()` returns, Java `CallVirtualFromConstructorBase(int)` constructor finishes, Java `CallVirtualFromConstructorDerived(int)` constructor finishes, `JNIEnv::NewObject()` returns instance. 8. C# `CallVirtualFromConstructorBase(int)` constructor calls `JavaObject.Construct(ref JniObjectReference, JniObjectReferenceOptions)`, to create a mapping between (3) and (1). In .NET for Android, this causes the C# instance created in (1) to *replace* the C# instance created in (5), which allows "Replaceable" instance to be replaced. In dotnet/java-interop, this replacement *didn't* happen, which meant that `ValueManager.PeekPeer(p.PeerReference)` would return the activation intermediary, *not* `p`, which confuses everyone. 9. `CallVirtualFromConstructorDerived` instance finishes construction. For awhile, dotnet/java-interop did not fully support this scenario around `JNIEnv::NewObject()`. Additionally, support for using `JNIEnv::NewObject()` as part of `JniPeerMembers.JniInstanceMethods.StartCreateInstance()` was *removed* in dec35f5. Which brings us to dotnet/android#9862: where there is an observed "race condition" around `Android.App.Application` subclass creation. *Two* instances of `AndroidApp` were created, one from the "normal" app startup: at crc647fae2f69c19dcd0d.AndroidApp.n_onCreate(Native Method) at crc647fae2f69c19dcd0d.AndroidApp.onCreate(AndroidApp.java:25) at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1316) and another from an `androidx.work.WorkerFactory`: at mono.android.TypeManager.n_activate(Native Method) at mono.android.TypeManager.Activate(TypeManager.java:7) at crc647fae2f69c19dcd0d.SyncWorker.<init>(SyncWorker.java:23) at java.lang.reflect.Constructor.newInstance0(Native Method) at java.lang.reflect.Constructor.newInstance(Constructor.java:343) at androidx.work.WorkerFactory.createWorkerWithDefaultFallback(WorkerFactory.java:95) However, what was odd about this "race condition" was that the *second* instance created would reliably win! Further investigation suggested that this was less of a "race condition" and more a bug in `AndroidValueManager`, wherein when "Replaceable" instances were created, an existing instance would *always* be replaced, even if the new instance was also Replaceable! This feels bananas; yes, Replaceable should be replaceable, but the expectation was that it would be replaced by *non*-Replaceable instances, not just any instance that came along later. Update `JniRuntimeJniValueManagerContract` to add a new `CreatePeer_ReplaceableDoesNotReplace()` test to codify the desired semantic that Replaceable instances do not replace Replaceable instances. Surprisingly, this new test did not fail on java-interop, as `ManagedValueManager.AddPeer()` bails early when `PeekPeer()` finds a value, while `AndroidValueManager.AddPeer()` does not bail early. An obvious fix for `CreatePeer_ReplaceableDoesNotReplace()` within dotnet/android would be to adopt the "`AddPeer()` calls `PeekPeer()`" logic from java-interop. The problem is that doing so breaks [`ObjectTest.JnienvCreateInstance_RegistersMultipleInstances()`][5], as seen in dotnet/android#10004! `JnienvCreateInstance_RegistersMultipleInstances()` in turn fails when `PeekPeer()` is used because follows the `JNIEnv::NewObject()` [construction codepath][6]! public CreateInstance_OverrideAbsListView_Adapter (Context context) : base ( JNIEnv.CreateInstance ( JcwType, "(Landroid/content/Context;)V", new JValue (context)), JniHandleOwnership.TransferLocalRef) { AdapterValue = new ArrayAdapter (context, 0); } as `JNIEnv.CreateInstance()` uses `JNIEnv.NewObject()`. We thus have a conundrum: how do we fix *both* `CreatePeer_ReplaceableDoesNotReplace()` *and* `JnienvCreateInstance_RegistersMultipleInstances()`? The answer is to add proper support for the `JNIEnv::NewObject()` construction scenario to dotnet/java-interop, which in turn requires "lowering" the setting of `.Replaceable`. Previously, we would set `.Replaceable` *after* the activation constructor was invoked: // dotnet/android TypeManager.CreateInstance(), paraphrasing var result = CreateProxy (type, handle, transfer); result.SetJniManagedPeerState (JniManagedPeerStates.Replaceable | JniManagedPeerStates.Activatable); return result; This is *too late*, as during execution of the activation constructor, the instance thinks it *isn't* replaceable, and thus creation of a new instance via the activation constructor will replace an already existing replaceable instance; it's not until *after* the constructor finished executing that we'd set `.Replaceable`. To fix this, update `JniRuntime.JniValueManager.TryCreatePeerInstance()` to first create an *uninitialized* instance, set `.Replaceable`, and *then* invoke the activation constructor. This allows `JniRuntime.JniValueManager.AddPeer()` to check to see if the new value is also replaceable, and ignore the replacement if appropriate. This in turn requires replacing: partial class /* JniRuntime. */ JniValueManager { protected virtual IJavaPeerable? TryCreatePeer () ref JniObjectReference reference, JniObjectReferenceOptions options, Type type); } with: partial class /* JniRuntime. */ JniValueManager { protected virtual bool TryConstructPeer () IJavaPeerable self, ref JniObjectReference reference, JniObjectReferenceOptions options, Type type); } This is fine because we haven't shipped `TryCreatePeer()` in a stable release yet. [^1]: See also [Framework Design Guidelines > Constructor Design][4]: > ❌ AVOID calling virtual members on an object inside its constructor. > Calling a virtual member will cause the most derived override to be > called, even if the constructor of the most derived type has not > been fully run yet. [0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#NewObject [1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#AllocObject [2]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#CallNonvirtual_type_Method_routines [3]: https://learn.microsoft.com/en-us/previous-versions/xamarin/android/internals/architecture#java-activation [4]: https://learn.microsoft.com/dotnet/standard/design-guidelines/constructor [5]: https://github.com/dotnet/android/blob/9ad492a42b384519a8b1f1987adae82335536d9c/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs#L68-L79 [6]: https://github.com/dotnet/android/blob/9ad492a42b384519a8b1f1987adae82335536d9c/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs#L151-L160
diff --git a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs
@@ -401,15 +401,36 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 					Type type)
 			{
 				type    = Runtime.TypeManager.GetInvokerType (type) ?? type;
-				return TryCreatePeer (ref reference, options, type);
+
+				var self        = GetUninitializedObject (type);
+				var constructed = false;
+				try {
+					constructed = TryConstructPeer (self, ref reference, options, type);
+				} finally {
+					if (!constructed) {
+						GC.SuppressFinalize (self);
+						self    = null;
+					}
+				}
+				return self;
+
+				static IJavaPeerable GetUninitializedObject (
+						[DynamicallyAccessedMembers (Constructors)]
+						Type type)
+				{
+					var v   = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (type);
+					v.SetJniManagedPeerState (JniManagedPeerStates.Replaceable | JniManagedPeerStates.Activatable);
+					return v;
+				}
 			}
 
 			const               BindingFlags    ActivationConstructorBindingFlags   = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;
 			static  readonly    Type            ByRefJniObjectReference = typeof (JniObjectReference).MakeByRefType ();
 			static  readonly    Type[]          JIConstructorSignature  = new Type [] { ByRefJniObjectReference, typeof (JniObjectReferenceOptions) };
 
 
-			protected virtual IJavaPeerable? TryCreatePeer (
+			protected virtual bool TryConstructPeer (
+					IJavaPeerable self,
 					ref JniObjectReference reference,
 					JniObjectReferenceOptions options,
 					[DynamicallyAccessedMembers (Constructors)]
@@ -421,11 +442,11 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 						reference,
 						options,
 					};
-					var p       = (IJavaPeerable) c.Invoke (args);
+					c.Invoke (self, args);
 					reference   = (JniObjectReference) args [0];
-					return p;
+					return true;
 				}
-				return null;
+				return false;
 			}
 
 			public object? CreateValue (
diff --git a/src/Java.Interop/PublicAPI.Unshipped.txt b/src/Java.Interop/PublicAPI.Unshipped.txt
@@ -4,7 +4,7 @@ static Java.Interop.JniEnvironment.EndMarshalMethod(ref Java.Interop.JniTransiti
 virtual Java.Interop.JniRuntime.OnEnterMarshalMethod() -> void
 virtual Java.Interop.JniRuntime.OnUserUnhandledException(ref Java.Interop.JniTransition transition, System.Exception! e) -> void
 virtual Java.Interop.JniRuntime.JniTypeManager.GetInvokerTypeCore(System.Type! type) -> System.Type?
-virtual Java.Interop.JniRuntime.JniValueManager.TryCreatePeer(ref Java.Interop.JniObjectReference reference, Java.Interop.JniObjectReferenceOptions options, System.Type! type) -> Java.Interop.IJavaPeerable?
+virtual Java.Interop.JniRuntime.JniValueManager.TryConstructPeer(Java.Interop.IJavaPeerable! self, ref Java.Interop.JniObjectReference reference, Java.Interop.JniObjectReferenceOptions options, System.Type! type) -> bool
 Java.Interop.JavaException.JavaException(ref Java.Interop.JniObjectReference reference, Java.Interop.JniObjectReferenceOptions transfer, Java.Interop.JniObjectReference throwableOverride) -> void
 Java.Interop.JavaException.SetJavaStackTrace(Java.Interop.JniObjectReference peerReferenceOverride = default(Java.Interop.JniObjectReference)) -> void
 Java.Interop.JniRuntime.JniTypeManager.GetInvokerType(System.Type! type) -> System.Type?
diff --git a/src/Java.Runtime.Environment/Java.Interop/ManagedValueManager.cs b/src/Java.Runtime.Environment/Java.Interop/ManagedValueManager.cs
@@ -57,9 +57,6 @@ public override void AddPeer (IJavaPeerable value)
 			var r = value.PeerReference;
 			if (!r.IsValid)
 				throw new ObjectDisposedException (value.GetType ().FullName);
-			var o = PeekPeer (value.PeerReference);
-			if (o != null)
-				return;
 
 			if (r.Type != JniObjectReferenceType.Global) {
 				value.SetPeerReference (r.NewGlobalRef ());
@@ -80,7 +77,7 @@ public override void AddPeer (IJavaPeerable value)
 					var p   = peers [i];
 					if (!JniEnvironment.Types.IsSameObject (p.PeerReference, value.PeerReference))
 						continue;
-					if (Replaceable (p)) {
+					if (Replaceable (p, value)) {
 						peers [i] = value;
 					} else {
 						WarnNotReplacing (key, value, p);
@@ -91,11 +88,12 @@ public override void AddPeer (IJavaPeerable value)
 			}
 		}
 
-		static bool Replaceable (IJavaPeerable peer)
+		static bool Replaceable (IJavaPeerable peer, IJavaPeerable value)
 		{
 			if (peer == null)
 				return true;
-			return (peer.JniManagedPeerState & JniManagedPeerStates.Replaceable) == JniManagedPeerStates.Replaceable;
+			return peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable) &&
+				!value.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable);
 		}
 
 		void WarnNotReplacing (int key, IJavaPeerable ignoreValue, IJavaPeerable keepValue)
diff --git a/tests/Java.Interop-Tests/Java.Interop/CallVirtualFromConstructorBase.cs b/tests/Java.Interop-Tests/Java.Interop/CallVirtualFromConstructorBase.cs
@@ -1,4 +1,5 @@
 ﻿using System;
+using System.Runtime.CompilerServices;
 
 using Java.Interop;
 using Java.Interop.GenericMarshaler;
@@ -16,14 +17,31 @@ public override JniPeerMembers JniPeerMembers {
 		}
 
 		public unsafe CallVirtualFromConstructorBase (int value)
+			: this (value, useNewObject: false)
+		{
+		}
+
+		public unsafe CallVirtualFromConstructorBase (int value, bool useNewObject)
 			: this (ref *InvalidJniObjectReference, JniObjectReferenceOptions.None)
 		{
 			if (PeerReference.IsValid)
 				return;
 
-			var peer    = JniPeerMembers.InstanceMethods.StartGenericCreateInstance ("(I)V", GetType (), value);
+			const string __id = "(I)V";
+
+			if (useNewObject) {
+				var ctors   = JniPeerMembers.InstanceMethods.GetConstructorsForType (GetType ());
+				var init    = ctors.GetConstructor (__id);
+
+				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
+				__args [0] = new JniArgumentValue (value);
+				var lref = JniEnvironment.Object.NewObject (ctors.JniPeerType.PeerReference, init, __args);
+				Construct (ref lref, JniObjectReferenceOptions.CopyAndDispose);
+				return;
+			}
+			var peer    = JniPeerMembers.InstanceMethods.StartGenericCreateInstance (__id, GetType (), value);
 			Construct (ref peer, JniObjectReferenceOptions.CopyAndDispose);
-			JniPeerMembers.InstanceMethods.FinishGenericCreateInstance ("(I)V", this, value);
+			JniPeerMembers.InstanceMethods.FinishGenericCreateInstance (__id, this, value);
 		}
 
 		public CallVirtualFromConstructorBase (ref JniObjectReference reference, JniObjectReferenceOptions options)
diff --git a/tests/Java.Interop-Tests/Java.Interop/CallVirtualFromConstructorDerived.cs b/tests/Java.Interop-Tests/Java.Interop/CallVirtualFromConstructorDerived.cs
@@ -1,4 +1,7 @@
-﻿using System;
+﻿#nullable enable
+
+using System;
+using System.Runtime.CompilerServices;
 
 using Java.Interop;
 
@@ -24,21 +27,38 @@ public override JniPeerMembers JniPeerMembers {
 		public  bool    InvokedConstructor;
 
 		public CallVirtualFromConstructorDerived (int value)
-			: base (value)
+			: this (value, useNewObject: false)
+		{
+		}
+
+		public CallVirtualFromConstructorDerived (int value, bool useNewObject)
+			: base (value, useNewObject)
 		{
 			InvokedConstructor = true;
-			if (value != calledValue)
+
+			if (useNewObject && calledValue != 0) {
+				// calledValue was set on a *different* instance! So it's 0 here.
+				throw new ArgumentException (
+						string.Format ("value '{0}' doesn't match expected value '{1}'.", value, 0),
+						"value");
+			}
+			if (!useNewObject && value != calledValue)
 				throw new ArgumentException (
 						string.Format ("value '{0}' doesn't match expected value '{1}'.", value, calledValue),
 						"value");
 		}
 
 		public  bool    InvokedActivationConstructor;
 
+		public  static  CallVirtualFromConstructorDerived?  Intermediate_FromCalledFromConstructor;
+		public  static  CallVirtualFromConstructorDerived?  Intermediate_FromActivationConstructor;
+
 		public CallVirtualFromConstructorDerived (ref JniObjectReference reference, JniObjectReferenceOptions options)
 			: base (ref reference, options)
 		{
 			InvokedActivationConstructor    = true;
+
+			Intermediate_FromActivationConstructor  = this;
 		}
 
 		public bool Called;
@@ -47,14 +67,16 @@ public override void CalledFromConstructor (int value)
 		{
 			Called      = true;
 			calledValue = value;
+
+			Intermediate_FromCalledFromConstructor  = this;
 		}
 
 		public static unsafe CallVirtualFromConstructorDerived NewInstance (int value)
 		{
 			JniArgumentValue* args = stackalloc JniArgumentValue [1];
 			args [0]    = new JniArgumentValue (value);
 			var o       = _members.StaticMethods.InvokeObjectMethod ("newInstance.(I)Lnet/dot/jni/test/CallVirtualFromConstructorDerived;", args);
-			return JniEnvironment.Runtime.ValueManager.GetValue<CallVirtualFromConstructorDerived> (ref o, JniObjectReferenceOptions.CopyAndDispose);
+			return JniEnvironment.Runtime.ValueManager.GetValue<CallVirtualFromConstructorDerived> (ref o, JniObjectReferenceOptions.CopyAndDispose)!;
 		}
 
 		delegate void CalledFromConstructorMarshalMethod (IntPtr jnienv, IntPtr n_self, int value);
@@ -63,7 +85,7 @@ static void CalledFromConstructorHandler (IntPtr jnienv, IntPtr n_self, int valu
 			var envp = new JniTransition (jnienv);
 			try {
 				var r_self  = new JniObjectReference (n_self);
-				var self    = JniEnvironment.Runtime.ValueManager.GetValue<CallVirtualFromConstructorDerived>(ref r_self, JniObjectReferenceOptions.Copy);
+				var self    = JniEnvironment.Runtime.ValueManager.GetValue<CallVirtualFromConstructorDerived>(ref r_self, JniObjectReferenceOptions.Copy)!;
 				self.CalledFromConstructor (value);
 				self.DisposeUnlessReferenced ();
 			}
diff --git a/tests/Java.Interop-Tests/Java.Interop/InvokeVirtualFromConstructorTests.cs b/tests/Java.Interop-Tests/Java.Interop/InvokeVirtualFromConstructorTests.cs
@@ -1,4 +1,5 @@
 ﻿using System;
+using System.Runtime.CompilerServices;
 
 using Java.Interop;
 
@@ -7,25 +8,145 @@
 namespace Java.InteropTests {
 
 	[TestFixture]
+#if !__ANDROID__
+	// We want stability around the CallVirtualFromConstructorDerived static fields
+	[NonParallelizable]
+#endif  // !__ANDROID__
 	public class InvokeVirtualFromConstructorTests : JavaVMFixture
 	{
 		[Test]
-		public void InvokeVirtualFromConstructor ()
+		public void CreateManagedInstanceFirst_WithAllocObject ()
 		{
-			using (var t = new CallVirtualFromConstructorDerived (42)) {
-				Assert.IsTrue (t.Called);
-				Assert.IsNotNull (JniRuntime.CurrentRuntime.ValueManager.PeekValue (t.PeerReference));
-			}
+			CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor    = null;
+			CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor    = null;
+
+			using var t = new CallVirtualFromConstructorDerived (42);
+			Assert.IsTrue (
+					t.Called,
+					"CalledFromConstructor method override should have been called.");
+			Assert.IsFalse (
+					t.InvokedActivationConstructor,
+					"Activation Constructor should have been called, as calledFromConstructor() is invoked before ManagedPeer.construct().");
+			Assert.IsTrue (
+					t.InvokedConstructor,
+					"(int) constructor should have been called, via ManagedPeer.construct().");
+
+			var registered      = JniRuntime.CurrentRuntime.ValueManager.PeekValue (t.PeerReference);
+			var acIntermediate  = CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor;
+			var cfIntermediate  = CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor;
+
+			Assert.AreSame (t, registered,
+					"Expected t and registered to be the same instance; " +
+					$"t={RuntimeHelpers.GetHashCode (t).ToString ("x")}, " +
+					$"registered={RuntimeHelpers.GetHashCode (registered).ToString ("x")}");
+			Assert.IsNull (acIntermediate,
+					"Activation Constructor should not have been called, because of AllocObject semantics");
+			Assert.AreSame (t, cfIntermediate,
+					"Expected t and cfIntermediate to be the same instance; " +
+					$"t={RuntimeHelpers.GetHashCode (t).ToString ("x")}, " +
+					$"cfIntermediate={RuntimeHelpers.GetHashCode (cfIntermediate).ToString ("x")}");
+
+			Dispose (ref CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor);
+			Dispose (ref CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor);
+		}
+
+		static void Dispose<T> (ref T peer)
+			where T : class, IJavaPeerable
+		{
+			if (peer == null)
+				return;
+
+			peer.Dispose ();
+			peer = null;
 		}
 
 		[Test]
-		public void ActivationConstructor ()
+		public void CreateManagedInstanceFirst_WithNewObject ()
 		{
-			var t = CallVirtualFromConstructorDerived.NewInstance (42);
-			using (t) {
-				Assert.IsTrue (t.InvokedActivationConstructor);
-				Assert.IsTrue (t.InvokedConstructor);
-			}
+			CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor    = null;
+			CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor    = null;
+
+			using var t = new CallVirtualFromConstructorDerived (42, useNewObject: true);
+			Assert.IsFalse (
+					t.Called,
+					"CalledFromConstructor method override was called on a different instance.");
+			Assert.IsFalse (
+					t.InvokedActivationConstructor,
+					"Activation Constructor should not have been called, as calledFromConstructor() is invoked before ManagedPeer.construct().");
+			Assert.IsTrue (
+					t.InvokedConstructor,
+					"(int) constructor should have been called, via ManagedPeer.construct().");
+
+			var registered      = JniRuntime.CurrentRuntime.ValueManager.PeekValue (t.PeerReference);
+			var acIntermediate  = CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor;
+			var cfIntermediate  = CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor;
+
+			Assert.AreSame (t, registered,
+					"Expected t and registered to be the same instance; " +
+					$"t={RuntimeHelpers.GetHashCode (t).ToString ("x")}, " +
+					$"registered={RuntimeHelpers.GetHashCode (registered).ToString ("x")}");
+			Assert.IsNotNull (acIntermediate,
+					"Activation Constructor should have been called, because of NewObject");
+			Assert.IsTrue (
+					acIntermediate.Called,
+					"CalledFromConstructor method override should have been called on acIntermediate.");
+			Assert.IsTrue (
+					acIntermediate.InvokedActivationConstructor,
+					"Activation Constructor should have been called on intermediate instance, as calledFromConstructor() is invoked before ManagedPeer.construct().");
+			Assert.AreNotSame (t, acIntermediate,
+					"Expected t and registered to be different instances; " +
+					$"t={RuntimeHelpers.GetHashCode (t).ToString ("x")}, " +
+					$"acIntermediate={RuntimeHelpers.GetHashCode (acIntermediate).ToString ("x")}");
+			Assert.AreNotSame (t, cfIntermediate,
+					"Expected t and cfIntermediate to be different instances; " +
+					$"t={RuntimeHelpers.GetHashCode (t).ToString ("x")}, " +
+					$"cfIntermediate={RuntimeHelpers.GetHashCode (cfIntermediate).ToString ("x")}");
+			Assert.AreSame (acIntermediate, cfIntermediate,
+					"Expected acIntermediate and cfIntermediate to be the same instance; " +
+					$"acIntermediate={RuntimeHelpers.GetHashCode (acIntermediate).ToString ("x")}, " +
+					$"cfIntermediate={RuntimeHelpers.GetHashCode (cfIntermediate).ToString ("x")}");
+
+			Dispose (ref CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor);
+			Dispose (ref CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor);
+		}
+
+		[Test]
+		public void CreateJavaInstanceFirst ()
+		{
+			CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor    = null;
+			CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor    = null;
+
+			using var t = CallVirtualFromConstructorDerived.NewInstance (42);
+
+			Assert.IsTrue (
+					t.Called,
+					"CalledFromConstructor method override should have been called.");
+			Assert.IsTrue (
+					t.InvokedActivationConstructor,
+					"Activation Constructor should have been called, as calledFromConstructor() is invoked before ManagedPeer.construct().");
+			Assert.IsTrue (
+					t.InvokedConstructor,
+					"(int) constructor should have been called, via ManagedPeer.construct().");
+
+			var registered      = JniRuntime.CurrentRuntime.ValueManager.PeekValue (t.PeerReference);
+			var acIntermediate  = CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor;
+			var cfIntermediate  = CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor;
+
+			Assert.AreSame (t, registered,
+					"Expected t and registered to be the same instance; " +
+					$"t={RuntimeHelpers.GetHashCode (t).ToString ("x")}, " +
+					$"registered={RuntimeHelpers.GetHashCode (registered).ToString ("x")}");
+			Assert.AreSame (t, acIntermediate,
+					"Expected t and registered to be the same instance; " +
+					$"t={RuntimeHelpers.GetHashCode (t).ToString ("x")}, " +
+					$"acIntermediate={RuntimeHelpers.GetHashCode (acIntermediate).ToString ("x")}");
+			Assert.AreSame (t, cfIntermediate,
+					"Expected t and cfIntermediate to be the same instance; " +
+					$"t={RuntimeHelpers.GetHashCode (t).ToString ("x")}, " +
+					$"cfIntermediate={RuntimeHelpers.GetHashCode (cfIntermediate).ToString ("x")}");
+
+			Dispose (ref CallVirtualFromConstructorDerived.Intermediate_FromActivationConstructor);
+			Dispose (ref CallVirtualFromConstructorDerived.Intermediate_FromCalledFromConstructor);
 		}
 	}
 }
diff --git a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs
