From 4f3be333a3460e9e61b6845babf47e5c76efb491 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 29 Jan 2025 08:50:20 -0500 Subject: [PATCH 1/4] [Java.Interop] Add JniRuntime.JniValueManager.GetPeer() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/dotnet/android/pull/9630/files#r1891090085 Context: https://github.com/dotnet/android/pull/9716 As part of NativeAOT prototyping within dotnet/android, we need to update `Java.Lang.Object.GetObject()` so that it uses `Java.Interop.JniRuntime.JniValueManager` APIs instead of its own `TypeManager.CreateInstance()` invocation, as `TypeManager.CreateInstance()` hits P/Invokes which don't currently work in the NativeAOT sample environment. However, updated it to *what*? The obvious thing to do would be to use `JniRuntime.JniValueManager.GetValue()`: partial class Object { internal static IJavaPeerable? GetObject (IntPtr handle, JniHandleOwnershipt ransfer, Type? type = null) { var r = PeekObject (handle, type); if (r != null) { JNIEnv.DeleteRef (handle, transfer); return r; } var reference = new JniObjectReference (handle); r = (IJavaPeerable) JNIEnvInit.ValueManager.GetValue ( ref reference, JniObjectReferenceOptions.Copy, type); JNIEnv.DeleteRef (handle, transfer); return r; } } The problem is that this blows up good: g__GetObject|74_11(IntPtr , Type ) at Android.Runtime.JNIEnv.<>c.b__74_9(Type type, IntPtr source, Int32 index) at Android.Runtime.JNIEnv.GetObjectArray(IntPtr , Type[] ) at Java.InteropTests.JnienvTest.<>c__DisplayClass26_0.b__1()> because somewhere in that stack trace we have a `java.lang.Integer` instance, and `.GetValue(Integer_ref, …)` returns a `System.Int32` containing the underling value, *not* an `IJavaPeerable` value for the `java.lang.Integer` instance. Consider: var i_class = new JniType ("java/lang/Integer"); var i_ctor = i_class.GetConstructor ("(I)V"); JniArgumentValue* i_args = stackalloc JniArgumentValue [1]; i_args [0] = new JniArgumentValue (42); var i_value = i_class.NewObject (i_ctor, i_args); var v = JniEnvironment.Runtime.ValueManager.GetValue (ref i_value, JniObjectReferenceOptions.CopyAndDispose, null); Console.WriteLine ($"v? {v} {v?.GetType ()}"); which prints `v? 42 System.Int32`. This was expected and desirable, until we try to use `GetValue()` for `Object.GetObject()`; the semantics don't match. Add a new `JniRuntime.JniValueManager.GetPeer()` method, which better matches the semantics that `Object.GetObject()` requires, allowing: partial class Object { internal static IJavaPeerable? GetObject (IntPtr handle, JniHandleOwnershipt ransfer, Type? type = null) { var r = JNIEnvInit.ValueManager.GetPeer (new JniObjectReference (handle)); JNIEnv.DeleteRef (handle, transfer); return r; } } Finally, add a new `JniRuntimeJniValueManagerContract` unit test, so that we have "more formalized" semantic requirements on `JniRuntime.JniValueManager` implementations. --- .../JniRuntime.JniValueManager.cs | 24 +- src/Java.Interop/PublicAPI.Unshipped.txt | 1 + .../Java.Interop-Tests.csproj | 2 + .../JniRuntimeJniValueManagerContract.cs | 292 ++++++++++++++++++ tests/TestJVM/TestJVM.csproj | 2 + 5 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs diff --git a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs index 55af4f7b4..ad8d04c35 100644 --- a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs +++ b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs @@ -32,7 +32,7 @@ partial class CreationOptions { public JniValueManager? ValueManager {get; set;} } - JniValueManager? valueManager; + internal JniValueManager? valueManager; public JniValueManager ValueManager { get => valueManager ?? throw new NotSupportedException (); } @@ -271,6 +271,28 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type) return type; } + public IJavaPeerable? GetPeer ( + JniObjectReference reference, + [DynamicallyAccessedMembers (Constructors)] + Type? targetType = null) + { + if (disposed) { + throw new ObjectDisposedException (GetType ().Name); + } + + if (!reference.IsValid) { + return null; + } + + var peeked = PeekPeer (reference); + if (peeked != null && + (targetType == null || + targetType.IsAssignableFrom (peeked.GetType ()))) { + return peeked; + } + return CreatePeer (ref reference, JniObjectReferenceOptions.Copy, targetType); + } + public virtual IJavaPeerable? CreatePeer ( ref JniObjectReference reference, JniObjectReferenceOptions transfer, diff --git a/src/Java.Interop/PublicAPI.Unshipped.txt b/src/Java.Interop/PublicAPI.Unshipped.txt index d4be03231..5a788dfa6 100644 --- a/src/Java.Interop/PublicAPI.Unshipped.txt +++ b/src/Java.Interop/PublicAPI.Unshipped.txt @@ -5,5 +5,6 @@ virtual Java.Interop.JniRuntime.OnEnterMarshalMethod() -> void virtual Java.Interop.JniRuntime.OnUserUnhandledException(ref Java.Interop.JniTransition transition, System.Exception! e) -> void 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.JniValueManager.GetPeer(Java.Interop.JniObjectReference reference, System.Type? targetType = null) -> Java.Interop.IJavaPeerable? Java.Interop.JniTypeSignatureAttribute.InvokerType.get -> System.Type? Java.Interop.JniTypeSignatureAttribute.InvokerType.set -> void diff --git a/tests/Java.Interop-Tests/Java.Interop-Tests.csproj b/tests/Java.Interop-Tests/Java.Interop-Tests.csproj index 0677be887..cbb684413 100644 --- a/tests/Java.Interop-Tests/Java.Interop-Tests.csproj +++ b/tests/Java.Interop-Tests/Java.Interop-Tests.csproj @@ -3,6 +3,8 @@ $(DotNetTargetFramework) false + true + ..\..\product.snk true $(DefineConstants);NO_MARSHAL_MEMBER_BUILDER_SUPPORT;NO_GC_BRIDGE_SUPPORT diff --git a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs new file mode 100644 index 000000000..a41c289a8 --- /dev/null +++ b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -0,0 +1,292 @@ +#nullable enable + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Threading; + +using Java.Interop; + +using NUnit.Framework; + +namespace Java.InteropTests { + + // Modifies JniRuntime.valueManager instance field; can't be done in parallel + [NonParallelizable] + public abstract class JniRuntimeJniValueManagerContract : JavaVMFixture { + + protected abstract Type ValueManagerType { + get; + } + + protected virtual JniRuntime.JniValueManager CreateValueManager () + { + var manager = Activator.CreateInstance (ValueManagerType) as JniRuntime.JniValueManager; + return manager ?? throw new InvalidOperationException ($"Could not create instance of `{ValueManagerType}`!"); + } + +#pragma warning disable CS8618 + JniRuntime.JniValueManager systemManager; + JniRuntime.JniValueManager valueManager; +#pragma warning restore CS8618 + + [SetUp] + public void CreateVM () + { + systemManager = JniRuntime.CurrentRuntime.valueManager!; + valueManager = CreateValueManager (); + valueManager.OnSetRuntime (JniRuntime.CurrentRuntime); + JniRuntime.CurrentRuntime.valueManager = valueManager; + } + + [TearDown] + public void DestroyVM () + { + JniRuntime.CurrentRuntime.valueManager = systemManager; + systemManager = null!; + valueManager?.Dispose (); + valueManager = null!; + } + + [Test] + public void AddPeer () + { + } + + int GetSurfacedPeersCount () + { + return valueManager.GetSurfacedPeers ().Count; + } + + [Test] + public void AddPeer_NoDuplicates () + { + int startPeerCount = GetSurfacedPeersCount (); + using (var v = new MyDisposableObject ()) { + // MyDisposableObject ctor implicitly calls AddPeer(); + Assert.AreEqual (startPeerCount + 1, GetSurfacedPeersCount (), DumpPeers ()); + valueManager.AddPeer (v); + Assert.AreEqual (startPeerCount + 1, GetSurfacedPeersCount (), DumpPeers ()); + } + } + + [Test] + public void ConstructPeer_ImplicitViaBindingConstructor_PeerIsInSurfacedPeers () + { + int startPeerCount = GetSurfacedPeersCount (); + + var g = new GetThis (); + var surfaced = valueManager.GetSurfacedPeers (); + Assert.AreEqual (startPeerCount + 1, surfaced.Count); + + var found = false; + foreach (var pr in surfaced) { + if (!pr.SurfacedPeer.TryGetTarget (out var p)) + continue; + if (object.ReferenceEquals (g, p)) { + found = true; + } + } + Assert.IsTrue (found); + + var localRef = g.PeerReference.NewLocalRef (); + g.Dispose (); + Assert.AreEqual (startPeerCount, GetSurfacedPeersCount ()); + Assert.IsNull (valueManager.PeekPeer (localRef)); + JniObjectReference.Dispose (ref localRef); + } + + [Test] + public void ConstructPeer_ImplicitViaBindingMethod_PeerIsInSurfacedPeers () + { + int startPeerCount = GetSurfacedPeersCount (); + + var g = new GetThis (); + var surfaced = valueManager.GetSurfacedPeers (); + Assert.AreEqual (startPeerCount + 1, surfaced.Count); + + var found = false; + foreach (var pr in surfaced) { + if (!pr.SurfacedPeer.TryGetTarget (out var p)) + continue; + if (object.ReferenceEquals (g, p)) { + found = true; + } + } + Assert.IsTrue (found); + + var localRef = g.PeerReference.NewLocalRef (); + g.Dispose (); + Assert.AreEqual (startPeerCount, GetSurfacedPeersCount ()); + Assert.IsNull (valueManager.PeekPeer (localRef)); + JniObjectReference.Dispose (ref localRef); + } + + + [Test] + public void CollectPeers () + { + // TODO + } + + [Test] + public void CreateValue () + { + using (var o = new JavaObject ()) { + var r = o.PeerReference; + var x = (IJavaPeerable) valueManager.CreateValue (ref r, JniObjectReferenceOptions.Copy)!; + Assert.AreNotSame (o, x); + x.Dispose (); + + x = valueManager.CreateValue (ref r, JniObjectReferenceOptions.Copy); + Assert.AreNotSame (o, x); + x!.Dispose (); + } + } + + [Test] + public void GetValue_ReturnsAlias () + { + var local = new JavaObject (); + local.UnregisterFromRuntime (); + Assert.IsNull (valueManager.PeekValue (local.PeerReference)); + // GetObject must always return a value (unless handle is null, etc.). + // However, since we called local.UnregisterFromRuntime(), + // JniRuntime.PeekObject() is null (asserted above), but GetObject() must + // **still** return _something_. + // In this case, it returns an _alias_. + // TODO: "most derived type" alias generation. (Not relevant here, but...) + var p = local.PeerReference; + var alias = JniRuntime.CurrentRuntime.ValueManager.GetValue (ref p, JniObjectReferenceOptions.Copy); + Assert.AreNotSame (local, alias); + alias!.Dispose (); + local.Dispose (); + } + + [Test] + public void GetValue_ReturnsNullWithNullHandle () + { + var r = new JniObjectReference (); + var o = valueManager.GetValue (ref r, JniObjectReferenceOptions.Copy); + Assert.IsNull (o); + } + + [Test] + public void GetValue_ReturnsNullWithInvalidSafeHandle () + { + var invalid = new JniObjectReference (); + Assert.IsNull (valueManager.GetValue (ref invalid, JniObjectReferenceOptions.CopyAndDispose)); + } + + [Test] + public unsafe void GetValue_FindBestMatchType () + { +#if !NO_MARSHAL_MEMBER_BUILDER_SUPPORT + using (var t = new JniType (TestType.JniTypeName)) { + var c = t.GetConstructor ("()V"); + var o = t.NewObject (c, null); + using (var w = valueManager.GetValue (ref o, JniObjectReferenceOptions.CopyAndDispose)) { + Assert.AreEqual (typeof (TestType), w!.GetType ()); + Assert.IsTrue (((TestType) w).ExecutedActivationConstructor); + } + } +#endif // !NO_MARSHAL_MEMBER_BUILDER_SUPPORT + } + + [Test] + public void PeekPeer () + { + Assert.IsNull (valueManager.PeekPeer (new JniObjectReference ())); + + using (var v = new MyDisposableObject ()) { + Assert.IsNotNull (valueManager.PeekPeer (v.PeerReference)); + Assert.AreSame (v, valueManager.PeekPeer (v.PeerReference)); + } + } + + [Test] + public void PeekValue () + { + JniObjectReference lref; + using (var o = new JavaObject ()) { + lref = o.PeerReference.NewLocalRef (); + Assert.AreSame (o, valueManager.PeekValue (lref)); + } + // At this point, the Java-side object is kept alive by `lref`, + // but the wrapper instance has been disposed, and thus should + // be unregistered, and thus unfindable. + Assert.IsNull (valueManager.PeekValue (lref)); + JniObjectReference.Dispose (ref lref); + } + + [Test] + public void PeekValue_BoxedObjects () + { + var marshaler = valueManager.GetValueMarshaler (); + var ad = AppDomain.CurrentDomain; + + var proxy = marshaler.CreateGenericArgumentState (ad); + Assert.AreSame (ad, valueManager.PeekValue (proxy.ReferenceValue)); + marshaler.DestroyGenericArgumentState (ad, ref proxy); + + var ex = new InvalidOperationException ("boo!"); + proxy = marshaler.CreateGenericArgumentState (ex); + Assert.AreSame (ex, valueManager.PeekValue (proxy.ReferenceValue)); + marshaler.DestroyGenericArgumentState (ex, ref proxy); + } + + void AllNestedRegistrationScopeTests () + { + AddPeer (); + AddPeer_NoDuplicates (); + ConstructPeer_ImplicitViaBindingConstructor_PeerIsInSurfacedPeers (); + CreateValue (); + GetValue_FindBestMatchType (); + GetValue_ReturnsAlias (); + GetValue_ReturnsNullWithInvalidSafeHandle (); + GetValue_ReturnsNullWithNullHandle (); + PeekPeer (); + PeekValue (); + PeekValue_BoxedObjects (); + } + + string DumpPeers () + { + return DumpPeers (valueManager.GetSurfacedPeers ()); + } + + static string DumpPeers (IEnumerable peers) + { + return string.Join ("," + Environment.NewLine, peers); + } + + + // also test: + // Singleton scenario + // Types w/o "activation" constructors -- need to support checking parent scopes + // nesting of scopes + // Adding an instance already added in a previous scope? + } + + public abstract class JniRuntimeJniValueManagerContract : JniRuntimeJniValueManagerContract { + + protected override Type ValueManagerType => typeof (T); + } + +#if !NETCOREAPP + [TestFixture] + public class JniRuntimeJniValueManagerContract_Mono : JniRuntimeJniValueManagerContract { + static Type MonoRuntimeValueManagerType = Type.GetType ("Java.Interop.MonoRuntimeValueManager, Java.Runtime.Environment", throwOnError:true)!; + + protected override Type ValueManagerType => MonoRuntimeValueManagerType; + } +#endif // !NETCOREAPP + + [TestFixture] + public class JniRuntimeJniValueManagerContract_NoGCIntegration : JniRuntimeJniValueManagerContract { + static Type ManagedValueManagerType = Type.GetType ("Java.Interop.ManagedValueManager, Java.Runtime.Environment", throwOnError:true)!; + + protected override Type ValueManagerType => ManagedValueManagerType; + } +} diff --git a/tests/TestJVM/TestJVM.csproj b/tests/TestJVM/TestJVM.csproj index 5660c4e4a..2ab635532 100644 --- a/tests/TestJVM/TestJVM.csproj +++ b/tests/TestJVM/TestJVM.csproj @@ -4,6 +4,8 @@ $(DotNetTargetFramework) enable false + true + ..\..\product.snk From c5a022c9d38e76a9bdc5d854c656b7f8522c2ced Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 29 Jan 2025 13:56:45 -0500 Subject: [PATCH 2/4] Avoid [NonParallelizable] on Android Fixes build failure: /Users/runner/work/1/s/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs(16,3): error CS0246: The type or namespace name 'NonParallelizableAttribute' could not be found (are you missing a using directive or an assembly reference?) /Users/runner/work/1/s/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs(16,3): error CS0246: The type or namespace name 'NonParallelizable' could not be found (are you missing a using directive or an assembly reference?) --- .../Java.Interop/JniRuntimeJniValueManagerContract.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs index a41c289a8..9709028dc 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -12,8 +12,11 @@ namespace Java.InteropTests { + // Android doesn't support `[NonParallelizable]`, but runs tests sequentially by default. +#if !__ANDROID__ // Modifies JniRuntime.valueManager instance field; can't be done in parallel [NonParallelizable] +#endif // !__ANDROID__ public abstract class JniRuntimeJniValueManagerContract : JavaVMFixture { protected abstract Type ValueManagerType { From bf25acea0f01ce8d7759f0187771092f70557305 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 29 Jan 2025 15:43:31 -0500 Subject: [PATCH 3/4] Exclude Java.Runtime.Environment references from Android. --- .../Java.Interop/JniRuntimeJniValueManagerContract.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs index 9709028dc..bc562cea3 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -277,6 +277,7 @@ public abstract class JniRuntimeJniValueManagerContract : JniRuntimeJniValueM protected override Type ValueManagerType => typeof (T); } +#if !__ANDROID__ #if !NETCOREAPP [TestFixture] public class JniRuntimeJniValueManagerContract_Mono : JniRuntimeJniValueManagerContract { @@ -293,3 +294,4 @@ public class JniRuntimeJniValueManagerContract_NoGCIntegration : JniRuntimeJniVa protected override Type ValueManagerType => ManagedValueManagerType; } } +#endif // !__ANDROID__ From 01a0448619e8f13d89894f63eda7d8a496af7856 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 29 Jan 2025 17:21:43 -0500 Subject: [PATCH 4/4] Doh! --- .../Java.Interop/JniRuntimeJniValueManagerContract.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs index bc562cea3..a112f155c 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -293,5 +293,5 @@ public class JniRuntimeJniValueManagerContract_NoGCIntegration : JniRuntimeJniVa protected override Type ValueManagerType => ManagedValueManagerType; } -} #endif // !__ANDROID__ +}