[Java.Interop] JNIEnv::NewObject and Replaceable instances

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 (

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?

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)

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)

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 ();
 			}

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);
 		}
 	}
 }