Skip to content

Commit

Permalink
[Java.Interop] .GetTypeSignature() supports unsigned types (#1312)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jonpryor
jonpryor authored Feb 21, 2025
1 parent 1cfb4f4 commit 9dea87d
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 22 deletions.
7 changes: 4 additions & 3 deletions src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,22 @@ 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;
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;
Expand All @@ -74,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;
}

Expand Down
32 changes: 18 additions & 14 deletions src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
};
#>

Expand Down Expand Up @@ -57,6 +57,11 @@ namespace Java.Interop {
return true;
<#
foreach (var type in types) {
if (!string.IsNullOrEmpty (type.UnsignedType)) {
#>
case TypeCode.<#= type.UnsignedType #>:
<#
}
#>
case TypeCode.<#= type.Type #>:
signature = GetCachedTypeSignature (ref __<#= type.Type #>TypeSignature, "<#= type.JniType #>", arrayRank: 0, keyword: true);
Expand All @@ -68,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;
}

Expand Down Expand Up @@ -185,7 +187,7 @@ namespace Java.Interop {
public override object? CreateValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
if (!reference.IsValid)
Expand All @@ -196,7 +198,7 @@ namespace Java.Interop {
public override <#= type.Type #> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
if (!reference.IsValid)
Expand Down Expand Up @@ -230,6 +232,7 @@ namespace Java.Interop {
state = new JniValueMarshalerState ();
}

[RequiresDynamicCode (ExpressionRequiresUnreferencedCode)]
[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize, Type? targetType)
{
Expand All @@ -242,6 +245,7 @@ namespace Java.Interop {
return sourceValue;
}

[RequiresDynamicCode (ExpressionRequiresUnreferencedCode)]
[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateReturnValueFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue)
{
Expand All @@ -256,7 +260,7 @@ namespace Java.Interop {
public override <#= type.Type #>? CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
if (!reference.IsValid)
Expand Down
4 changes: 2 additions & 2 deletions src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -184,7 +184,7 @@ public IEnumerable<JniTypeSignature> 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);

Expand Down
14 changes: 11 additions & 3 deletions tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void GetTypeSignature_Type ()
Assert.Throws<ArgumentException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[,])));
Assert.Throws<ArgumentException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[,][])));
Assert.Throws<ArgumentException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[][,])));
Assert.Throws<ArgumentException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (Action<>)));
Assert.Throws<NotSupportedException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (Action<>)));
Assert.AreEqual (null, JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (JniRuntimeTest)).SimpleReference);

AssertGetJniTypeInfoForType (typeof (string), "java/lang/String", false, 0);
Expand All @@ -44,6 +44,12 @@ public void GetTypeSignature_Type ()
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);
Expand Down Expand Up @@ -89,14 +95,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]
Expand Down

0 comments on commit 9dea87d

Please sign in to comment.