[Java.Interop] CreatePeer() must satisfy targetType (#1308)

Context: dotnet/android#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 in this:
var peer = JniEnvironment.Runtime.ValueManager.GetPeer (
ref h,
JniObjectReferenceOptions.CopyAndDispose,
targetType:typeof (IWindowManager));
var wm = (IWindowManager) peer;
`peer` is a value which *does not implement* `IWindowManager`.
It was, in fact, returning a `Java.Lang.Object` instance (!).
Consequently, the cast to `IWindowManager` throws.
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.

Author: jonpryor

src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs

@@ -348,7 +348,7 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 IJavaPeerable?CreatePeerInstance(
 refJniObjectReferenceklass,
 [DynamicallyAccessedMembers(Constructors)]
-TypefallbackType,
+TypetargetType,
 refJniObjectReferencereference,
 JniObjectReferenceOptionstransfer)
 {
@@ -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)){
 varpeer=TryCreatePeerInstance(refreference,transfer,type);
 
 if(peer!=null){
@@ -381,7 +381,7 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 }
 JniObjectReference.Dispose(refklass,JniObjectReferenceOptions.CopyAndDispose);
 
-returnTryCreatePeerInstance(refreference,transfer,fallbackType);
+returnTryCreatePeerInstance(refreference,transfer,targetType);
 }
 
 IJavaPeerable?TryCreatePeerInstance(

tests/Java.Interop-Tests/Java.Interop-Tests.csproj

@@ -36,6 +36,7 @@
  </ItemGroup>
 
  <ItemGroup>
+ <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\net\dot\jni\test\AnotherJavaInterfaceImpl.java" />
  <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CrossReferenceBridge.java" />
  <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CallNonvirtualBase.java" />
  <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CallNonvirtualDerived.java" />

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),

tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs

@@ -133,6 +133,30 @@ public void CollectPeers ()
 // TODO
 }
 
+[Test]
+publicvoidCreatePeer_InvalidHandleReturnsNull()
+{
+varr=newJniObjectReference();
+varo=valueManager.CreatePeer(refr,JniObjectReferenceOptions.Copy,null);
+Assert.IsNull(o);
+}
+
+[Test]
+publicunsafevoidCreatePeer_UsesFallbackType()
+{
+usingvart=newJniType(AnotherJavaInterfaceImpl.JniTypeName);
+
+varctor=t.GetConstructor("()V");
+varlref=t.NewObject(ctor,null);
+
+usingvarp=valueManager.CreatePeer(reflref,JniObjectReferenceOptions.CopyAndDispose,typeof(IJavaInterface));
+
+Assert.IsFalse(lref.IsValid);// .CopyAndDispose disposes
+
+Assert.IsNotNull(p);
+Assert.AreSame(typeof(IJavaInterfaceInvoker),p!.GetType());
+}
+
 [Test]
 publicvoidCreateValue()
 {
@@ -294,4 +318,38 @@ public class JniRuntimeJniValueManagerContract_NoGCIntegration : JniRuntimeJniVa
 protectedoverrideTypeValueManagerType=>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)]
+publicclassAnotherJavaInterfaceImpl:JavaObject{
+internalconststringJniTypeName="net/dot/jni/test/AnotherJavaInterfaceImpl";
+
+internalstaticreadonlyJniPeerMembers_members=newJniPeerMembers(JniTypeName,typeof(AnotherJavaInterfaceImpl));
+
+publicoverrideJniPeerMembersJniPeerMembers{
+get{return_members;}
+}
+
+AnotherJavaInterfaceImpl(refJniObjectReferencereference,JniObjectReferenceOptionsoptions)
+:base(refreference,options)
+{
+}
+
+publicunsafeAnotherJavaInterfaceImpl()
+:base(ref*InvalidJniObjectReference,JniObjectReferenceOptions.None)
+{
+conststringid="()V";
+varpeer=_members.InstanceMethods.StartCreateInstance(id,GetType(),null);
+Construct(refpeer,JniObjectReferenceOptions.CopyAndDispose);
+_members.InstanceMethods.FinishCreateInstance(id,this,null);
+}
+}
 }

tests/Java.Interop-Tests/java/net/dot/jni/test/AnotherJavaInterfaceImpl.java

@@ -0,0 +1,13 @@
+packagenet.dot.jni.test;
+
+publicclassAnotherJavaInterfaceImpl
+implementsJavaInterface, Cloneable
+{
+publicStringgetValue() {
+return"Another hello from Java!";
+	}
+
+publicObjectclone() {
+returnthis;
+	}
+}