[Java.Interop] Support Desugar + interface static methods (#1077)

Context: 1f27ab5
Context: dotnet/android@f6f11a5
Context: dotnet/android#7663 (comment)
Commit 1f27ab5 added partial support for [desugaring][0], which
rewrites Java bytecode so that [default interface methods][1] and
[static methods in interfaces][2] can be supported on pre-Android 7.0
(API-24) devices, as the pre-API-24 ART runtime does not directly
support those bytecode constructs.
The hope was that commit 1f27ab5 in concert with
dotnet/android@f6f11a5a would allow static methods on
interfaces to work, by having
`Java.Interop.JniPeerMembers.JniStaticMethods.GetMethodInfo()`
call `JniRuntime.JniTypeManager.GetStaticMethodFallbackTypes()`.
xamarin/xamarin-android would then override
`GetStaticMethodFallbackTypes()`, allowing `GetMethodInfo()` to
instead resolve the static method from the fallback type, allowing
the static method invocation to work.
> TODO: Update xamarin-android to override
> `GetStaticMethodFallbackTypes()`, to return
> `$"{jniSimpleReference}$-CC"`.
What *actually* happened? Not enough testing happened, such that
when it was *actually* attempted, it blew up bigly:
JNI DETECTED ERROR IN APPLICATION: can't call static int com.xamarin.android.StaticMethodsInterface$-CC.getValue() with class java.lang.Class<com.xamarin.android.StaticMethodsInterface>
in call to CallStaticIntMethodA
from void crc641149b9fe658fbe8e.MainActivity.n_onCreate(android.os.Bundle)
Oops.
What went wrong?
The problem is that [`JNIEnv::CallStatic*Method()`][3] requires that
we provide the `jclass clazz` value for the type that declares the
static method, but we're providing the wrong class!
Specifically, consider this Java interface:
public interface StaticMethodsInterface {
static int getValue() {
return 3;
}
}
In a desugared environment, this is transformed into the *pair* of
types:
public interface StaticMethodsInterface {
}
public class StaticMethodsInterface$-CC {
public static int getValue() {
return 3;
}
}
`GetStaticMethodFallbackTypes("StaticMethodsInterface")` returns
`StaticMethodsInterface$-CC`, allowing `GetMethodInfo()` to lookup
`StaticMethodsInterface$-CC` and resolve the method
`getValue.()I`. However, when
`JniPeerMembers.JniStaticMethods.Invoke*Method()` is later invoked,
the `jclass` value for `StaticMethodsInterface` is provided, *not*
the value for `StaticMethodsInterface$-CC`. It's as if we do:
var from = new Java.Interop.JniType ("com/xamarin/android/StaticMethodsInterface");
var to = new Java.Interop.JniType ("com/xamarin/android/StaticMethodsInterface$-CC");
var m = to.GetStaticMethod ("getValue", "()I");
var r = Java.Interop.JniEnvironment.StaticMethods.CallStaticIntMethod (from.PeerReference, m);
The problem is that we need to use `to.PeerReference`, *not*
`from.PeerReference`!
Fix `GetMethodInfo()` so that when a method is resolved from a
fallback type, the type instance is stored in the
`JniMethodInfo.StaticRedirect` field, and then update the
`Invoke*Method()` methods so that `JniMethodInfo.StaticRedirect` is
passed to `JNIEnv::CallStatic*Method()` if it is set, before
defaulting to `JniPeerMembers.JniPeerType`.
"Optimization opportunity": the approach taken does not attempt to
cache the `JniType` instances which correspond to the fallback types.
Consequently, it is possible that multiple GREFs could be consumed
for the `JniMethodInfo.StaticRedirect` instances, as each instance
will be unique. This was done for expediency, and because @jonpryor
doesn't know if this will be an actual problem in practice.
Unrelatedly, fix the `JniPeerMembers(string, Type, bool)` constructor
so that it participates in type remapping. Without this change,
interface types cannot be renamed by the 1f27ab5 infrastructure.
[0]: https://developer.android.com/studio/write/java8-support#library-desugaring
[1]: https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html
[2]: https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html#static
[3]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#CallStatic_type_Method_routines

Author: jonpryor

src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs

@@ -66,6 +66,18 @@ JniMethodInfo GetMethodInfo (string method, string signature)
 returnMembers.JniPeerType.GetStaticMethod(method,signature);
 }
 
+#pragma warning disable CA1801
+JniTypeGetMethodDeclaringType(JniMethodInfomethod)
+{
+#if NET
+if(method.StaticRedirect!=null){
+returnmethod.StaticRedirect;
+}
+#endif // NET
+returnMembers.JniPeerType;
+}
+#pragma warning restore CA1801
+
 #if NET
 JniMethodInfo?FindInFallbackTypes(stringmethod,stringsignature)
 {
@@ -80,6 +92,8 @@ JniMethodInfo GetMethodInfo (string method, string signature)
 continue;
 }
 if(t.TryGetStaticMethod(method,signature,outvarm)){
+m.StaticRedirect=t;
+t=null;
 returnm;
 }
 }
@@ -94,61 +108,61 @@ JniMethodInfo GetMethodInfo (string method, string signature)
 publicunsafevoidInvokeVoidMethod(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-JniEnvironment.StaticMethods.CallStaticVoidMethod(Members.JniPeerType.PeerReference,m,parameters);
+JniEnvironment.StaticMethods.CallStaticVoidMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafeboolInvokeBooleanMethod(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticBooleanMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticBooleanMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafesbyteInvokeSByteMethod(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticByteMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticByteMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafecharInvokeCharMethod(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticCharMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticCharMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafeshortInvokeInt16Method(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticShortMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticShortMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafeintInvokeInt32Method(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticIntMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticIntMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafelongInvokeInt64Method(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticLongMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticLongMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafefloatInvokeSingleMethod(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticFloatMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticFloatMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafedoubleInvokeDoubleMethod(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticDoubleMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticDoubleMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 
 publicunsafeJniObjectReferenceInvokeObjectMethod(stringencodedMember,JniArgumentValue*parameters)
 {
 varm=GetMethodInfo(encodedMember);
-returnJniEnvironment.StaticMethods.CallStaticObjectMethod(Members.JniPeerType.PeerReference,m,parameters);
+returnJniEnvironment.StaticMethods.CallStaticObjectMethod(GetMethodDeclaringType(m).PeerReference,m,parameters);
 }
 }}
 }

src/Java.Interop/Java.Interop/JniPeerMembers.cs

@@ -12,7 +12,7 @@ public partial class JniPeerMembers {
 privateboolisInterface;
 
 publicJniPeerMembers(stringjniPeerTypeName,TypemanagedPeerType,boolisInterface)
-:this(jniPeerTypeName,managedPeerType,checkManagedPeerType:true,isInterface:isInterface)
+:this(jniPeerTypeName=GetReplacementType(jniPeerTypeName),managedPeerType,checkManagedPeerType:true,isInterface:isInterface)
 {
 }
 

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

@@ -53,14 +53,12 @@
  <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\com\xamarin\interop\RenameClassBase1.java" />
  <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\com\xamarin\interop\RenameClassBase2.java" />
  <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\com\xamarin\interop\RenameClassDerived.java" />
+ <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\com\xamarin\interop\AndroidInterface.java" />
+ <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\com\xamarin\interop\DesugarAndroidInterface$_CC.java" />
  <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\com\xamarin\interop\SelfRegistration.java" />
  <JavaInteropTestJarInclude="$(MSBuildThisFileDirectory)java\com\xamarin\interop\TestType.java" />
  </ItemGroup>
 
- <TargetName="BuildInteropTestJar"BeforeTargets="Build"Inputs="@(JavaInteropTestJar)"Outputs="$(OutputPath)interop-test.jar">
- <MakeDirDirectories="$(IntermediateOutputPath)it-classes" />
- <ExecCommand="&quot;$(JavaCPath)&quot; $(_JavacSourceOptions) -d &quot;$(IntermediateOutputPath)it-classes&quot; -classpath &quot;$(OutputPath)../$(Configuration)/java-interop.jar&quot; @(JavaInteropTestJar->'%(Identity)', ' ')" />
- <ExecCommand="&quot;$(JarPath)&quot; cf &quot;$(OutputPath)interop-test.jar&quot; -C &quot;$(IntermediateOutputPath)it-classes&quot; ." />
- </Target>
+ <ImportProject="Java.Interop-Tests.targets" />
 
 </Project>

tests/Java.Interop-Tests/Java.Interop-Tests.targets

@@ -0,0 +1,21 @@
+<Project>
+
+ <TargetName="BuildInteropTestJar"
+BeforeTargets="Build"
+Inputs="@(JavaInteropTestJar)"
+Outputs="$(OutputPath)interop-test.jar">
+ <MakeDirDirectories="$(IntermediateOutputPath)it-classes" />
+ <ItemGroup>
+ <_SourceInclude="@(JavaInteropTestJar->Replace('%5c', '/'))" />
+ </ItemGroup>
+ <WriteLinesToFile
+File="$(IntermediateOutputPath)_java_sources.txt"
+Lines="@(_Source)"
+Overwrite="True"
+ />
+ <ExecCommand="&quot;$(JavaCPath)&quot; $(_JavacSourceOptions) -d &quot;$(IntermediateOutputPath)it-classes&quot; -classpath &quot;$(OutputPath)../$(Configuration)/java-interop.jar&quot;&quot;@$(IntermediateOutputPath)_java_sources.txt&quot;" />
+ <DeleteFiles="$(IntermediateOutputPath)_java_sources.txt" />
+ <ExecCommand="&quot;$(JarPath)&quot; cf &quot;$(OutputPath)interop-test.jar&quot; -C &quot;$(IntermediateOutputPath)it-classes&quot; ." />
+ </Target>
+
+</Project>

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

@@ -91,8 +91,8 @@ IEnumerable<string> CreateSimpleReferencesEnumerator (Type type)
 // "potentially non-existent" types ensures that we don't throw
 // from places we don't want to internally throw.
 returnnew[]{
-desugarType,
-$"{jniSimpleReference}$-CC"
+$"{desugarType}$_CC",// For JniPeerMembersTests.DesugarInterfaceStaticMethod()
+$"{jniSimpleReference}$-CC",
 };
 }
 

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

@@ -108,6 +108,38 @@ public void ReplaceInstanceMethodWithStaticMethod ()
 // Shouldn't throw; should instead invoke ObjectHelper.getHashCodeHelper(Object)
 o.remappedToStaticHashCode();
 }
+
+#if !__ANDROID__
+// Note: this test looks up a static method from one class, then
+// calls `JNIEnv::CallStaticObjectMethod()` passing in a jclass
+// for a *different* class.
+//
+// This appears to work on Desktop JVM.
+//
+// On Android, this will ABORT the app:
+// JNI DETECTED ERROR IN APPLICATION: can't call static int com.xamarin.interop.DesugarAndroidInterface$_CC.getClassName() with class java.lang.Class<com.xamarin.interop.AndroidInterface>
+// in call to CallStaticObjectMethodA
+//
+// *Fascinating* the differences that can appear between JVM implementations
+[Test]
+publicunsafevoidDoesTheJmethodNeedToMatchDeclaringType()
+{
+variface=newJniType("com/xamarin/interop/AndroidInterface");
+vardesugar=newJniType("com/xamarin/interop/DesugarAndroidInterface$_CC");
+varm=desugar.GetStaticMethod("getClassName","()Ljava/lang/String;");
+
+varr=JniEnvironment.StaticMethods.CallStaticObjectMethod(iface.PeerReference,m,null);
+vars=JniEnvironment.Strings.ToString(refr,JniObjectReferenceOptions.CopyAndDispose);
+Assert.AreEqual("DesugarAndroidInterface$-CC",s);
+}
+#endif // !__ANDROID__
+
+[Test]
+publicvoidDesugarInterfaceStaticMethod()
+{
+vars=IAndroidInterface.getClassName();
+Assert.AreEqual("DesugarAndroidInterface$-CC",s);
+}
 #endif // NET
 }
 
@@ -208,4 +240,19 @@ public override unsafe int hashCode ()
 returnbase.hashCode();
 }
 }
+
+#if NET
+[JniTypeSignature(JniTypeName)]
+interfaceIAndroidInterface:IJavaPeerable{
+internalconststringJniTypeName="com/xamarin/interop/AndroidInterface";
+
+privatestaticJniPeerMembers_members=newJniPeerMembers(JniTypeName,typeof(IAndroidInterface),isInterface:true);
+
+publicstaticunsafestringgetClassName()
+{
+vars=_members.StaticMethods.InvokeObjectMethod("getClassName.()Ljava/lang/String;",null);
+returnJniEnvironment.Strings.ToString(refs,JniObjectReferenceOptions.CopyAndDispose);
+}
+}
+#endif // NET
 }

tests/Java.Interop-Tests/java/com/xamarin/interop/AndroidInterface.java

@@ -0,0 +1,14 @@
+packagecom.xamarin.interop;
+
+// When Android Desugaring is enabled -- the default when targeting API-25 and earlier --
+// certain Java constructs result in Java bytecode rewriting.
+// Interface static methods are *moved* into $-CC types.
+publicinterfaceAndroidInterface {
+
+// When Desugaring is enabled, this is moved to `AndroidInterface$-CC.getClassName()`,
+// and the original `AndroidInterface.getClassName()` *no longer exists*.
+
+// public static String getClassName() {
+// return "AndroidInterface";
+// }
+}

tests/Java.Interop-Tests/java/com/xamarin/interop/DesugarAndroidInterface$_CC.java

@@ -0,0 +1,8 @@
+packagecom.xamarin.interop;
+
+publicclassDesugarAndroidInterface$_CC {
+
+publicstaticStringgetClassName() {
+return"DesugarAndroidInterface$-CC";
+	}
+}