Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
radimitrov opened this issue Jan 4, 2023 · 6 comments
Closed
Assignees
Labels
Area: Bindings Issues in Java Library Binding projects.

Comments

@radimitrov
Copy link

Android application type

Android for .NET (net6.0-android, etc.)

Affected platform version

VS 2022, net7.0-android

Description

App crashes at runtime for debug build with a JNI error related to default/static Java interface method binding. Doesn't matter if the debugger is attached. Might be related to #7125 but reverting to the old binding behaviour with <AndroidBoundInterfacesContainStaticAndDefaultInterfaceMethods>false</AndroidBoundInterfacesContainStaticAndDefaultInterfaceMethods> didn't help. Same for true value.

Steps to Reproduce

Bind any Java library with a default static interface method.

Did you find any workaround?

Release build

Relevant log output

12-16 22:24:43.893 18210 18210 F DEBUG   : signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
12-16 22:24:43.893 18210 18210 F DEBUG   : Abort message: 'java_vm_ext.cc:542] JNI DETECTED ERROR IN APPLICATION: can't call static org.eclipse.tm4e.core.registry.IThemeSource org.eclipse.tm4e.core.registry.IThemeSource$-CC.fromString(org.eclipse.tm4e.core.registry.IThemeSource$ContentType, java.lang.String) with class java.lang.Class<org.eclipse.tm4e.core.registry.IThemeSource>'
12-16 22:24:43.893 18210 18210 F DEBUG   :     x0  0000000000000000  x1  00000000000046e8  x2  0000000000000006  x3  0000000000000008
12-16 22:24:43.893 18210 18210 F DEBUG   :     x4  fefeff734dce4667  x5  fefeff734dce4667  x6  fefeff734dce4667  x7  7f7f7f7f7fff7f7f
12-16 22:24:43.893 18210 18210 F DEBUG   :     x8  0000000000000083  x9  dd61bb06df041932  x10 0000000000000000  x11 fffffffc7ffffbdf
12-16 22:24:43.893 18210 18210 F DEBUG   :     x12 0000000000000001  x13 ffffffffffffffff  x14 ffffffffff000000  x15 ffffffffffffffff
12-16 22:24:43.893 18210 18210 F DEBUG   :     x16 00000074528692b0  x17 0000007452788cd0  x18 0000000000000010  x19 00000000000046e8
12-16 22:24:43.893 18210 18210 F DEBUG   :     x20 00000000000046e8  x21 00000073be9f5b00  x22 000000000000000b  x23 00000073cdd27443
12-16 22:24:43.893 18210 18210 F DEBUG   :     x24 00000073cdd272f7  x25 0000000000000001  x26 0000007fd92f07e0  x27 0000000000000043
12-16 22:24:43.893 18210 18210 F DEBUG   :     x28 00000073ce513720  x29 0000007fd92f0690
12-16 22:24:43.893 18210 18210 F DEBUG   :     sp  0000007fd92f0650  lr  000000745277c144  pc  000000745277c16c
12-16 22:24:43.923 18210 18210 F DEBUG   : 
12-16 22:24:43.923 18210 18210 F DEBUG   : backtrace:
12-16 22:24:43.923 18210 18210 F DEBUG   :     #00 pc 000000000002216c  /system/lib64/libc.so (abort+116)
12-16 22:24:43.923 18210 18210 F DEBUG   :     #01 pc 0000000000465684  /system/lib64/libart.so (art::Runtime::Abort(char const*)+1196)
12-16 22:24:43.923 18210 18210 F DEBUG   :     #02 pc 0000000000008ce0  /system/lib64/libbase.so (android::base::LogMessage::~LogMessage()+724)
12-16 22:24:43.923 18210 18210 F DEBUG   :     #03 pc 00000000002e5ec8  /system/lib64/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+1640)
12-16 22:24:43.924 18210 18210 F DEBUG   :     #04 pc 00000000002e6038  /system/lib64/libart.so (art::JavaVMExt::JniAbortV(char const*, char const*, std::__va_list)+116)
12-16 22:24:43.924 18210 18210 F DEBUG   :     #05 pc 00000000000fdabc  /system/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::AbortF(char const*, ...)+148)
12-16 22:24:43.924 18210 18210 F DEBUG   :     #06 pc 00000000001017d0  /system/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::CheckMethodAndSig(art::ScopedObjectAccess&, _jobject*, _jclass*, _jmethodID*, art::Primitive::Type, art::InvokeType)+1160)
12-16 22:24:43.924 18210 18210 F DEBUG   :     #07 pc 0000000000101e40  /system/lib64/libart.so (art::(anonymous namespace)::CheckJNI::CallMethodA(char const*, _JNIEnv*, _jobject*, _jclass*, _jmethodID*, jvalue*, art::Primitive::Type, art::InvokeType)+748)
12-16 22:24:43.924 18210 18210 F DEBUG   :     #08 pc 000000000004d680  /data/app/com.companyname.AndroidBindingsJNICrashes-OMT4AzuyDOaf_ESYip9Szg==/lib/arm64/libmonodroid.so (java_interop_jnienv_call_static_object_method_a+48)
12-16 22:24:43.924 18210 18210 F DEBUG   :     #09 pc 00000000002af4c4  /data/app/com.companyname.AndroidBindingsJNICrashes-OMT4AzuyDOaf_ESYip9Szg==/lib/arm64/libmonosgen-2.0.so
12-16 22:24:43.924 18210 18210 F DEBUG   :     #10 pc 00000000002ae210  /data/app/com.companyname.AndroidBindingsJNICrashes-OMT4AzuyDOaf_ESYip9Szg==/lib/arm64/libmonosgen-2.0.so
12-16 22:24:43.924 18210 18210 F DEBUG   :     #11 pc 00000000002a48a0  /data/app/com.companyname.AndroidBindingsJNICrashes-OMT4AzuyDOaf_ESYip9Szg==/lib/arm64/libmonosgen-2.0.so
12-16 22:24:43.924 18210 18210 F DEBUG   :     #12 pc 00000000002afdf4  /data/app/com.companyname.AndroidBindingsJNICrashes-OMT4AzuyDOaf_ESYip9Szg==/lib/arm64/libmonosgen-2.0.so
12-16 22:24:43.924 18210 18210 F DEBUG   :     #13 pc 00000000002b035c  /data/app/com.companyname.AndroidBindingsJNICrashes-OMT4AzuyDOaf_ESYip9Szg==/lib/arm64/libmonosgen-2.0.so
12-16 22:24:43.924 18210 18210 F DEBUG   :     #14 pc 000000000000b7f0  <anonymous:000000744e8c1000>
12-16 22:24:44.237  1077  1077 E /system/bin/tombstoned: Tombstone written to: /data/tombstones/tombstone_07
@radimitrov radimitrov added Area: App Runtime Issues in `libmonodroid.so`. needs-triage Issues that need to be assigned. labels Jan 4, 2023
@grendello grendello added Area: Bindings Issues in Java Library Binding projects. and removed Area: App Runtime Issues in `libmonodroid.so`. needs-triage Issues that need to be assigned. labels Jan 4, 2023
@jpobst
Copy link
Contributor

jpobst commented Jan 9, 2023

Simplified test case
AndroidApp2.zip

@jpobst
Copy link
Contributor

jpobst commented Jan 9, 2023

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>

@jpobst 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 Jan 9, 2023
@jonpryor
Copy link
Member

jonpryor commented Jan 9, 2023

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.

@jonpryor
Copy link
Member

jonpryor commented Jan 9, 2023

@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:

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);

@jonpryor
Copy link
Member

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 xamarin/java.interop@1f27ab5

jonpryor added a commit to jonpryor/java.interop that referenced this issue Jan 20, 2023
jonpryor added a commit to jonpryor/java.interop that referenced this issue Jan 20, 2023
jonpryor added a commit to jonpryor/java.interop that referenced this issue Jan 20, 2023
jonpryor added a commit to jonpryor/java.interop that referenced this issue Jan 25, 2023
Context: 1f27ab5
Context: xamarin/xamarin-android@f6f11a5
Context: xamarin/xamarin-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
xamarin/xamarin-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
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 16, 2023
Context: 1f27ab5
Context: xamarin/xamarin-android@f6f11a5
Context: xamarin/xamarin-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
xamarin/xamarin-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
jonpryor added a commit to xamarin/java.interop that referenced this issue Feb 22, 2023
Context: 1f27ab5
Context: xamarin/xamarin-android@f6f11a5
Context: xamarin/xamarin-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
xamarin/xamarin-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
@jpobst
Copy link
Contributor

jpobst commented Apr 17, 2024

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

xamarin/java.interop#1077

@jpobst jpobst closed this as completed Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Bindings Issues in Java Library Binding projects.
Projects
None yet
Development

No branches or pull requests

4 participants