-
Notifications
You must be signed in to change notification settings - Fork 54
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
[Java.Interop] Support System.Byte, somewhat #1312
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Feb 20, 2025
Does It Build™? (And fix the unit tests…?)
/azp run |
No pipelines are associated with this pull request. |
jonathanpeppers
approved these changes
Feb 20, 2025
This comes in via `Xamarin.Android.JcwGenTests.KotlinUnsignedTypesTests.TestUnsignedArrayTypeMembers()` https://github.com/dotnet/android/blob/137214ada0339e8e2e839c2a45042c372e757e4e/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/KotlinUnsignedTypesTests.cs#L52 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 ) The generated `UintArrayInstanceMethod()` is: // Metadata.xml XPath method reference: path="/api/package[@name='foo']/class[@name='UnsignedInstanceMethods']/method[@name='uintArrayInstanceMethod--ajY-9A' and count(parameter)=1 and parameter[1][@type='uint[]']]" [Register ("uintArrayInstanceMethod--ajY-9A", "([I)[I", "")] public unsafe uint[] UintArrayInstanceMethod (uint[] value) { const string __id = "uintArrayInstanceMethod--ajY-9A.([I)[I"; IntPtr native_value = JNIEnv.NewArray (value); try { JniArgumentValue* __args = stackalloc JniArgumentValue [1]; __args [0] = new JniArgumentValue (native_value); var __rm = _members.InstanceMethods.InvokeNonvirtualObjectMethod (__id, this, __args); return (uint[]?) JNIEnv.GetArray (__rm.Handle, JniHandleOwnership.TransferLocalRef, typeof (uint))!; } finally { if (value != null) { JNIEnv.CopyArray (native_value, value); JNIEnv.DeleteLocalRef (native_value); } global::System.GC.KeepAlive (value); } } The offending line is the `JNIEnv.GetArray (__rm.Handle, JniHandleOwnership.TransferLocalRef, typeof (uint))`. This used to work in dotnet/android because `JavaNativeTypeManager.GetPrimitiveClass()` treats unsigned types as their signed counterparts: https://github.com/dotnet/java-interop/blob/62635a3ffee4c9ec421eb029b86e61267a544f92/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs#L260-L287 See also: 71afce5 dotnet/android@aa5e597eba which adds unsigned types for Kotlin support
[Java.Interop] .GetTypeSignature() supports unsigned types (#1312)
Context: https://github.com/dotnet/android/pull/9747
Context: https://github.com/dotnet/android/pull/9812
Context: 71afce55812fc2c5efd6002580b539d8394f7bca
Context: https://github.com/dotnet/android/commit/aa5e597ebacce385ac96bc91bd10453e65608634
Context: https://github.com/dotnet/android/commit/f800c1a6ee8d584f28d6b7a03485b7091e2d3115
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 71afce55
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 |
jonpryor
pushed a commit
to dotnet/android
that referenced
this pull request
Feb 21, 2025
Changes: dotnet/java-interop@f30e420...9dea87d * 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: #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 (f800c1a). 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}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
(
JNIEnv.monodroid_typemap_managed_to_java()
is P/Invoke.)The reasonable fix/workaround: update
JNIEnv.FindClass(Type)
to instead useJniRuntime.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
?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 Javabyte
asSystem.SByte
..NET for Android, though, bound Java
byte
asSystem.Byte
.This "minor" change meant that lots of unit tests started failing, e.g.
NetworkInterfacesTest.DotNetInterfacesShouldEqualJavaInterfaces()
: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 thatTypeCode.Byte
is treated as a synonym forTypeCode.SByte
. This is in fact all that's needed in order to add support forbyte[]
!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 aSystem.NotSupportedException
is thrown instead of aSystem.ArgumentException
. This fixesJava.InteropTests.JnienvTest.NewOpenGenericTypeThrows()
.Also,
JniBuiltinMarshalers.cs
got some hand-made changes, rendering it out of sync withJniBuiltinMarshalers.tt
. Regenerate it.