Skip to content

Commit

Permalink
[jcw-gen] Do not register static methods on an interface. (#745)
Browse files Browse the repository at this point in the history
Context: 855b7e9
Context: dotnet/android#5251
Context: dotnet/android#5262

When an Java interface contains a `static` method in API-30+, we use
C# 8.0's Default Interface Methods support to place corresponding
`static` methods in the C# interface binding:

	// Java
	public /* partial */ interface Comparator<T> {}
	    int compare(T o1, T o2);

	    static <T>
	    Comparator<T> comparingDouble(ToDoubleFunction<? super T> keyExtractor) {…}

	    // …
	}

	// C# binding
	public partial interface IComparator {
	    [Register ("compare", "(Ljava/lang/Object;Ljava/lang/Object;)I", "GetCompare_Ljava_lang_Object_Ljava_lang_Object_Handler:Java.Util.IComparatorInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
	    int Compare (Java.Lang.Object? o1, Java.Lang.Object? o2);

	    [Register ("comparingDouble", "(Ljava/util/function/ToDoubleFunction;)Ljava/util/Comparator;", "", ApiSince = 24)]
	    [global::Java.Interop.JavaTypeParameters (new string [] {"T"})]
	    public static unsafe Java.Util.IComparator? ComparingDouble (Java.Util.Functions.IToDoubleFunction? keyExtractor) {…}
	}

This works until you (1) attempt to implement the C# interface:

	public partial class CompareSizesByArea : Java.Lang.Object, Java.Util.IComparator
	{
	    public int Compare (Java.Lang.Object lhs, Java.Lang.Object rhs) {…}
	}

and then (2) *instantiate* that class:

	var comparator = new CompareSizesByArea();

The result is an `ArgumentException`:

	System.ArgumentException: Couldn't bind to method ''.
	  at System.Delegate.GetCandidateMethod (System.Type type, System.Type target, System.String method, System.Reflection.BindingFlags bflags, System.Boolean ignoreCase, System.Boolean throwOnBindFailure)
	  at System.Delegate.CreateDelegate (System.Type type, System.Type target, System.String method, System.Boolean ignoreCase, System.Boolean throwOnBindFailure)
	  at System.Delegate.CreateDelegate (System.Type type, System.Type target, System.String method)
	  at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType nativeClass, System.Type type, System.String methods)
	  at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len)
	  /* missing stack entries */
	  at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_alloc_object(intptr,intptr&,intptr)
	  at Java.Interop.JniEnvironment+Object.AllocObject (Java.Interop.JniObjectReference type)
	  at Java.Interop.JniType.AllocObject ()
	  at Java.Interop.JniPeerMembers+JniInstanceMethods.StartCreateInstance (System.String constructorSignature, System.Type declaringType, Java.Interop.JniArgumentValue* parameters)
	  at Java.Lang.Object..ctor ()
	  at App16.MainActivity+CompareSizesByArea..ctor ()

Why this fails is because of the intermediate Java Callable Wrapper,
which emits a method override for every method in the interface,
*including `static` methods*:

	// Java callable wrapper
	public /* partial */ class CompareSizesByArea extends java.lang.Object, implements java.util.Comparator {
	    public static final String __md_methods;
	    static {
	        __md_methods =
	            "n_compare:(Ljava/lang/Object;Ljava/lang/Object;)I:GetCompare_Ljava_lang_Object_Ljava_lang_Object_Handler:Java.Util.IComparatorInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null\n" +
	            … +
	            "n_comparingDouble:(Ljava/util/function/ToDoubleFunction;)Ljava/util/Comparator;:\n" +
	            …
	        mono.android.Runtime.register ("CompareSizesByArea, …", CompareSizesByArea.class, __md_methods);
	    }

	    public int compare (java.lang.Object p0, java.lang.Object p1)
	    {
	        return n_compare (p0, p1);
	    }

	    private native int n_compare (java.lang.Object p0, java.lang.Object p1);


	    public java.util.Comparator comparingDouble (java.util.function.ToDoubleFunction p0)
	    {
	        return n_comparingDouble (p0);
	    }

	    private native java.util.Comparator n_comparingDouble (java.util.function.ToDoubleFunction p0);
	}

(Note: The `/* missing stack entries */` line (above) is from the
Java-side `CompareSizesByArea` static constructor execution, which
calls `Runtime.register()`, which in turn calls
`JNIEnv.RegisterJniNatives()`.)

The `Runtime.register()` invocation eventually hits
[`AndroidTypeManager.RegisterNativeMembers()`][0], which "splits"
`__md_methods` into lines, and each line into `:`-separated values.
One of those values is expected to be a "connector method", obtainable
via Reflection.  In the case of `n_comparingDouble`, the connector
method *is not specified*: note that the line ends with `:`, unlike
the entry for `n_compare`, which lists `GetCompare_…`.

This "missing" connector method is the cause of the
`ArgumentException`.

Fix the problem by *skipping* `static` methods found from interfaces
when emitting Java Callable Wrappers.  This ensures that `__md_methods`
won't contain entries with missing "connector" methods.

An integration test is part of dotnet/android#5262, as we're
not currently able to use C#8 Default Interface Methods in a `net472`
target framework for unit testing within the xamarin/Java.Interop repo.

[0]: https://github.com/xamarin/xamarin-android/blob/15b37758e9d65c82aebe1a327747b285f38ce791/src/Mono.Android/Android.Runtime/AndroidRuntime.cs#L394-L413
  • Loading branch information
jpobst committed Nov 10, 2020
1 parent 54ba3c3 commit 99897b2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ void AddNestedTypes (TypeDefinition type)
return d;
})
.Where (d => GetRegisterAttributes (d).Any ())
.SelectMany (d => d.Methods)) {
.SelectMany (d => d.Methods)
.Where (m => !m.IsStatic)) {
AddMethod (imethod, imethod);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ extends java.lang.Object
__md_methods =
""n_GetInstance:()Lcrc64197ae30a36756915/ExportsMembers;:__export__\n"" +
""n_GetValue:()Ljava/lang/String;:__export__\n"" +
""n_staticMethodNotMangled:()V:__export__\n"" +
""n_methodNamesNotMangled:()V:__export__\n"" +
""n_CompletelyDifferentName:(Ljava/lang/String;I)Ljava/lang/String;:__export__\n"" +
""n_methodThatThrows:()V:__export__\n"" +
Expand Down Expand Up @@ -218,6 +219,14 @@ public java.lang.String GetValue ()
private native java.lang.String n_GetValue ();
public static void staticMethodNotMangled ()
{
n_staticMethodNotMangled ();
}
private static native void n_staticMethodNotMangled ();
public void methodNamesNotMangled ()
{
n_methodNamesNotMangled ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,11 @@ public string GetValue ()
return "value";
}

[Export]
public static void staticMethodNotMangled ()
{
}

[Export]
public void methodNamesNotMangled ()
{
Expand Down

0 comments on commit 99897b2

Please sign in to comment.