diff --git a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs index c33f4b9ae..1ce3e6543 100644 --- a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs +++ b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs @@ -401,7 +401,27 @@ 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; @@ -409,7 +429,8 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type) 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 index f25b9f6bb..90cde5ae6 100644 --- 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 index 46fd69206..aff5d4432 100644 --- 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 index 25e36762a..9fe6ebe8b 100644 --- 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 index 78e09d469..739b85e08 100644 --- 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,10 +27,22 @@ 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"); @@ -35,10 +50,15 @@ public CallVirtualFromConstructorDerived (int 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,6 +67,8 @@ public override void CalledFromConstructor (int value) { Called = true; calledValue = value; + + Intermediate_FromCalledFromConstructor = this; } public static unsafe CallVirtualFromConstructorDerived NewInstance (int value) @@ -54,7 +76,7 @@ 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 index ed3e585e0..95d8e81d5 100644 --- 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 index 782b6e765..475403875 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -5,6 +5,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using System.Threading; using Java.Interop; @@ -159,6 +160,65 @@ public unsafe void CreatePeer_UsesFallbackType () Assert.AreSame (typeof (IJavaInterfaceInvoker), p!.GetType ()); } + [Test] + public void CreatePeer_CreatesNewValueUsingActivationConstructor () + { + using var v1 = new AnotherJavaInterfaceImpl (); + var lref = v1.PeerReference.NewLocalRef (); + try { + using var v2 = valueManager.CreatePeer (ref lref, JniObjectReferenceOptions.CopyAndDispose, typeof (AnotherJavaInterfaceImpl)); + Assert.AreNotSame (v1, v2, "CreatePeer() should create new values"); + } + finally { + JniObjectReference.Dispose (ref lref); + } + } + + [Test] + public void CreatePeer_ThrowsIfNoActivationConstructorPresent () + { + using var v1 = new GetThis (); + var lref = v1.PeerReference.NewLocalRef (); + var ex = Assert.Throws<NotSupportedException> ( + () => valueManager.CreatePeer (ref lref, JniObjectReferenceOptions.CopyAndDispose, typeof (GetThis)), + $"`GetThis` has no activation constructor, so attempting to use it should throw NotSupportedException."); + Assert.IsTrue (lref.IsValid, "lref should still be valid"); + JniObjectReference.Dispose (ref lref); + } + + [Test] + public void CreatePeer_ReplaceableDoesNotReplace () + { + var v = new AnotherJavaInterfaceImpl (); + var lref = v.PeerReference.NewLocalRef (); + v.Dispose (); + + try { + Assert.IsNull (valueManager.PeekPeer (lref), "v.Dispose() should have unregistered the peer."); + var peer1 = valueManager.CreatePeer (ref lref, JniObjectReferenceOptions.Copy, typeof (AnotherJavaInterfaceImpl)); + Assert.IsTrue ( + peer1!.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable), + $"Expected peer1.JniManagedPeerState to have .Replaceable, but was {peer1.JniManagedPeerState}."); + Assert.AreSame (peer1, valueManager.PeekPeer (lref), + $"Expected peer1==PeekValue(peer1.PeerReference); it's the only one that should exist!"); + + var peer2 = valueManager.CreatePeer (ref lref, JniObjectReferenceOptions.Copy, typeof (AnotherJavaInterfaceImpl)); + Assert.IsTrue ( + peer2!.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable), + $"Expected peer2.JniManagedPeerState to have .Replaceable, but was {peer2.JniManagedPeerState}."); + Assert.AreNotSame (peer1, peer2, "Expected peer1 and peer2 to be different instances."); + + var peeked = valueManager.PeekPeer (lref); + Assert.AreSame (peer1, peeked, + "Expected peer1 and peeked to be the same instance; " + + $"peeked={RuntimeHelpers.GetHashCode (peeked).ToString ("x")}, " + + $"peer1={RuntimeHelpers.GetHashCode (peer1).ToString ("x")}, " + + $"peer2={RuntimeHelpers.GetHashCode (peer2).ToString ("x")}"); + } finally { + JniObjectReference.Dispose (ref lref); + } + } + [Test] public void CreateValue () {