From 62635a3ffee4c9ec421eb029b86e61267a544f92 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 20 Feb 2025 12:24:04 -0500 Subject: [PATCH 1/2] [Java.Interop] Support System.Byte, somewhat Context: https://github.com/dotnet/android/pull/9747 Context: https://github.com/dotnet/android/pull/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 --- .../Java.Interop/JniBuiltinMarshalers.cs | 1 + .../Java.Interop/JniBuiltinMarshalers.tt | 13 ++++++++++--- .../Java.Interop/JniRuntime.JniTypeManager.cs | 4 ++-- .../Java.Interop/JniTypeManagerTests.cs | 9 ++++++--- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs b/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs index 3ab2744cc..b555c9b5e 100644 --- a/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs +++ b/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs @@ -49,6 +49,7 @@ static bool GetBuiltInTypeSignature (Type type, ref JniTypeSignature signature) case TypeCode.Boolean: signature = GetCachedTypeSignature (ref __BooleanTypeSignature, "Z", arrayRank: 0, keyword: true); return true; + case TypeCode.Byte: case TypeCode.SByte: signature = GetCachedTypeSignature (ref __SByteTypeSignature, "B", arrayRank: 0, keyword: true); return true; diff --git a/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt b/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt index e5c911bf5..84dd657e5 100644 --- a/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt +++ b/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt @@ -57,6 +57,11 @@ namespace Java.Interop { return true; <# foreach (var type in types) { + if (type.Name == "Byte") { +#> + case TypeCode.Byte: +<# + } #> case TypeCode.<#= type.Type #>: signature = GetCachedTypeSignature (ref __<#= type.Type #>TypeSignature, "<#= type.JniType #>", arrayRank: 0, keyword: true); @@ -185,7 +190,7 @@ namespace Java.Interop { public override object? CreateValue ( ref JniObjectReference reference, JniObjectReferenceOptions options, - [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] + [DynamicallyAccessedMembers (Constructors)] Type? targetType) { if (!reference.IsValid) @@ -196,7 +201,7 @@ namespace Java.Interop { public override <#= type.Type #> CreateGenericValue ( ref JniObjectReference reference, JniObjectReferenceOptions options, - [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] + [DynamicallyAccessedMembers (Constructors)] Type? targetType) { if (!reference.IsValid) @@ -230,6 +235,7 @@ namespace Java.Interop { state = new JniValueMarshalerState (); } + [RequiresDynamicCode (ExpressionRequiresUnreferencedCode)] [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)] public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize, Type? targetType) { @@ -242,6 +248,7 @@ namespace Java.Interop { return sourceValue; } + [RequiresDynamicCode (ExpressionRequiresUnreferencedCode)] [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)] public override Expression CreateReturnValueFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue) { @@ -256,7 +263,7 @@ namespace Java.Interop { public override <#= type.Type #>? CreateGenericValue ( ref JniObjectReference reference, JniObjectReferenceOptions options, - [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] + [DynamicallyAccessedMembers (Constructors)] Type? targetType) { if (!reference.IsValid) diff --git a/src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs b/src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs index 9a8e5be9d..17ccbb705 100644 --- a/src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs +++ b/src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs @@ -146,7 +146,7 @@ public JniTypeSignature GetTypeSignature (Type type) if (type == null) throw new ArgumentNullException (nameof (type)); if (type.ContainsGenericParameters) - throw new ArgumentException ($"'{type}' contains a generic type definition. This is not supported.", nameof (type)); + throw new NotSupportedException ($"'{type}' contains a generic type definition. This is not supported."); type = GetUnderlyingType (type, out int rank); @@ -184,7 +184,7 @@ public IEnumerable GetTypeSignatures (Type type) if (type == null) yield break; if (type.ContainsGenericParameters) - throw new ArgumentException ($"'{type}' contains a generic type definition. This is not supported.", nameof (type)); + throw new NotSupportedException ($"'{type}' contains a generic type definition. This is not supported."); type = GetUnderlyingType (type, out int rank); diff --git a/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs b/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs index 60e64fd59..bb636b9ab 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs @@ -18,7 +18,7 @@ public void GetTypeSignature_Type () Assert.Throws(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[,]))); Assert.Throws(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[,][]))); Assert.Throws(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[][,]))); - Assert.Throws(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (Action<>))); + Assert.Throws(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (Action<>))); Assert.AreEqual (null, JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (JniRuntimeTest)).SimpleReference); AssertGetJniTypeInfoForType (typeof (string), "java/lang/String", false, 0); @@ -40,6 +40,7 @@ public void GetTypeSignature_Type () AssertGetJniTypeInfoForType (typeof (StringComparison[]), "[I", true, 1); AssertGetJniTypeInfoForType (typeof (StringComparison[][]), "[[I", true, 2); + AssertGetJniTypeInfoForType (typeof (byte[]), "[B", true, 1); AssertGetJniTypeInfoForType (typeof (int[]), "[I", true, 1); AssertGetJniTypeInfoForType (typeof (int[][]), "[[I", true, 2); AssertGetJniTypeInfoForType (typeof (int[][][]), "[[[I", true, 3); @@ -89,14 +90,16 @@ public void GetTypeSignature_Type () static void AssertGetJniTypeInfoForType (Type type, string jniType, bool isKeyword, int arrayRank) { var info = JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (type); + Assert.IsTrue (info.IsValid, $"info.IsValid for `{type}`"); // `GetTypeSignature() and `GetTypeSignatures()` should be "in sync"; verify that! var info2 = JniRuntime.CurrentRuntime.TypeManager.GetTypeSignatures (type).FirstOrDefault (); + Assert.IsTrue (info2.IsValid, $"info2.IsValid for `{type}`"); Assert.AreEqual (jniType, info.Name, $"info.Name for `{type}`"); - Assert.AreEqual (jniType, info2.Name, $"info.Name for `{type}`"); + Assert.AreEqual (jniType, info2.Name, $"info2.Name for `{type}`"); Assert.AreEqual (arrayRank, info.ArrayRank, $"info.ArrayRank for `{type}`"); - Assert.AreEqual (arrayRank, info2.ArrayRank, $"info.ArrayRank for `{type}`"); + Assert.AreEqual (arrayRank, info2.ArrayRank, $"info2.ArrayRank for `{type}`"); } [Test] From c1db895c07c275954ce73f0006a86141af258f60 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 20 Feb 2025 16:43:24 -0500 Subject: [PATCH 2/2] Support the other unsigned types, too! 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: 71afce55812fc2c5efd6002580b539d8394f7bca dotnet/android@aa5e597eba which adds unsigned types for Kotlin support --- .../Java.Interop/JniBuiltinMarshalers.cs | 6 ++--- .../Java.Interop/JniBuiltinMarshalers.tt | 23 ++++++++----------- .../Java.Interop/JniTypeManagerTests.cs | 7 +++++- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs b/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs index b555c9b5e..4728c8987 100644 --- a/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs +++ b/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs @@ -56,12 +56,15 @@ static bool GetBuiltInTypeSignature (Type type, ref JniTypeSignature signature) case TypeCode.Char: signature = GetCachedTypeSignature (ref __CharTypeSignature, "C", arrayRank: 0, keyword: true); return true; + case TypeCode.UInt16: case TypeCode.Int16: signature = GetCachedTypeSignature (ref __Int16TypeSignature, "S", arrayRank: 0, keyword: true); return true; + case TypeCode.UInt32: case TypeCode.Int32: signature = GetCachedTypeSignature (ref __Int32TypeSignature, "I", arrayRank: 0, keyword: true); return true; + case TypeCode.UInt64: case TypeCode.Int64: signature = GetCachedTypeSignature (ref __Int64TypeSignature, "J", arrayRank: 0, keyword: true); return true; @@ -75,9 +78,6 @@ static bool GetBuiltInTypeSignature (Type type, ref JniTypeSignature signature) case TypeCode.DBNull: case TypeCode.Decimal: case TypeCode.Empty: - case TypeCode.UInt16: - case TypeCode.UInt32: - case TypeCode.UInt64: return false; } diff --git a/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt b/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt index 84dd657e5..38b3a18d8 100644 --- a/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt +++ b/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt @@ -18,14 +18,14 @@ using Java.Interop.Expressions; namespace Java.Interop { <# var types = new[]{ - new { Name = "Boolean", Type = "Boolean", JniCallType = "Boolean", JniType = "Z", GetValue = "booleanValue" }, - new { Name = "Byte", Type = "SByte", JniCallType = "Byte", JniType = "B", GetValue = "byteValue" }, - new { Name = "Character", Type = "Char", JniCallType = "Char", JniType = "C", GetValue = "charValue" }, - new { Name = "Short", Type = "Int16", JniCallType = "Short", JniType = "S", GetValue = "shortValue" }, - new { Name = "Integer", Type = "Int32", JniCallType = "Int", JniType = "I", GetValue = "intValue" }, - new { Name = "Long", Type = "Int64", JniCallType = "Long", JniType = "J", GetValue = "longValue" }, - new { Name = "Float", Type = "Single", JniCallType = "Float", JniType = "F", GetValue = "floatValue" }, - new { Name = "Double", Type = "Double", JniCallType = "Double", JniType = "D", GetValue = "doubleValue" }, + new { Name = "Boolean", Type = "Boolean", UnsignedType = "", JniCallType = "Boolean", JniType = "Z", GetValue = "booleanValue" }, + new { Name = "Byte", Type = "SByte", UnsignedType = "Byte", JniCallType = "Byte", JniType = "B", GetValue = "byteValue" }, + new { Name = "Character", Type = "Char", UnsignedType = "", JniCallType = "Char", JniType = "C", GetValue = "charValue" }, + new { Name = "Short", Type = "Int16", UnsignedType = "UInt16", JniCallType = "Short", JniType = "S", GetValue = "shortValue" }, + new { Name = "Integer", Type = "Int32", UnsignedType = "UInt32", JniCallType = "Int", JniType = "I", GetValue = "intValue" }, + new { Name = "Long", Type = "Int64", UnsignedType = "UInt64", JniCallType = "Long", JniType = "J", GetValue = "longValue" }, + new { Name = "Float", Type = "Single", UnsignedType = "", JniCallType = "Float", JniType = "F", GetValue = "floatValue" }, + new { Name = "Double", Type = "Double", UnsignedType = "", JniCallType = "Double", JniType = "D", GetValue = "doubleValue" }, }; #> @@ -57,9 +57,9 @@ namespace Java.Interop { return true; <# foreach (var type in types) { - if (type.Name == "Byte") { + if (!string.IsNullOrEmpty (type.UnsignedType)) { #> - case TypeCode.Byte: + case TypeCode.<#= type.UnsignedType #>: <# } #> @@ -73,9 +73,6 @@ namespace Java.Interop { case TypeCode.DBNull: case TypeCode.Decimal: case TypeCode.Empty: - case TypeCode.UInt16: - case TypeCode.UInt32: - case TypeCode.UInt64: return false; } diff --git a/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs b/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs index bb636b9ab..c49b1f2eb 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs @@ -40,11 +40,16 @@ public void GetTypeSignature_Type () AssertGetJniTypeInfoForType (typeof (StringComparison[]), "[I", true, 1); AssertGetJniTypeInfoForType (typeof (StringComparison[][]), "[[I", true, 2); - AssertGetJniTypeInfoForType (typeof (byte[]), "[B", true, 1); AssertGetJniTypeInfoForType (typeof (int[]), "[I", true, 1); AssertGetJniTypeInfoForType (typeof (int[][]), "[[I", true, 2); AssertGetJniTypeInfoForType (typeof (int[][][]), "[[[I", true, 3); + // We map unsigned types to their signed counterparts + AssertGetJniTypeInfoForType (typeof (byte[]), "[B", true, 1); + AssertGetJniTypeInfoForType (typeof (ushort[]), "[S", true, 1); + AssertGetJniTypeInfoForType (typeof (uint[]), "[I", true, 1); + AssertGetJniTypeInfoForType (typeof (ulong[]), "[J", true, 1); + AssertGetJniTypeInfoForType (typeof (JavaSByteArray), "[B", true, 1); AssertGetJniTypeInfoForType (typeof (JavaInt16Array), "[S", true, 1); AssertGetJniTypeInfoForType (typeof (JavaInt32Array), "[I", true, 1);