Skip to content

Commit bbaeda6

Browse files
jonpryorjonpryor
authored
[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
1 parent 9e0a469 commit bbaeda6

File tree

8 files changed

+120
-18
lines changed

8 files changed

+120
-18
lines changed

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

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,18 @@ JniMethodInfo GetMethodInfo (string method, string signature)
6666
return Members.JniPeerType.GetStaticMethod (method, signature);
6767
}
6868

69+
#pragma warning disable CA1801
70+
JniType GetMethodDeclaringType (JniMethodInfo method)
71+
{
72+
#if NET
73+
if (method.StaticRedirect != null) {
74+
return method.StaticRedirect;
75+
}
76+
#endif // NET
77+
return Members.JniPeerType;
78+
}
79+
#pragma warning restore CA1801
80+
6981
#if NET
7082
JniMethodInfo? FindInFallbackTypes (string method, string signature)
7183
{
@@ -80,6 +92,8 @@ JniMethodInfo GetMethodInfo (string method, string signature)
8092
continue;
8193
}
8294
if (t.TryGetStaticMethod (method, signature, out var m)) {
95+
m.StaticRedirect = t;
96+
t = null;
8397
return m;
8498
}
8599
}
@@ -94,61 +108,61 @@ JniMethodInfo GetMethodInfo (string method, string signature)
94108
public unsafe void InvokeVoidMethod (string encodedMember, JniArgumentValue* parameters)
95109
{
96110
var m = GetMethodInfo (encodedMember);
97-
JniEnvironment.StaticMethods.CallStaticVoidMethod (Members.JniPeerType.PeerReference, m, parameters);
111+
JniEnvironment.StaticMethods.CallStaticVoidMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
98112
}
99113

100114
public unsafe bool InvokeBooleanMethod (string encodedMember, JniArgumentValue* parameters)
101115
{
102116
var m = GetMethodInfo (encodedMember);
103-
return JniEnvironment.StaticMethods.CallStaticBooleanMethod (Members.JniPeerType.PeerReference, m, parameters);
117+
return JniEnvironment.StaticMethods.CallStaticBooleanMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
104118
}
105119

106120
public unsafe sbyte InvokeSByteMethod (string encodedMember, JniArgumentValue* parameters)
107121
{
108122
var m = GetMethodInfo (encodedMember);
109-
return JniEnvironment.StaticMethods.CallStaticByteMethod (Members.JniPeerType.PeerReference, m, parameters);
123+
return JniEnvironment.StaticMethods.CallStaticByteMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
110124
}
111125

112126
public unsafe char InvokeCharMethod (string encodedMember, JniArgumentValue* parameters)
113127
{
114128
var m = GetMethodInfo (encodedMember);
115-
return JniEnvironment.StaticMethods.CallStaticCharMethod (Members.JniPeerType.PeerReference, m, parameters);
129+
return JniEnvironment.StaticMethods.CallStaticCharMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
116130
}
117131

118132
public unsafe short InvokeInt16Method (string encodedMember, JniArgumentValue* parameters)
119133
{
120134
var m = GetMethodInfo (encodedMember);
121-
return JniEnvironment.StaticMethods.CallStaticShortMethod (Members.JniPeerType.PeerReference, m, parameters);
135+
return JniEnvironment.StaticMethods.CallStaticShortMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
122136
}
123137

124138
public unsafe int InvokeInt32Method (string encodedMember, JniArgumentValue* parameters)
125139
{
126140
var m = GetMethodInfo (encodedMember);
127-
return JniEnvironment.StaticMethods.CallStaticIntMethod (Members.JniPeerType.PeerReference, m, parameters);
141+
return JniEnvironment.StaticMethods.CallStaticIntMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
128142
}
129143

130144
public unsafe long InvokeInt64Method (string encodedMember, JniArgumentValue* parameters)
131145
{
132146
var m = GetMethodInfo (encodedMember);
133-
return JniEnvironment.StaticMethods.CallStaticLongMethod (Members.JniPeerType.PeerReference, m, parameters);
147+
return JniEnvironment.StaticMethods.CallStaticLongMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
134148
}
135149

136150
public unsafe float InvokeSingleMethod (string encodedMember, JniArgumentValue* parameters)
137151
{
138152
var m = GetMethodInfo (encodedMember);
139-
return JniEnvironment.StaticMethods.CallStaticFloatMethod (Members.JniPeerType.PeerReference, m, parameters);
153+
return JniEnvironment.StaticMethods.CallStaticFloatMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
140154
}
141155

142156
public unsafe double InvokeDoubleMethod (string encodedMember, JniArgumentValue* parameters)
143157
{
144158
var m = GetMethodInfo (encodedMember);
145-
return JniEnvironment.StaticMethods.CallStaticDoubleMethod (Members.JniPeerType.PeerReference, m, parameters);
159+
return JniEnvironment.StaticMethods.CallStaticDoubleMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
146160
}
147161

148162
public unsafe JniObjectReference InvokeObjectMethod (string encodedMember, JniArgumentValue* parameters)
149163
{
150164
var m = GetMethodInfo (encodedMember);
151-
return JniEnvironment.StaticMethods.CallStaticObjectMethod (Members.JniPeerType.PeerReference, m, parameters);
165+
return JniEnvironment.StaticMethods.CallStaticObjectMethod (GetMethodDeclaringType (m).PeerReference, m, parameters);
152166
}
153167
}}
154168
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public partial class JniPeerMembers {
1212
private bool isInterface;
1313

1414
public JniPeerMembers (string jniPeerTypeName, Type managedPeerType, bool isInterface)
15-
: this (jniPeerTypeName, managedPeerType, checkManagedPeerType: true, isInterface: isInterface)
15+
: this (jniPeerTypeName = GetReplacementType (jniPeerTypeName), managedPeerType, checkManagedPeerType: true, isInterface: isInterface)
1616
{
1717
}
1818

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,12 @@
5353
<JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\com\xamarin\interop\RenameClassBase1.java" />
5454
<JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\com\xamarin\interop\RenameClassBase2.java" />
5555
<JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\com\xamarin\interop\RenameClassDerived.java" />
56+
<JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\com\xamarin\interop\AndroidInterface.java" />
57+
<JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\com\xamarin\interop\DesugarAndroidInterface$_CC.java" />
5658
<JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\com\xamarin\interop\SelfRegistration.java" />
5759
<JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\com\xamarin\interop\TestType.java" />
5860
</ItemGroup>
5961

60-
<Target Name="BuildInteropTestJar" BeforeTargets="Build" Inputs="@(JavaInteropTestJar)" Outputs="$(OutputPath)interop-test.jar">
61-
<MakeDir Directories="$(IntermediateOutputPath)it-classes" />
62-
<Exec Command="&quot;$(JavaCPath)&quot; $(_JavacSourceOptions) -d &quot;$(IntermediateOutputPath)it-classes&quot; -classpath &quot;$(OutputPath)../$(Configuration)/java-interop.jar&quot; @(JavaInteropTestJar->'%(Identity)', ' ')" />
63-
<Exec Command="&quot;$(JarPath)&quot; cf &quot;$(OutputPath)interop-test.jar&quot; -C &quot;$(IntermediateOutputPath)it-classes&quot; ." />
64-
</Target>
62+
<Import Project="Java.Interop-Tests.targets" />
6563

6664
</Project>

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<Project>
2+
3+
<Target Name="BuildInteropTestJar"
4+
BeforeTargets="Build"
5+
Inputs="@(JavaInteropTestJar)"
6+
Outputs="$(OutputPath)interop-test.jar">
7+
<MakeDir Directories="$(IntermediateOutputPath)it-classes" />
8+
<ItemGroup>
9+
<_Source Include="@(JavaInteropTestJar->Replace('%5c', '/'))" />
10+
</ItemGroup>
11+
<WriteLinesToFile
12+
File="$(IntermediateOutputPath)_java_sources.txt"
13+
Lines="@(_Source)"
14+
Overwrite="True"
15+
/>
16+
<Exec Command="&quot;$(JavaCPath)&quot; $(_JavacSourceOptions) -d &quot;$(IntermediateOutputPath)it-classes&quot; -classpath &quot;$(OutputPath)../$(Configuration)/java-interop.jar&quot; &quot;@$(IntermediateOutputPath)_java_sources.txt&quot;" />
17+
<Delete Files="$(IntermediateOutputPath)_java_sources.txt" />
18+
<Exec Command="&quot;$(JarPath)&quot; cf &quot;$(OutputPath)interop-test.jar&quot; -C &quot;$(IntermediateOutputPath)it-classes&quot; ." />
19+
</Target>
20+
21+
</Project>

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ IEnumerable<string> CreateSimpleReferencesEnumerator (Type type)
9191
// "potentially non-existent" types ensures that we don't throw
9292
// from places we don't want to internally throw.
9393
return new[]{
94-
desugarType,
95-
$"{jniSimpleReference}$-CC"
94+
$"{desugarType}$_CC", // For JniPeerMembersTests.DesugarInterfaceStaticMethod()
95+
$"{jniSimpleReference}$-CC",
9696
};
9797
}
9898

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,38 @@ public void ReplaceInstanceMethodWithStaticMethod ()
108108
// Shouldn't throw; should instead invoke ObjectHelper.getHashCodeHelper(Object)
109109
o.remappedToStaticHashCode ();
110110
}
111+
112+
#if !__ANDROID__
113+
// Note: this test looks up a static method from one class, then
114+
// calls `JNIEnv::CallStaticObjectMethod()` passing in a jclass
115+
// for a *different* class.
116+
//
117+
// This appears to work on Desktop JVM.
118+
//
119+
// On Android, this will ABORT the app:
120+
// 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>
121+
// in call to CallStaticObjectMethodA
122+
//
123+
// *Fascinating* the differences that can appear between JVM implementations
124+
[Test]
125+
public unsafe void DoesTheJmethodNeedToMatchDeclaringType ()
126+
{
127+
var iface = new JniType ("com/xamarin/interop/AndroidInterface");
128+
var desugar = new JniType ("com/xamarin/interop/DesugarAndroidInterface$_CC");
129+
var m = desugar.GetStaticMethod ("getClassName", "()Ljava/lang/String;");
130+
131+
var r = JniEnvironment.StaticMethods.CallStaticObjectMethod (iface.PeerReference, m, null);
132+
var s = JniEnvironment.Strings.ToString (ref r, JniObjectReferenceOptions.CopyAndDispose);
133+
Assert.AreEqual ("DesugarAndroidInterface$-CC", s);
134+
}
135+
#endif // !__ANDROID__
136+
137+
[Test]
138+
public void DesugarInterfaceStaticMethod ()
139+
{
140+
var s = IAndroidInterface.getClassName ();
141+
Assert.AreEqual ("DesugarAndroidInterface$-CC", s);
142+
}
111143
#endif // NET
112144
}
113145

@@ -208,4 +240,19 @@ public override unsafe int hashCode ()
208240
return base.hashCode ();
209241
}
210242
}
243+
244+
#if NET
245+
[JniTypeSignature (JniTypeName)]
246+
interface IAndroidInterface : IJavaPeerable {
247+
internal const string JniTypeName = "com/xamarin/interop/AndroidInterface";
248+
249+
private static JniPeerMembers _members = new JniPeerMembers (JniTypeName, typeof (IAndroidInterface), isInterface: true);
250+
251+
public static unsafe string getClassName ()
252+
{
253+
var s = _members.StaticMethods.InvokeObjectMethod ("getClassName.()Ljava/lang/String;", null);
254+
return JniEnvironment.Strings.ToString (ref s, JniObjectReferenceOptions.CopyAndDispose);
255+
}
256+
}
257+
#endif // NET
211258
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package com.xamarin.interop;
2+
3+
// When Android Desugaring is enabled -- the default when targeting API-25 and earlier --
4+
// certain Java constructs result in Java bytecode rewriting.
5+
// Interface static methods are *moved* into $-CC types.
6+
public interface AndroidInterface {
7+
8+
// When Desugaring is enabled, this is moved to `AndroidInterface$-CC.getClassName()`,
9+
// and the original `AndroidInterface.getClassName()` *no longer exists*.
10+
11+
// public static String getClassName() {
12+
// return "AndroidInterface";
13+
// }
14+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package com.xamarin.interop;
2+
3+
public class DesugarAndroidInterface$_CC {
4+
5+
public static String getClassName() {
6+
return "DesugarAndroidInterface$-CC";
7+
}
8+
}

0 commit comments

Comments
 (0)