From e323d1b55e0056b434e2b65eec44978c644a430c Mon Sep 17 00:00:00 2001 From: Jonathan Pryor <jonpryor@vt.edu> Date: Fri, 14 Feb 2025 15:53:24 -0500 Subject: [PATCH] [Java.Interop] CreatePeer() must satisfy targetType Context: https://github.com/dotnet/android/pull/9747 Context: https://discord.com/channels/732297728826277939/732297837953679412/1339638847864176640 Context: https://discord.com/channels/732297728826277939/732297837953679412/1340011105510101063 While trying to get a MAUI sample running atop NativeAOT, we're observing the following crash: E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidCastException: Arg_InvalidCastException E AndroidRuntime: at Java.Lang.Object._GetObject[T](IntPtr, JniHandleOwnership) + 0x64 E AndroidRuntime: at Microsoft.Maui.WindowOverlay.Initialize() + 0x168 Further investigation shows that the crash is from accessing the `Activity.WindowManager` property: namespace Android.App; partial class Activity { public virtual unsafe Android.Views.IWindowManager? WindowManager { // Metadata.xml XPath method reference: path="/api/package[@name='android.app']/class[@name='Activity']/method[@name='getWindowManager' and count(parameter)=0]" [Register ("getWindowManager", "()Landroid/view/WindowManager;", "GetGetWindowManagerHandler")] get { const string __id = "getWindowManager.()Landroid/view/WindowManager;"; try { var __rm = _members.InstanceMethods.InvokeVirtualObjectMethod (__id, this, null); return global::Java.Lang.Object.GetObject<Android.Views.IWindowManager> (__rm.Handle, JniHandleOwnership.TransferLocalRef); } finally { } } } } `Object.GetObject<T>()` is now a wrapper around `JniRuntime.JniValueManager.GetPeer()`, so the problem, rephrased, is that this: var peer = JniEnvironment.Runtime.ValueManager.GetPeer ( ref h, JniObjectReferenceOptions.CopyAndDispose, targetType:typeof (IWindowManager)); returns a value which *does not implement* `IWindowManager`. It was, in fact, returning a `Java.Lang.Object` instance (!). The cause of the bug is that `JniRuntime.JniValueManager.CreatePeer()` was not checking that the `Type` returned form `Runtime.TypeManager.GetType(JniTypeSignature)` was compatible with `targetType`; instead, it returned the first type in the inheritance chain that had an activation constructor. This was `Java.Lang.Object`. Later, when `_GetObject<T>()` tried to cast the return value of `JniRuntime.JniValueManager.GetPeer()` to `IWindowManager`, it failed. Fix this by updating `CreatePeer()` to check that the `Type` from `JniRuntime.JniTypeManager.GetType(JniTypeSignature)` is assignable to `targetType`. This ensures that we *don't* return a `Java.Lang.Object` instance, allowing the cast to succeed. Update `JniRuntimeJniValueManagerContract` to test these new semantics. --- .../JniRuntime.JniValueManager.cs | 6 +- .../Java.Interop-Tests.csproj | 1 + .../Java.Interop/JavaVMFixture.cs | 1 + .../JniRuntimeJniValueManagerContract.cs | 58 +++++++++++++++++++ .../jni/test/AnotherJavaInterfaceImpl.java | 13 +++++ 5 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 tests/Java.Interop-Tests/java/net/dot/jni/test/AnotherJavaInterfaceImpl.java diff --git a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs index eb9e23790..e08986e61 100644 --- a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs +++ b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs @@ -348,7 +348,7 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type) IJavaPeerable? CreatePeerInstance ( ref JniObjectReference klass, [DynamicallyAccessedMembers (Constructors)] - Type fallbackType, + Type targetType, ref JniObjectReference reference, JniObjectReferenceOptions transfer) { @@ -362,7 +362,7 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type) type = Runtime.TypeManager.GetType (sig); - if (type != null) { + if (type != null && targetType.IsAssignableFrom (type)) { var peer = TryCreatePeerInstance (ref reference, transfer, type); if (peer != null) { @@ -381,7 +381,7 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type) } JniObjectReference.Dispose (ref klass, JniObjectReferenceOptions.CopyAndDispose); - return TryCreatePeerInstance (ref reference, transfer, fallbackType); + return TryCreatePeerInstance (ref reference, transfer, targetType); } IJavaPeerable? TryCreatePeerInstance ( diff --git a/tests/Java.Interop-Tests/Java.Interop-Tests.csproj b/tests/Java.Interop-Tests/Java.Interop-Tests.csproj index cbb684413..4e881e793 100644 --- a/tests/Java.Interop-Tests/Java.Interop-Tests.csproj +++ b/tests/Java.Interop-Tests/Java.Interop-Tests.csproj @@ -36,6 +36,7 @@ </ItemGroup> <ItemGroup> + <JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\net\dot\jni\test\AnotherJavaInterfaceImpl.java" /> <JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CrossReferenceBridge.java" /> <JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CallNonvirtualBase.java" /> <JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CallNonvirtualDerived.java" /> diff --git a/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs b/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs index ee3413795..40da2e321 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs @@ -37,6 +37,7 @@ class JavaVMFixtureTypeManager : JniRuntime.JniTypeManager { [GenericHolder<int>.JniTypeName] = typeof (GenericHolder<>), [RenameClassBase.JniTypeName] = typeof (RenameClassBase), [RenameClassDerived.JniTypeName] = typeof (RenameClassDerived), + [AnotherJavaInterfaceImpl.JniTypeName] = typeof (AnotherJavaInterfaceImpl), [CallVirtualFromConstructorBase.JniTypeName] = typeof (CallVirtualFromConstructorBase), [CallVirtualFromConstructorDerived.JniTypeName] = typeof (CallVirtualFromConstructorDerived), [CrossReferenceBridge.JniTypeName] = typeof (CrossReferenceBridge), diff --git a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs index a112f155c..21727ce2e 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -133,6 +133,30 @@ public void CollectPeers () // TODO } + [Test] + public void CreatePeer_InvalidHandleReturnsNull () + { + var r = new JniObjectReference (); + var o = valueManager.CreatePeer (ref r, JniObjectReferenceOptions.Copy, null); + Assert.IsNull (o); + } + + [Test] + public unsafe void CreatePeer_UsesFallbackType () + { + using var t = new JniType (AnotherJavaInterfaceImpl.JniTypeName); + + var ctor = t.GetConstructor ("()V"); + var lref = t.NewObject (ctor, null); + + using var p = valueManager.CreatePeer (ref lref, JniObjectReferenceOptions.CopyAndDispose, typeof (IJavaInterface)); + + Assert.IsFalse (lref.IsValid); // .CopyAndDispose disposes + + Assert.IsNotNull (p); + Assert.AreSame (typeof (IJavaInterfaceInvoker), p!.GetType ()); + } + [Test] public void CreateValue () { @@ -294,4 +318,38 @@ public class JniRuntimeJniValueManagerContract_NoGCIntegration : JniRuntimeJniVa protected override Type ValueManagerType => ManagedValueManagerType; } #endif // !__ANDROID__ + + // Note: Java side implements JavaInterface, while managed binding DOES NOT. + // This is so that `CreatePeer(…, typeof(IJavaInterface))` tests don't use an existing AnotherJavaInterfaceImpl instance. + // + // This is mostly identical to MyJavaInterfaceImpl; the important difference is that + // it contains an activation constructor, while MyJavaInterfaceImpl does not. + // MyJavaInterfaceImpl can't have one, as that's what provokes the NotSupportedException in the JavaAs() tests. + // + // We want one here so that in "bad" `CreatePeer()` implementations, we'll find this peer and construct it + // before verifying that it satisfies the targetType requirement. + [JniTypeSignature (JniTypeName, GenerateJavaPeer=false)] + public class AnotherJavaInterfaceImpl : JavaObject { + internal const string JniTypeName = "net/dot/jni/test/AnotherJavaInterfaceImpl"; + + internal static readonly JniPeerMembers _members = new JniPeerMembers (JniTypeName, typeof (AnotherJavaInterfaceImpl)); + + public override JniPeerMembers JniPeerMembers { + get {return _members;} + } + + AnotherJavaInterfaceImpl (ref JniObjectReference reference, JniObjectReferenceOptions options) + : base (ref reference, options) + { + } + + public unsafe AnotherJavaInterfaceImpl () + : base (ref *InvalidJniObjectReference, JniObjectReferenceOptions.None) + { + const string id = "()V"; + var peer = _members.InstanceMethods.StartCreateInstance (id, GetType (), null); + Construct (ref peer, JniObjectReferenceOptions.CopyAndDispose); + _members.InstanceMethods.FinishCreateInstance (id, this, null); + } + } } diff --git a/tests/Java.Interop-Tests/java/net/dot/jni/test/AnotherJavaInterfaceImpl.java b/tests/Java.Interop-Tests/java/net/dot/jni/test/AnotherJavaInterfaceImpl.java new file mode 100644 index 000000000..02005c388 --- /dev/null +++ b/tests/Java.Interop-Tests/java/net/dot/jni/test/AnotherJavaInterfaceImpl.java @@ -0,0 +1,13 @@ +package net.dot.jni.test; + +public class AnotherJavaInterfaceImpl + implements JavaInterface, Cloneable +{ + public String getValue() { + return "Another hello from Java!"; + } + + public Object clone() { + return this; + } +}