-
Notifications
You must be signed in to change notification settings - Fork 538
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
[Mono.Android] JNIEnv.FindClass(Type)
now uses TypeManager
#9812
Conversation
Context: #9811 The .NET MAUI template + NativeAOT currently crashes with: 02-18 15:59:24.575 12907 12907 E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidProgramException: InvalidProgram_Specific, IntPtr Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(System.Type, Byte*) 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Internal.Runtime.TypeLoaderExceptionHelper.CreateInvalidProgramException(ExceptionStringID, String) + 0x4c 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(Type, Byte*) + 0x18 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Android.Runtime.JNIEnv.TypemapManagedToJava(Type) + 0x104 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Android.Runtime.JNIEnv.GetJniName(Type) + 0x1c 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Android.Runtime.JNIEnv.FindClass(Type) + 0x38 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray(IJavaObject[]) + 0x28 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray[T](T[]) + 0x94 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Android.Graphics.Drawables.LayerDrawable..ctor(Drawable[] layers) + 0xd4 02-18 15:59:24.575 12907 12907 E AndroidRuntime: at Microsoft.Maui.Platform.MauiRippleDrawableExtensions.UpdateMauiRippleDrawableBackground(View, Paint, IButtonStroke, Func`1, Func`1, Action) + 0x2ac This appears to be related to array usage, such as `LayerDrawable.ctor(Drawable[])` in this example. I can reproduce the same crash using a `ColorStateList.ctor(int[][], int[])` in our NativeAOT "hello world" sample: 02-19 10:45:29.728 28692 28692 E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidProgramException: InvalidProgram_Specific, IntPtr Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(System.Type, Byte*) 02-19 10:45:29.728 28692 28692 E AndroidRuntime: at Internal.Runtime.TypeLoaderExceptionHelper.CreateInvalidProgramException(ExceptionStringID, String) + 0x4c 02-19 10:45:29.728 28692 28692 E AndroidRuntime: at Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(Type, Byte*) + 0x18 02-19 10:45:29.728 28692 28692 E AndroidRuntime: at Android.Runtime.JNIEnv.TypemapManagedToJava(Type) + 0x104 02-19 10:45:29.728 28692 28692 E AndroidRuntime: at Android.Runtime.JNIEnv.GetJniName(Type) + 0x1c 02-19 10:45:29.728 28692 28692 E AndroidRuntime: at Android.Runtime.JNIEnv.FindClass(Type) + 0x38 02-19 10:45:29.728 28692 28692 E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray[T](T[]) + 0xa8 02-19 10:45:29.728 28692 28692 E AndroidRuntime: at Android.Content.Res.ColorStateList..ctor(Int32[][], Int32[]) + 0xdc 02-19 10:45:29.728 28692 28692 E AndroidRuntime: at NativeAOT.MainActivity.OnCreate(Bundle savedInstanceState) + 0xb8 As an alternative to #9811, we can update `JNIEnv.FindClass(Type)` to go through `TypeManager` instead of using `TypemapManagedToJava`. After this change, the sample works and prints a log message indicating `ColorStateList` is created successfully: 02-19 13:12:25.924 2665 2665 D NativeAOT: MainActivity.OnCreate() ColorStateList: ColorStateList{mThemeAttrs=nullmChangingConfigurations=0mStateSpecs=[[0, 1]]mColors=[0, 1]mDefaultColor=0}
} catch (Java.Lang.Throwable e) { | ||
if (!((e is Java.Lang.NoClassDefFoundError) || (e is Java.Lang.ClassNotFoundException))) | ||
throw; | ||
int rank = JavaNativeTypeManager.GetArrayInfo (type, out type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should log or something in the error path so we have something to look for in NativeAOT builds to see if we're hitting the error path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It calls monodroid_log
for e
currently, which would also fail spectacularly...
I put on the list to cleanup monodroid_log()
along with the Android system property parsing:
but what's odd is that we have a similar lookup for which passes, so why would |
Possibly because Java.Interop maps |
There are a couple places in java.interop that list new { JniType = "B", JniMarshalType = "Byte", ManagedType = "SByte", TypeModifier = "SByte" },
++new { JniType = "B", JniMarshalType = "Byte", ManagedType = "Byte", TypeModifier = "Byte" }, |
Context: dotnet/android#9747 Context: dotnet/android#9812 In the ongoing epic to get MAUI running atop NativeAOT, we hit our longstanding NativeAOT nemesis: P/Invoke: E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidProgramException: InvalidProgram_Specific, IntPtr Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(System.Type, Byte*) E AndroidRuntime: at Internal.Runtime.TypeLoaderExceptionHelper.CreateInvalidProgramException(ExceptionStringID, String) + 0x4c E AndroidRuntime: at Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(Type, Byte*) + 0x18 E AndroidRuntime: at Android.Runtime.JNIEnv.TypemapManagedToJava(Type) + 0x104 E AndroidRuntime: at Android.Runtime.JNIEnv.GetJniName(Type) + 0x1c E AndroidRuntime: at Android.Runtime.JNIEnv.FindClass(Type) + 0x38 E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray(IJavaObject[]) + 0x28 E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray[T](T[]) + 0x94 E AndroidRuntime: at Android.Graphics.Drawables.LayerDrawable..ctor(Drawable[] layers) + 0xd4 E AndroidRuntime: at Microsoft.Maui.Platform.MauiRippleDrawableExtensions.UpdateMauiRippleDrawableBackground(View, Paint, IButtonStroke, Func`1, Func`1, Action) + 0x2ac (`JNIEnv.monodroid_typemap_managed_to_java()` is P/Invoke.) The reasonable fix/workaround: update `JNIEnv.FindClass(Type)` to instead use `JniRuntime.JniTypeManager.GetTypeSignature(Type)`. (Also known as "embrace more JniRuntime abstractions!".) Unfortunately, this straightforward idea hits a minor schism between the .NET for Android and builtin java-interop world orders: How should Java `byte` be bound? For starters, what *is* a [Java `byte`][0]? > The values of the integral types are integers in the following ranges: > > * For `byte`, from -128 to 127, inclusive The Java `byte` is *signed*! Because of that, and because java-interop originated as a Second System Syndrome rebuild of Xamarin.Android, *of course* java-interop bound Java `byte` as `System.SByte`. .NET for Android, though, bound Java `byte` as `System.Byte`. This "minor" change meant that lots of unit tests started failing, e.g. [`NetworkInterfacesTest.DotNetInterfacesShouldEqualJavaInterfaces()`][2]: System.ArgumentException : Could not determine Java type corresponding to System.Byte[], System.Private.CoreLib, Version=10.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e. Arg_ParamName_Name, type at Android.Runtime.JNIEnv.FindClass(Type ) at Android.Runtime.JNIEnv.AssertCompatibleArrayTypes(IntPtr , Type ) at Android.Runtime.JNIEnv._GetArray(IntPtr , Type ) at Android.Runtime.JNIEnv.GetArray(IntPtr , JniHandleOwnership , Type ) at Java.Net.NetworkInterface.GetHardwareAddress() at System.NetTests.NetworkInterfacesTest.GetInfos(IEnumeration interfaces) at System.NetTests.NetworkInterfacesTest.DotNetInterfacesShouldEqualJavaInterfaces() at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags ) Rephrased, `runtime.TypeManager.GetTypeSignature(typeof(byte[]))` returned a "default" `JniTypeSignature` instance. It's time to reduce the size of this schism. Update `JniBuiltinMarshalers.GetBuiltInTypeSignature()` so that `TypeCode.Byte` is treated as a synonym for `TypeCode.SByte`. This is in fact all that's needed in order to add support for `byte[]`! It's *not* all that's necessary to fix all unit tests. Update `JniRuntime.JniTypeManager.GetTypeSignature()` and `.GetTypeSignatures()` so that if the type is an open generic type a `System.NotSupportedException` is thrown instead of a `System.ArgumentException`. This fixes [`Java.InteropTests.JnienvTest.NewOpenGenericTypeThrows()`][3]. Also, `JniBuiltinMarshalers.cs` got some hand-made changes, rendering it out of sync with `JniBuiltinMarshalers.tt`. Regenerate it. [0]: https://docs.oracle.com/javase/specs/jls/se21/html/jls-4.html#jls-4.2.1 [1]: https://github.com/dotnet/java-interop/blob/f30e420a1638dc013302e85dcf76642c10c26832/Documentation/Motivation.md [2]: https://github.com/dotnet/android/blob/1b1f1452f6b05707418d6605c06e106e6a2a6381/tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/NetworkInterfaces.cs#L107-L137 [3]: https://github.com/dotnet/android/blob/1b1f1452f6b05707418d6605c06e106e6a2a6381/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs#L107-L116
Does It Build™? (And fix the unit tests…?)
dotnet/java-interop#1302 fixed the `Could not determine Java type corresponding to System.Byte[]` crash. What replaced it was: Mono.Android.NET_Tests, Java.InteropTests.JnienvTest.NewObjectArrayWithNonJavaType / Release System.ArgumentException : Could not determine Java type corresponding to System.Type, System.Private.CoreLib, Version=10.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e. Arg_ParamName_Name, type The deal is that [`JavaNativeTypeManager.ToJniName(Type)`][0] defaults to using `java/lang/Object` if there is no type mapping. This is "reasonable" because of the "auto-box into `Android.Runtime.JavaObject`" behavior, meaning it *is* possible to create a Java array that "holds" `System.Type` instances. (Because the Java-side array is actually a `java.lang.Object[]`, each instance of which is an `Android.Runtime.JavaObject`, which in turn holds the `System.Type` instance.) However, this is *not* a semantic that `JniRuntime.JniTypeManager.GetTypeSignature()` maintains, and thus this was lost in 1b1f145. Update `JNIEnv.FindClass(Type)` so that if `.GetTypeSignature()` can't find the typemapping, default to `java/lang/Object`. This allows all tests to pass, locally. [0]: https://github.com/dotnet/java-interop/blob/62635a3ffee4c9ec421eb029b86e61267a544f92/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs#L164-L168
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Context: dotnet/android#9747 Context: dotnet/android#9812 Context: 71afce5 Context: dotnet/android@aa5e597 Context: dotnet/android@f800c1a In the ongoing epic to get MAUI running atop NativeAOT, we hit our longstanding NativeAOT nemesis: a P/Invoke: E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidProgramException: InvalidProgram_Specific, IntPtr Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(System.Type, Byte*) E AndroidRuntime: at Internal.Runtime.TypeLoaderExceptionHelper.CreateInvalidProgramException(ExceptionStringID, String) + 0x4c E AndroidRuntime: at Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(Type, Byte*) + 0x18 E AndroidRuntime: at Android.Runtime.JNIEnv.TypemapManagedToJava(Type) + 0x104 E AndroidRuntime: at Android.Runtime.JNIEnv.GetJniName(Type) + 0x1c E AndroidRuntime: at Android.Runtime.JNIEnv.FindClass(Type) + 0x38 E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray(IJavaObject[]) + 0x28 E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray[T](T[]) + 0x94 E AndroidRuntime: at Android.Graphics.Drawables.LayerDrawable..ctor(Drawable[] layers) + 0xd4 E AndroidRuntime: at Microsoft.Maui.Platform.MauiRippleDrawableExtensions.UpdateMauiRippleDrawableBackground(View, Paint, IButtonStroke, Func`1, Func`1, Action) + 0x2ac (`JNIEnv.monodroid_typemap_managed_to_java()` is P/Invoke. Why are P/Invokes bad? See dotnet/android@f800c1a6.) The reasonable fix/workaround: update `JNIEnv.FindClass(Type)` to instead use `JniRuntime.JniTypeManager.GetTypeSignature(Type)`. (Also known as "embrace more JniRuntime abstractions!".) Unfortunately, this straightforward idea hits a minor schism between the .NET for Android and builtin java-interop world orders: How should Java `byte` be bound? For starters, what *is* a [Java `byte`][0]? > The values of the integral types are integers in the following ranges: > > * For `byte`, from -128 to 127, inclusive The Java `byte` is *signed*! Because of that, and because java-interop originated as a Second System Syndrome rebuild of Xamarin.Android, *of course* java-interop bound Java `byte` as `System.SByte`. .NET for Android, though, bound Java `byte` as `System.Byte`. This "minor" change meant that lots of unit tests started failing, e.g. [`NetworkInterfacesTest.DotNetInterfacesShouldEqualJavaInterfaces()`][2]: System.ArgumentException : Could not determine Java type corresponding to System.Byte[], System.Private.CoreLib, Version=10.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e. Arg_ParamName_Name, type at Android.Runtime.JNIEnv.FindClass(Type ) at Android.Runtime.JNIEnv.AssertCompatibleArrayTypes(IntPtr , Type ) at Android.Runtime.JNIEnv._GetArray(IntPtr , Type ) at Android.Runtime.JNIEnv.GetArray(IntPtr , JniHandleOwnership , Type ) at Java.Net.NetworkInterface.GetHardwareAddress() at System.NetTests.NetworkInterfacesTest.GetInfos(IEnumeration interfaces) at System.NetTests.NetworkInterfacesTest.DotNetInterfacesShouldEqualJavaInterfaces() at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags ) Rephrased, `runtime.TypeManager.GetTypeSignature(typeof(byte[]))` returned a "default" `JniTypeSignature` instance. It's time to reduce the size of this schism. Update `JniBuiltinMarshalers.GetBuiltInTypeSignature()` so that `TypeCode.Byte` is treated as a synonym for `TypeCode.SByte`. This is in fact all that's needed in order to add support for `byte[]`! Repeat this exercise for all other unsigned types: `ushort`, `uint`, and `ulong`, as Kotlin unsigned types require it; see also 71afce5 and dotnet/android@aa5e597eba. This fixes the exception: System.InvalidCastException : Unable to cast from '[I' to '[Ljava/lang/Object;'. at Android.Runtime.JNIEnv.AssertCompatibleArrayTypes(IntPtr , Type ) at Android.Runtime.JNIEnv._GetArray(IntPtr , Type ) at Android.Runtime.JNIEnv.GetArray(IntPtr , JniHandleOwnership , Type ) at Foo.UnsignedInstanceMethods.UintArrayInstanceMethod(UInt32[] value) at Xamarin.Android.JcwGenTests.KotlinUnsignedTypesTests.TestUnsignedArrayTypeMembers() at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags ) This is *not* all that's necessary to fix all dotnet/android tests. Update `JniRuntime.JniTypeManager.GetTypeSignature()` and `.GetTypeSignatures()` so that if the type is an open generic type a `System.NotSupportedException` is thrown instead of a `System.ArgumentException`. This fixes [`Java.InteropTests.JnienvTest.NewOpenGenericTypeThrows()`][3]. Also, `JniBuiltinMarshalers.cs` has some hand-made changes, rendering it out of sync with `JniBuiltinMarshalers.tt`. Update `JniBuiltinMarshalers.tt` appropriately and regenerate it. [0]: https://docs.oracle.com/javase/specs/jls/se21/html/jls-4.html#jls-4.2.1 [1]: https://github.com/dotnet/java-interop/blob/f30e420a1638dc013302e85dcf76642c10c26832/Documentation/Motivation.md [2]: https://github.com/dotnet/android/blob/1b1f1452f6b05707418d6605c06e106e6a2a6381/tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/NetworkInterfaces.cs#L107-L137 [3]: https://github.com/dotnet/android/blob/1b1f1452f6b05707418d6605c06e106e6a2a6381/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs#L107-L116
Changes: ...9dea87dc1f3052ed0f9499c9858b27c83e7d139e
Draft commit message: Bump to dotnet/java-interop/main@9dea87dc; FindClass & TypeManager (#9812)
Changes: https://github.com/dotnet/java-interop/compare/f30e420a1638dc013302e85dcf76642c10c26832...9dea87dc1f3052ed0f9499c9858b27c83e7d139e
* dotnet/java-interop@9dea87dc: [Java.Interop] .GetTypeSignature() supports unsigned types (dotnet/java-interop#1312)
* dotnet/java-interop@1cfb4f4d: [generator] Add support for emitting `[UnsupportedOSPlatform]` (dotnet/java-interop#1307)
Context: https://github.com/dotnet/android/pull/9811
The .NET MAUI template + NativeAOT currently crashes with:
E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidProgramException: InvalidProgram_Specific, IntPtr Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(System.Type, Byte*)
E AndroidRuntime: at Internal.Runtime.TypeLoaderExceptionHelper.CreateInvalidProgramException(ExceptionStringID, String) + 0x4c
E AndroidRuntime: at Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(Type, Byte*) + 0x18
E AndroidRuntime: at Android.Runtime.JNIEnv.TypemapManagedToJava(Type) + 0x104
E AndroidRuntime: at Android.Runtime.JNIEnv.GetJniName(Type) + 0x1c
E AndroidRuntime: at Android.Runtime.JNIEnv.FindClass(Type) + 0x38
E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray(IJavaObject[]) + 0x28
E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray[T](T[]) + 0x94
E AndroidRuntime: at Android.Graphics.Drawables.LayerDrawable..ctor(Drawable[] layers) + 0xd4
E AndroidRuntime: at Microsoft.Maui.Platform.MauiRippleDrawableExtensions.UpdateMauiRippleDrawableBackground(View, Paint, IButtonStroke, Func`1, Func`1, Action) + 0x2ac
This appears to be related to array usage, such as
`LayerDrawable.ctor(Drawable[])` in this example.
I can reproduce the same crash using a
`ColorStateList.ctor(int[][], int[])` in `samples/NativeAOT`:
E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidProgramException: InvalidProgram_Specific, IntPtr Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(System.Type, Byte*)
E AndroidRuntime: at Internal.Runtime.TypeLoaderExceptionHelper.CreateInvalidProgramException(ExceptionStringID, String) + 0x4c
E AndroidRuntime: at Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(Type, Byte*) + 0x18
E AndroidRuntime: at Android.Runtime.JNIEnv.TypemapManagedToJava(Type) + 0x104
E AndroidRuntime: at Android.Runtime.JNIEnv.GetJniName(Type) + 0x1c
E AndroidRuntime: at Android.Runtime.JNIEnv.FindClass(Type) + 0x38
E AndroidRuntime: at Android.Runtime.JNIEnv.NewArray[T](T[]) + 0xa8
E AndroidRuntime: at Android.Content.Res.ColorStateList..ctor(Int32[][], Int32[]) + 0xdc
E AndroidRuntime: at NativeAOT.MainActivity.OnCreate(Bundle savedInstanceState) + 0xb8
Update `JNIEnv.FindClass(Type)` to go through `TypeManager` instead
of using `TypemapManagedToJava`. This avoids the P/Invoke which is
causing the crash (f800c1a6).
Note that we can't directly use
`JniRuntime.JniTypeManager.GetTypeSignature()`, as the previous use
of `JavaNativeTypeManager.ToJniName(Type)` would default to using
`java/lang/Object` if there was no typemap entry for `type`.
After this change, the sample works and prints a log message
indicating `ColorStateList` is created successfully:
D NativeAOT: MainActivity.OnCreate() ColorStateList: ColorStateList{mThemeAttrs=nullmChangingConfigurations=0mStateSpecs=[[0, 1]]mColors=[0, 1]mDefaultColor=0} |
Context: #9811
The .NET MAUI template + NativeAOT currently crashes with:
This appears to be related to array usage, such as
LayerDrawable.ctor(Drawable[])
in this example.I can reproduce the same crash using a
ColorStateList.ctor(int[][], int[])
in our NativeAOT "hello world" sample:As an alternative to #9811, we can update
JNIEnv.FindClass(Type)
to go throughTypeManager
instead of usingTypemapManagedToJava
.After this change, the sample works and prints a log message indicating
ColorStateList
is created successfully: