Skip to content

Commit

Permalink
[Java.Interop] Support System.Byte, somewhat
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jonpryor committed Feb 20, 2025
1 parent f30e420 commit 62635a3
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 10 additions & 3 deletions src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
{
Expand All @@ -242,6 +248,7 @@ namespace Java.Interop {
return sourceValue;
}

[RequiresDynamicCode (ExpressionRequiresUnreferencedCode)]
[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateReturnValueFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue)
{
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs
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
9 changes: 6 additions & 3 deletions tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs
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 @@ -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);
Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit 62635a3

Please sign in to comment.