Skip to content

Commit

Permalink
[Java.Interop, generator] Fix GetPeerMembers for interfaces (#486)
Browse files Browse the repository at this point in the history
Context: dotnet/android#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.
  • Loading branch information
jpobst authored and jonpryor committed Aug 29, 2019
1 parent 75b1189 commit f0c28a4
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 15 deletions.
1 change: 1 addition & 0 deletions gendarme-ignore.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 12 additions & 3 deletions src/Java.Interop/Java.Interop/JniPeerMembers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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}");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down

0 comments on commit f0c28a4

Please sign in to comment.