Default/Static interface method desugar remapping not working in .NET 7 #7663

Activity

1. radimitrov

   added

   Area: App RuntimeIssues in `libmonodroid.so`.Issues in `libmonodroid.so`.

   needs-triageIssues that need to be assigned.Issues that need to be assigned.

   on Jan 4, 2023
2. radimitrov

   assigned

   grendello

   on Jan 4, 2023
3. grendello

   assigned

   jonpryor

   on Jan 4, 2023
4. grendello

   added

   Area: BindingsIssues in Java Library Binding projects.Issues in Java Library Binding projects.

   and removed

   Area: App RuntimeIssues in `libmonodroid.so`.Issues in `libmonodroid.so`.

   needs-triageIssues that need to be assigned.Issues that need to be assigned.

   on Jan 4, 2023
5. grendello

   assigned

   jpobst

   on Jan 4, 2023
6. jpobst commented on Jan 9, 2023

   

   jpobst

   on Jan 9, 2023

   Contributor

   More actions

   Simplified test case
   AndroidApp2.zip
7. jpobst commented on Jan 9, 2023

   

   jpobst

   on Jan 9, 2023

   Contributor

   More actions

   Default and static methods are not supported by Android before API Level 24:
   #4574

   We thought we had worked around it for .NET 7, but there seems to be a bug preventing it from working like we expected. We will continue to investigate that.

   The workaround is to target API-24+ by setting this in your project file:

   <SupportedOSPlatformVersion>24</SupportedOSPlatformVersion>
8. jpobst

   changed the title [-]Debug build JNI crash on bindings of static interface methods[/-] [+]Default/Static interface method desugar remapping not working in .NET 7[/+] on Jan 9, 2023
9. jpobst

   unassigned

   grendello

   and

   jpobst

   on Jan 9, 2023
10. jonpryor commented on Jan 9, 2023

    

    jonpryor

    on Jan 9, 2023

    Contributor

    More actions

    On the investigation front, consider AndroidApp2.zip (earlier comment): it has a static method invocation on an interface:

    var val = IStaticMethodsInterface.Value;

    which results in a crash:

    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)
    

    However, if I manually do what the binding should be doing -- I still need to verify + review that the binding is doing this -- then it works:

    static void TryInvokeStaticInterfaceMethod()
    {
    	Console.WriteLine ("# jonp: trying to get class `com/xamarin/android/StaticMethodsInterface$-CC`…");
    	var t = new Java.Interop.JniType ("com/xamarin/android/StaticMethodsInterface$-CC");
    	Console.WriteLine ($"# jonp: t={t}");
    
    	Console.WriteLine ("# jonp: trying to get static method `getValue()I`…");
    	var m = t.GetStaticMethod ("getValue", "()I");
    	Console.WriteLine ($"# jonp: m={m}");
    
    	Console.WriteLine ("# jonp: Invoking static method `getValue()I`…");
    	var r = Java.Interop.JniEnvironment.StaticMethods.CallStaticIntMethod (t.PeerReference, m);
    	Console.WriteLine ($"# jonp: r={r}");
    }

    Invoking TryInvokeStaticInterfaceMethod() works as expected, with no crash:

    # jonp: trying to get class `com/xamarin/android/StaticMethodsInterface$-CC`…
    # jonp: t=JniType(Name='com/xamarin/android/StaticMethodsInterface$-CC' PeerReference=0x1cb6/G)
    # jonp: trying to get static method `getValue()I`…
    # jonp: m=JniMethodInfo(ID=0x1c3)
    # jonp: Invoking static method `getValue()I`…
    # jonp: r=3
    

    TODO: figure out why the current binding infrastructure isn't doing what the above manual binding does.
11. jonpryor commented on Jan 9, 2023

    

    jonpryor

    on Jan 9, 2023

    Contributor

    More actions

    @jonpryor wrote:

    TODO: figure out why the current binding infrastructure isn't doing what the above manual binding does.

    The answer is because we don't pass the jclass corresponding to the type where we found the method; see also:

    • https://github.com/xamarin/java.interop/blob/main/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs#L70-L91
    • https://github.com/xamarin/java.interop/blob/main/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs#L127

    What we do is more like:

    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");
    
    // Note: needs to use `to.PeerReference`, *not* `from.PeerReference`
    var r = Java.Interop.JniEnvironment.StaticMethods.CallStaticIntMethod (from.PeerReference, m);
12. jonpryor commented on Jan 10, 2023

    

    jonpryor

    on Jan 10, 2023

    Contributor

    More actions

    On the investigation front, consider AndroidApp2.zip (earlier comment): it has a default interface method invocation:

    var foo = new MyDefaultClass ();
    var val2 = (foo as IDefaultMethodsInterface).Bar;

    which results in a crash:

    Java.Lang.AbstractMethodError: abstract method "int com.xamarin.android.DefaultMethodsInterface.getBar()"
       at Java.Interop.JniEnvironment.InstanceMethods.CallNonvirtualIntMethod(JniObjectReference instance, JniObjectReference type, JniMethodInfo method, JniArgumentValue* args)
       at Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeVirtualInt32Method(String encodedMember, IJavaPeerable self, JniArgumentValue* parameters)
       at Com.Xamarin.Android.IDefaultMethodsInterface.get_Bar()
       at AndroidApp2.MainActivity.OnCreate(Bundle savedInstanceState)
       at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_savedInstanceState)
       at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0)
      --- End of managed Java.Lang.AbstractMethodError stack trace ---
    java.lang.AbstractMethodError: abstract method "int com.xamarin.android.DefaultMethodsInterface.getBar()"
        at crc641149b9fe658fbe8e.MainActivity.n_onCreate(Native Method)
        at crc641149b9fe658fbe8e.MainActivity.onCreate(MainActivity.java:30)
        …
    

    This is also due to desugaring, which is enabled when $(SupportedOSPlatformVersion) is less than 24. When desugaring is not enabled, default interface method invocation works as expected, and it works by performing a non-virtual method invocation on the method declared in the interface.

    When desugaring is enabled, things fail because nothing is where we expect it; if you use dexdump (in the Android SDK, in e.g. build-tools/31.0.3) on classes.dex:

    % unzip bin/Debug/net7.0-android33.0/com.companyname.AndroidApp2-Signed.apk classes.dex
    % $ANDROID_SDK_PATH/build-tools/31.0.3/dexdump classes.dex
    

    and look for the getBar method, you see two things:

    1. An abstract declaration is on DefaultMethodsInterface:

        Class #6            -
          Class descriptor  : 'Lcom/xamarin/android/DefaultMethodsInterface;'
          …
          Virtual methods   -
            …
            #1              : (in Lcom/xamarin/android/DefaultMethodsInterface;)
              name          : 'getBar'
              type          : '()I'
              access        : 0x0401 (PUBLIC ABSTRACT)
              code          : (none)
       

    2. The non-abstract method containing the default interface method body is in DefaultMethodsInterface$-CC (?!) in a method with a $default$ prefix (?!), and "this" becomes a parameter:

        Class #5            -
          Class descriptor  : 'Lcom/xamarin/android/DefaultMethodsInterface$-CC;'
          Access flags      : 0x1011 (PUBLIC FINAL SYNTHETIC)
          …
          Direct methods    -
            …
            #1              : (in Lcom/xamarin/android/DefaultMethodsInterface$-CC;)
              name          : '$default$getBar'
              type          : '(Lcom/xamarin/android/DefaultMethodsInterface;)I'
              access        : 0x0009 (PUBLIC STATIC)
              code          -
       

    Rephrased, when attempting to non-virtually invoke DefaultMethodsInterface.getBar():

    DefaultMethodsInterface instance = …
    int b = instance.getBar();

    when Desugaring is present, we need to instead invoke a static method and reorder the parameters, a'la:

    DefaultMethodsInterface instance = …
    
    int b = DefaultMethodsInterface$-CC.getBar(instance);

    This actually feels reminiscent of the "ClassRewrites[*].Methods[*] with MakeStatic: true" support in dotnet/java-interop@1f27ab5
13. jonpryor

    added 4 commits that reference this issue on Jan 20, 2023

    [Java.Interop] Support Desugar + interface static methods

    ...

    fb3e241

    [Java.Interop] Support Desugar + interface static methods

    ...

    2b32ccd

    [Java.Interop] Support Desugar + interface static methods

    ...

    ed3326e

    [Java.Interop] Support Desugar + interface static methods

    ...

    3b68eb2
14. jonpryor

    added 2 commits that reference this issue on Feb 16, 2023

    [Java.Interop] Support Desugar + interface static methods

    ...

    205040b

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

    ...

    bbaeda6
15. jpobst commented on Apr 17, 2024

    

    jpobst

    on Apr 17, 2024

    Contributor

    More actions

    We believe desugar remapping is now fully supported in net8.0-android and beyond.

    dotnet/java-interop#1077
16. jpobst

    closed this as completedon Apr 17, 2024
17. github-actions

    locked and limited conversation to collaborators on May 18, 2024