From 2f21171abfa0493f7679dec333671f0d79a80f6a Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 15 Feb 2023 19:42:22 -0500 Subject: [PATCH 1/3] Try xamarin/java.interop#1077 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/java.interop/pull/1077 Context: https://github.com/xamarin/java.interop/commit/1f27ab552d03aeb74cdc6f8985fcffbfdb9a7ddf Context: f6f11a5a797cd8602854942dccb0f2b1db57c473 [Desugaring][0] is the process of rewriting Java bytecode so that Java 8+ constructs can be used on Android pre-7.0 (API-24), as API-24 is the Android version which added native support for Java 8 features such as [interface default methods][1]. One of the implications of desugaring is that methods can "move"; consider this Java interface: package example; public interface StaticMethodsInterface { static int getValue() { return 3; } } Java.Interop bindings will attempt to invoke the `getValue().I` method on the type `example/StaticMethodsInterface`: public partial interface IStaticMethodsInterface : IJavaObject, IJavaPeerable { private static readonly JniPeerMembers _members = new XAPeerMembers ("example/StaticMethodsInterface", typeof (IStaticMethodsInterface), isInterface: true); static unsafe int Value { get { const string __id = "getValue.()I"; var __rm = _members.StaticMethods.InvokeInt32Method (__id, null); return __rm; } } } The problem is that, after Desugaring, the Java side *actually* looks like this: package example; public interface StaticMethodsInterface { } public class StaticMethodsInterface$-CC { public static int getValue() {return 3;} } Commits xamarin/java.interop@1f27ab55 and f6f11a5a added partial runtime support for this scenario via `AndroidTypeManager.GetStaticMethodFallbackTypesCore()`, which would attempt to lookup types with a `$-CC` suffix. While this was a good start, it wasn't ever actually *tested* end-to-end. Consequently, instead of *working*, this would instead cause the process to *abort*: JNI DETECTED ERROR IN APPLICATION: can't call static int example.StaticMethodsInterface$-CC.getValue() with class java.lang.Class in call to CallStaticIntMethodA from void crc….MainActivity.n_onCreate(android.os.Bundle) Oops. xamarin/java.interop#1077 improves our runtime support for invoking *`static`* methods on interfaces. Add a new `XASdkDeployTests.SupportDesugaringStaticInterfaceMethods()` test which performs an on-device, end-to-end invocation of a static method on a Java interface, to ensure that things *actually* work. *Note*: if `$(SupportedOSPlatformVersion)` is 24 or higher, this test will work even without xamarin/java.interop#1077, as Desugaring is *disabled* in that case. [0]: https://developer.android.com/studio/write/java8-support#library-desugaring [1]: https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html --- .gitmodules | 4 +- build-tools/scripts/Jar.targets | 9 +++- external/Java.Interop | 2 +- .../Utilities/ResourceData.cs | 2 + .../MSBuildDeviceIntegration.csproj | 3 ++ .../Resources/StaticMethodsInterface.java | 7 +++ .../Tests/XASdkDeployTests.cs | 53 +++++++++++++++++++ 7 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 tests/MSBuildDeviceIntegration/Resources/StaticMethodsInterface.java diff --git a/.gitmodules b/.gitmodules index f8838a54d10..5565144196f 100644 --- a/.gitmodules +++ b/.gitmodules @@ -12,8 +12,8 @@ branch = main [submodule "external/Java.Interop"] path = external/Java.Interop - url = https://github.com/xamarin/java.interop.git - branch = main + url = https://github.com/jonpryor/java.interop.git + branch = jonp-use-right-class-for-remapped-static-method-invocations [submodule "external/lz4"] path = external/lz4 url = https://github.com/lz4/lz4.git diff --git a/build-tools/scripts/Jar.targets b/build-tools/scripts/Jar.targets index 75a0c533218..477afd8612c 100644 --- a/build-tools/scripts/Jar.targets +++ b/build-tools/scripts/Jar.targets @@ -35,9 +35,16 @@ <_DestDir>$(IntermediateOutputPath)__CreateTestJarFile-bin <_AndroidJar>-bootclasspath "$(AndroidSdkDirectory)\platforms\android-$(_AndroidApiLevelName)\android.jar" <_CP>-cp "$(_JavaInteropJarPath)" + <_JavacFilesResponse>$(IntermediateOutputPath)__javac_response.txt + - + + javaSourceTestInterface = new Lazy (() => GetResourceData ("JavaSourceTestInterface.java")); static Lazy remapActivityJava = new Lazy (() => GetResourceData ("RemapActivity.java")); static Lazy remapActivityXml = new Lazy (() => GetResourceData ("RemapActivity.xml")); + static Lazy idmStaticMethodsInterface = new Lazy (() => GetResourceData ("StaticMethodsInterface.java")); public static byte[] JavaSourceJarTestJar => javaSourceJarTestJar.Value; public static byte[] JavaSourceJarTestSourcesJar => javaSourceJarTestSourcesJar.Value; @@ -34,6 +35,7 @@ static class ResourceData public static string JavaSourceTestInterface => Encoding.ASCII.GetString (javaSourceTestInterface.Value); public static string RemapActivityJava => Encoding.UTF8.GetString (remapActivityJava.Value); public static string RemapActivityXml => Encoding.UTF8.GetString (remapActivityXml.Value); + public static string IdmStaticMethodsInterface => Encoding.UTF8.GetString (idmStaticMethodsInterface.Value); static byte[] GetResourceData (string name) { diff --git a/tests/MSBuildDeviceIntegration/MSBuildDeviceIntegration.csproj b/tests/MSBuildDeviceIntegration/MSBuildDeviceIntegration.csproj index d6bb578c27b..cf067e4e5b5 100644 --- a/tests/MSBuildDeviceIntegration/MSBuildDeviceIntegration.csproj +++ b/tests/MSBuildDeviceIntegration/MSBuildDeviceIntegration.csproj @@ -31,6 +31,9 @@ %(FileName)%(Extension) + + %(FileName)%(Extension) + diff --git a/tests/MSBuildDeviceIntegration/Resources/StaticMethodsInterface.java b/tests/MSBuildDeviceIntegration/Resources/StaticMethodsInterface.java new file mode 100644 index 00000000000..1b8e157b758 --- /dev/null +++ b/tests/MSBuildDeviceIntegration/Resources/StaticMethodsInterface.java @@ -0,0 +1,7 @@ +package example; + +public interface StaticMethodsInterface { + static int getValue() { + return 3; + } +} diff --git a/tests/MSBuildDeviceIntegration/Tests/XASdkDeployTests.cs b/tests/MSBuildDeviceIntegration/Tests/XASdkDeployTests.cs index 68eb6d66170..f97e3e7d85b 100644 --- a/tests/MSBuildDeviceIntegration/Tests/XASdkDeployTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/XASdkDeployTests.cs @@ -155,6 +155,59 @@ public void TypeAndMemberRemapping ([Values (false, true)] bool isRelease) ); } + [Test] + public void SupportDesugaringStaticInterfaceMethods () + { + AssertHasDevices (); + if (!Builder.UseDotNet) { + Assert.Ignore ("Skipping. Test not relevant under Classic."); + } + + var proj = new XASdkProject () { + IsRelease = true, + OtherBuildItems = { + new AndroidItem.AndroidJavaSource ("StaticMethodsInterface.java") { + Encoding = new UTF8Encoding (encoderShouldEmitUTF8Identifier: false), + TextContent = () => ResourceData.IdmStaticMethodsInterface, + Metadata = { + { "Bind", "True" }, + }, + }, + }, + }; + + // Note: To properly test, Desugaring must be *enabled*, which requires that + // `$(SupportedOSPlatformVersion)` be *less than* 23. 21 is currently the default, + // but set this explicitly anyway just so that this implicit requirement is explicit. + proj.SetProperty (proj.ReleaseProperties, "SupportedOSPlatformVersion", "21"); + + proj.MainActivity = proj.DefaultMainActivity.Replace ("//${AFTER_ONCREATE}", @" + Console.WriteLine ($""# jonp static interface default method invocation; IStaticMethodsInterface.Value={Example.IStaticMethodsInterface.Value}""); +"); + proj.SetRuntimeIdentifier (DeviceAbi); + var relativeProjDir = Path.Combine ("temp", TestName); + var fullProjDir = Path.Combine (Root, relativeProjDir); + TestOutputDirectories [TestContext.CurrentContext.Test.ID] = fullProjDir; + var files = proj.Save (); + proj.Populate (relativeProjDir, files); + proj.CopyNuGetConfig (relativeProjDir); + var dotnet = new DotNetCLI (proj, Path.Combine (fullProjDir, proj.ProjectFilePath)); + + Assert.IsTrue (dotnet.Build (), "`dotnet build` should succeed"); + Assert.IsTrue (dotnet.Run (), "`dotnet run` should succeed"); + + bool didLaunch = WaitForActivityToStart (proj.PackageName, "MainActivity", + Path.Combine (fullProjDir, "logcat.log")); + Assert.IsTrue (didLaunch, "MainActivity should have launched!"); + var logcatOutput = File.ReadAllText (Path.Combine (fullProjDir, "logcat.log")); + + StringAssert.Contains ( + "IStaticMethodsInterface.Value=3", + logcatOutput, + "Was IStaticMethodsInterface.Value executed?" + ); + } + [Test] [Category ("Debugger"), Category ("Node-4")] public void DotNetDebug ([Values("net6.0-android", "net7.0-android")] string targetFramework) From df9401a6e62b66e4d3841c932fb9fd502ac597ce Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 21 Feb 2023 18:55:25 -0500 Subject: [PATCH 2/3] Remap AndroidInterface to DesugarAndroidInterface$_CC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/xamarin-android/pull/7799#issuecomment-1439020631 `DesugarInterfaceStaticMethod()` (from Java.Interop) fails: Java.Lang.NoSuchMethodError : no static method "Lcom/xamarin/interop/AndroidInterface;.getClassName()Ljava/lang/String;" at Java.Interop.JniEnvironment.StaticMethods.GetStaticMethodID(JniObjectReference , String , String ) at Java.Interop.JniType.GetStaticMethod(String , String ) at Java.Interop.JniPeerMembers.JniStaticMethods.GetMethodInfo(String , String ) at Java.Interop.JniPeerMembers.JniStaticMethods.GetMethodInfo(String ) at Java.Interop.JniPeerMembers.JniStaticMethods.InvokeObjectMethod(String , JniArgumentValue* ) at Java.InteropTests.IAndroidInterface.getClassName() at Java.InteropTests.JniPeerMembersTests.DesugarInterfaceStaticMethod() at System.Reflection.MethodInvoker.InterpretedInvoke(Object , IntPtr* ) at System.Reflection.MethodInvoker.Invoke(Object , IntPtr* , BindingFlags ) --- End of managed Java.Lang.NoSuchMethodError stack trace --- java.lang.NoSuchMethodError: no static method "Lcom/xamarin/interop/AndroidInterface;.getClassName()Ljava/lang/String;" at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method) at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:27) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2189) This is because `AndroidInterface` isn't desugared, so there is no `AndroidInterface$-CC` type. `AndroidInterface` isn't desugared, because it's *empty*. There are thus two ways to make `DesugarInterfaceStaticMethod()` work: 1. Provide an Android-specific `AndroidInterface` type, which contains a `static` interface method. This will cause the type to be desugared, creating the `AndroidInterface$-CC` type, and the test should thus work. 2. Use type remapping (f6f11a5a) to remap `AndroidInterface$-CC` to `DesugarAndroidInterface$_CC`. (1) is (somewhat?) "duplicative" of the new test `SupportDesugaringStaticInterfaceMethods()`. We could do that, but… ¯\_(ツ)_/¯ Which leaves (2), which is "more interesting". It's more interesting in part because it also presents a problem! `AndroidRuntime.GetStaticMethodFallbackTypesCore()` doesn't support/use type remapping! Meaning that even if we have a `` to map `AndroidInterface` to `DesugarAndroidInterface$_CC`, that remapping won't be used! 🧐 Update `AndroidRuntime.GetStaticMethodFallbackTypesCore()` so that the returned types are *after* calling `AndroidRuntime.GetReplacementTypeCore()`, which looks up `` values. This (should?) allow use to remap `AndroidInterface` with `DesugarAndroidInterface$_CC`, allowing the `DesugarInterfaceStaticMethod()` test to pass. 🤞 --- src/Mono.Android/Android.Runtime/AndroidRuntime.cs | 7 +++++-- .../Runtime-Microsoft.Android.Sdk/Remaps.xml | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs index fa98adf6337..e819369fef9 100644 --- a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs +++ b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs @@ -314,9 +314,12 @@ protected override IEnumerable GetSimpleReferences (Type type) desugarType.Append ("Desugar").Append (name); } + var typeWithPrefix = desugarType.ToString (); + var typeWithSuffix = $"{jniSimpleReference}$-CC"; + return new[]{ - desugarType.ToString (), - $"{jniSimpleReference}$-CC" + GetReplacementTypeCore (typeWithPrefix) ?? typeWithPrefix, + GetReplacementTypeCore (typeWithSuffix) ?? typeWithSuffix, }; } diff --git a/tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Remaps.xml b/tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Remaps.xml index 6238cdc346a..5324e8e189f 100644 --- a/tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Remaps.xml +++ b/tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Remaps.xml @@ -2,6 +2,9 @@ + Date: Wed, 22 Feb 2023 12:22:23 -0500 Subject: [PATCH 3/3] Bump to xamarin/Java.Interop/main@bbaeda6f Changes: https://github.com/xamarin/java.interop/compare/9e0a4690adb71c04cd88a5b54bea3c3f8da73cc0...bbaeda6f698369bdd48bfc2dd1a32a41c9d23229 * xamarin/java.interop@bbaeda6f: [Java.Interop] Support Desugar + interface static methods (xamarin/java.interop#1077) --- .gitmodules | 4 ++-- external/Java.Interop | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitmodules b/.gitmodules index 5565144196f..f8838a54d10 100644 --- a/.gitmodules +++ b/.gitmodules @@ -12,8 +12,8 @@ branch = main [submodule "external/Java.Interop"] path = external/Java.Interop - url = https://github.com/jonpryor/java.interop.git - branch = jonp-use-right-class-for-remapped-static-method-invocations + url = https://github.com/xamarin/java.interop.git + branch = main [submodule "external/lz4"] path = external/lz4 url = https://github.com/lz4/lz4.git diff --git a/external/Java.Interop b/external/Java.Interop index 205040b6c04..bbaeda6f698 160000 --- a/external/Java.Interop +++ b/external/Java.Interop @@ -1 +1 @@ -Subproject commit 205040b6c049ae7d42f30c67d3f4ac65a22fb77f +Subproject commit bbaeda6f698369bdd48bfc2dd1a32a41c9d23229