From f4627522246c301f2ccac12abcb9031ebce00722 Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Wed, 28 Aug 2019 12:53:19 -0500 Subject: [PATCH] [Java.Interop, generator] Fix GetPeerMembers for interfaces (#486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/xamarin-android/pull/3533 Consider the following Java interface which contains a default interface method: // Java package com.xamarin.android; public interface DefaultMethodsInterface { default int foo () { return 0; } } This is bound as: // C# Binding public partial interface IDefaultMethodsInterface : IJavaPeerable { static readonly JniPeerMembers _members = new JniPeerMembers ( "java/DefaultMethodsInterface", typeof (IDefaultMethodsInterface)); [Register ("foo", "()V", "…")] virtual unsafe int Foo () { return _members.InstanceMethods.InvokeVirtualInt32Method ("foo.()V", this, null); } } If we "implement" the interface from C# *without* implementing `IDefaultMethodsInterface.Foo()`: // C# class ManagedOverrideDefault : Java.Lang.Object, IDefaultMethodsInterface { } Then invoke the default interface method: var over = new ManagedOverrideDefault (); (over as IDefaultMethodsInterface).Foo (); The attempt to invoke the default implementation of `IDefaultMethodsInterface.Foo()` causes a crash: Java.Lang.NoSuchMethodError : no non-static method "Ljava/lang/Object;.foo()I" The cause of the crash is as follows: `IDefaultMethodsInterface.Foo()` calls `JniPeerMembers.JniInstanceMethods.InvokeVirtualInt32Method()` tries to determine if the method is overridden, and if the method is *not* overridden -- as is the case here -- then we hit the non-virtual invocation path: // src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods_Invoke.cs var j = Members.GetPeerMembers (self); var n = j.InstanceMethods.GetMethodInfo (encodedMember); return JniEnvironment.InstanceMethods.CallNonvirtualIntMethod (self.PeerReference, j.JniPeerType.PeerReference, n, parameters); `JniInstanceMethods.Members` will be the `IDefaultMethodsInterface._members` field, and `Members.GetPeerMembers()` would return `self.JniPeerMembers`. Because this is a C# class which inherits `Java.Lang.Object`, `Java.Lang.Object._members` is returned. We then attempt to resolve `foo()` from `java.lang.Object`, which *does not exist*, which is why the `NoSuchMethodError` is thrown. The fix is to "intercept" the `Members.GetPeerMembers()` invocation so that `j` refers to `IDefaultMethodsInterface._members`, *not* `Java.Lang.Object._members`. This would allow `n` to refer to `DefaultMethodsInterface.foo()`, which *does* exist, and *can* be invoked via `JniEnvironment.InstanceMethods.CallNonvirtualIntMethod()`. Add the following `JniPeerMembers` constructor overload: partial class JniPeerMembers { public JniPeerMembers (string jniPeerTypeName, Type managedPeerType, bool isInterface); } Update `JniPeerMembers.GetPeerMembers()` so that if `isInterface` is true, we return `this` (the *interface*s `JniPeerMembers` instance). Update `generator` so that `_members` construction within interfaces sets the `isInterface` parameter to true, resulting in: partial interface IDefaultMethodsInterface { static readonly JniPeerMembers _members = new JniPeerMembers ( "java/DefaultMethodsInterface", typeof (IDefaultMethodsInterface), isInterface: true); } This allows `(over as IDefaultMethodsInterface).Foo()` to work without an exception being thrown. --- gendarme-ignore.txt | 1 + src/Java.Interop/Java.Interop/JniPeerMembers.cs | 15 ++++++++++++--- .../JavaInteropCodeGenerator.cs | 12 ++++++------ .../JavaInterop1/WriteInterfaceDefaultMethod.txt | 2 +- .../WriteInterfaceDefaultProperty.txt | 2 +- .../WriteInterfaceDefaultPropertyGetterOnly.txt | 2 +- .../WriteInterfaceDefaultMethod.txt | 2 +- .../WriteInterfaceDefaultProperty.txt | 2 +- .../WriteInterfaceDefaultPropertyGetterOnly.txt | 2 +- 9 files changed, 25 insertions(+), 15 deletions(-) diff --git a/gendarme-ignore.txt b/gendarme-ignore.txt index a18208a9a..7bb3f7d60 100644 --- a/gendarme-ignore.txt +++ b/gendarme-ignore.txt @@ -351,6 +351,7 @@ T: Java.Interop.JniInt32ArrayElements T: Java.Interop.JniInt64ArrayElements T: Java.Interop.JniLocationException T: Java.Interop.JniMethodInfo +T: Java.Interop.JniPeerMembers T: Java.Interop.JniSByteArrayElements T: Java.Interop.JniSingleArrayElements T: Java.Interop.JniType diff --git a/src/Java.Interop/Java.Interop/JniPeerMembers.cs b/src/Java.Interop/Java.Interop/JniPeerMembers.cs index b37d10c2b..50cf4dcf1 100644 --- a/src/Java.Interop/Java.Interop/JniPeerMembers.cs +++ b/src/Java.Interop/Java.Interop/JniPeerMembers.cs @@ -7,8 +7,15 @@ namespace Java.Interop { public partial class JniPeerMembers { + private bool isInterface; + + public JniPeerMembers (string jniPeerTypeName, Type managedPeerType, bool isInterface) + : this (jniPeerTypeName, managedPeerType, checkManagedPeerType: true, isInterface: isInterface) + { + } + public JniPeerMembers (string jniPeerTypeName, Type managedPeerType) - : this (jniPeerTypeName, managedPeerType, checkManagedPeerType: true) + : this (jniPeerTypeName, managedPeerType, checkManagedPeerType: true, isInterface: false) { if (managedPeerType == null) throw new ArgumentNullException ("managedPeerType"); @@ -27,7 +34,7 @@ public JniPeerMembers (string jniPeerTypeName, Type managedPeerType) ManagedPeerType = managedPeerType; } - JniPeerMembers (string jniPeerTypeName, Type managedPeerType, bool checkManagedPeerType) + JniPeerMembers (string jniPeerTypeName, Type managedPeerType, bool checkManagedPeerType, bool isInterface = false) { if (jniPeerTypeName == null) throw new ArgumentNullException (nameof (jniPeerTypeName)); @@ -51,6 +58,8 @@ public JniPeerMembers (string jniPeerTypeName, Type managedPeerType) JniPeerTypeName = jniPeerTypeName; ManagedPeerType = managedPeerType; + this.isInterface = isInterface; + instanceMethods = new JniInstanceMethods (this); instanceFields = new JniInstanceFields (this); staticMethods = new JniStaticMethods (this); @@ -140,7 +149,7 @@ protected virtual bool UsesVirtualDispatch (IJavaPeerable value, Type declaringT protected virtual JniPeerMembers GetPeerMembers (IJavaPeerable value) { - return value.JniPeerMembers; + return isInterface ? this : value.JniPeerMembers; } internal static void AssertSelf (IJavaPeerable self) diff --git a/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs b/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs index 3c89671a1..2873d8df0 100644 --- a/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs +++ b/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs @@ -29,7 +29,7 @@ static string GetInvokeType (string type) internal override void WriteClassHandle (ClassGen type, string indent, bool requireNew) { - WritePeerMembers (indent + '\t', true, requireNew, type.RawJniName, type.Name); + WritePeerMembers (indent + '\t', true, requireNew, type.RawJniName, type.Name, false); writer.WriteLine ("{0}\tinternal static {1}IntPtr class_ref {{", indent, requireNew ? "new " : string.Empty); writer.WriteLine ("{0}\t\tget {{", indent); @@ -55,12 +55,12 @@ internal override void WriteClassHandle (ClassGen type, string indent, bool requ internal override void WriteClassHandle (InterfaceGen type, string indent, string declaringType) { - WritePeerMembers (indent, false, true, type.RawJniName, declaringType); + WritePeerMembers (indent, false, true, type.RawJniName, declaringType, type.Name == declaringType); } internal override void WriteClassInvokerHandle (ClassGen type, string indent, string declaringType) { - WritePeerMembers (indent, true, true, type.RawJniName, declaringType); + WritePeerMembers (indent, true, true, type.RawJniName, declaringType, false); writer.WriteLine (); writer.WriteLine ("{0}public override global::Java.Interop.JniPeerMembers JniPeerMembers {{", indent); @@ -75,7 +75,7 @@ internal override void WriteClassInvokerHandle (ClassGen type, string indent, st internal override void WriteInterfaceInvokerHandle (InterfaceGen type, string indent, string declaringType) { - WritePeerMembers (indent, true, true, type.RawJniName, declaringType); + WritePeerMembers (indent, true, true, type.RawJniName, declaringType, false); writer.WriteLine (); writer.WriteLine ("{0}static IntPtr java_class_ref {{", indent); @@ -262,10 +262,10 @@ internal override void WriteFieldSetBody (Field field, string indent, GenBase ty writer.WriteLine ("{0}}}", indent); } - void WritePeerMembers (string indent, bool isInternal, bool isNew, string rawJniType, string declaringType) + void WritePeerMembers (string indent, bool isInternal, bool isNew, string rawJniType, string declaringType, bool isInterface) { var signature = $"{(isInternal ? "internal " : "")}static {(isNew ? "new " : "")}readonly JniPeerMembers _members = "; - var type = $"new {GetPeerMembersType ()} (\"{rawJniType}\", typeof ({declaringType}));"; + var type = $"new {GetPeerMembersType ()} (\"{rawJniType}\", typeof ({declaringType}){(isInterface ? ", isInterface: true" : string.Empty)});"; writer.WriteLine ($"{indent}{signature}{type}"); } diff --git a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultMethod.txt b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultMethod.txt index 6eece822a..ca34ddc70 100644 --- a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultMethod.txt +++ b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultMethod.txt @@ -1,7 +1,7 @@ // Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']" [Register ("java/code/IMyInterface", "", "java.code.IMyInterfaceInvoker")] public partial interface IMyInterface : IJavaObject, IJavaPeerable { - static new readonly JniPeerMembers _members = new JniPeerMembers ("java/code/IMyInterface", typeof (IMyInterface)); + static new readonly JniPeerMembers _members = new JniPeerMembers ("java/code/IMyInterface", typeof (IMyInterface), isInterface: true); static Delegate cb_DoSomething; #pragma warning disable 0169 diff --git a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultProperty.txt b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultProperty.txt index 0a9514cbf..4bfa1b7cd 100644 --- a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultProperty.txt +++ b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultProperty.txt @@ -1,7 +1,7 @@ // Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']" [Register ("java/code/IMyInterface", "", "java.code.IMyInterfaceInvoker")] public partial interface IMyInterface : IJavaObject, IJavaPeerable { - static new readonly JniPeerMembers _members = new JniPeerMembers ("java/code/IMyInterface", typeof (IMyInterface)); + static new readonly JniPeerMembers _members = new JniPeerMembers ("java/code/IMyInterface", typeof (IMyInterface), isInterface: true); static Delegate cb_get_Value; #pragma warning disable 0169 diff --git a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt index 6eb8aa760..43c93eb91 100644 --- a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt +++ b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt @@ -1,7 +1,7 @@ // Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']" [Register ("java/code/IMyInterface", "", "java.code.IMyInterfaceInvoker")] public partial interface IMyInterface : IJavaObject, IJavaPeerable { - static new readonly JniPeerMembers _members = new JniPeerMembers ("java/code/IMyInterface", typeof (IMyInterface)); + static new readonly JniPeerMembers _members = new JniPeerMembers ("java/code/IMyInterface", typeof (IMyInterface), isInterface: true); static Delegate cb_get_Value; #pragma warning disable 0169 diff --git a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultMethod.txt b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultMethod.txt index ff77d6c71..9681c4527 100644 --- a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultMethod.txt +++ b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultMethod.txt @@ -1,7 +1,7 @@ // Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']" [Register ("java/code/IMyInterface", "", "java.code.IMyInterfaceInvoker")] public partial interface IMyInterface : IJavaObject, IJavaPeerable { - static new readonly JniPeerMembers _members = new XAPeerMembers ("java/code/IMyInterface", typeof (IMyInterface)); + static new readonly JniPeerMembers _members = new XAPeerMembers ("java/code/IMyInterface", typeof (IMyInterface), isInterface: true); static Delegate cb_DoSomething; #pragma warning disable 0169 diff --git a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultProperty.txt b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultProperty.txt index a0c56ba92..955fd7400 100644 --- a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultProperty.txt +++ b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultProperty.txt @@ -1,7 +1,7 @@ // Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']" [Register ("java/code/IMyInterface", "", "java.code.IMyInterfaceInvoker")] public partial interface IMyInterface : IJavaObject, IJavaPeerable { - static new readonly JniPeerMembers _members = new XAPeerMembers ("java/code/IMyInterface", typeof (IMyInterface)); + static new readonly JniPeerMembers _members = new XAPeerMembers ("java/code/IMyInterface", typeof (IMyInterface), isInterface: true); static Delegate cb_get_Value; #pragma warning disable 0169 diff --git a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt index a0f879bd5..fefd8f47e 100644 --- a/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt +++ b/tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt @@ -1,7 +1,7 @@ // Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']" [Register ("java/code/IMyInterface", "", "java.code.IMyInterfaceInvoker")] public partial interface IMyInterface : IJavaObject, IJavaPeerable { - static new readonly JniPeerMembers _members = new XAPeerMembers ("java/code/IMyInterface", typeof (IMyInterface)); + static new readonly JniPeerMembers _members = new XAPeerMembers ("java/code/IMyInterface", typeof (IMyInterface), isInterface: true); static Delegate cb_get_Value; #pragma warning disable 0169