Skip to content

(Re)move the type.ContainsGenericParameters check in JniRuntime.JniTypeManager.GetTypeSignature()? #1324

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

Closed
jonpryor opened this issue Mar 18, 2025 · 0 comments · Fixed by #1336

Comments

@jonpryor
Copy link
Contributor

jonpryor commented Mar 18, 2025

Context: dotnet/android#9913
Context: dotnet/android@de49d96

dotnet/android#9913 updated JNIEnv.GetJniName(Type) to use JniRuntime.JniTypeManager.GetTypeSignature(Type). It promptly failed on CI:

E NUnit   :  : System.NotSupportedException : 'Java.InteropTests.GenericHolder`1[T]' contains a generic type definition. This is not supported.
E NUnit   :    at Java.Interop.JniRuntime.JniTypeManager.GetTypeSignature(Type )
E NUnit   :    at Android.Runtime.JNIEnv.GetJniName(Type )
E NUnit   :    at Java.Interop.TypeManager.RegisterType(String , Type )
E NUnit   :    at Android.Runtime.JNIEnvInit.RegisterJniNatives(IntPtr , Int32 , IntPtr , IntPtr , Int32 )
E NUnit   :    at Java.Interop.JniEnvironment.Object.AllocObject(JniObjectReference )
E NUnit   :    at Java.Interop.JniType.AllocObject()
E NUnit   :    at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String , Type , JniArgumentValue* )
E NUnit   :    at Java.Lang.Object..ctor()
E NUnit   :    at Java.InteropTests.GenericHolder`1[[System.Int32, System.Private.CoreLib, Version=10.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]..ctor()
E NUnit   :    at Java.InteropTests.JnienvTest.NewClosedGenericTypeWorks()
E NUnit   :    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
E NUnit   :    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

because of this check:

if (type.ContainsGenericParameters)
throw new NotSupportedException ($"'{type}' contains a generic type definition. This is not supported.");

Should we have that check, first?

We certainly allow generic types to be Java.Lang.Object subclasses, e.g. GenericHolder<T>! Constructing instances of such types need to go through the type manager; should .GetTypeSignature() really be asserting that generic type parameters not exist?

Or is it because we're using the type definition? typeof(GenericHolder<int>).ContainsGenericParameters is false, i.e. that would pass the check; it's typeof(GenericHolder<>).ContainsGenericParameters which fails.

Indeed, it's because between JniEnvironment.Object.AllocObject() and JNIEnvInit.RegisterJniNatives() is the static constructor for the Java Callable Wrapper for GenericHolder<T>, which must implicitly use the type definition, as it cannot know what type parameters will be used:

public /* partial */  class GenericHolder_1
{
	public static final String __md_methods;
	static {
		__md_methods = "";
		mono.android.Runtime.register ("Java.InteropTests.GenericHolder`1, Mono.Android-Tests", GenericHolder_1.class, __md_methods);
	}
}

Given that we later use a generic type definition:

var genericDef = isGeneric ? type.GetGenericTypeDefinition () : type;
if (isGeneric) {
if (genericDef == typeof (JavaArray<>) || genericDef == typeof (JavaObjectArray<>)) {
var r = GetTypeSignature (type.GenericTypeArguments [0]);
return r.AddArrayRank (rank + 1);
}
var genericSimpleRef = GetSimpleReference (genericDef);
if (genericSimpleRef != null)
return new JniTypeSignature (genericSimpleRef, rank, false);
}

we can probably just remove the check entirely.

jonpryor pushed a commit to dotnet/android that referenced this issue Mar 18, 2025
`JNIEnv.GetJniName()` would fail under NativeAOT, as
`TypemapManagedToJava()` will eventually call a P/Invoke that
doesn't exist.

Update this method to go through the
`JniRuntime.JniTypeManager.GetTypeSignature()`, the abstraction that
should be used instead.

TODO: dotnet/java-interop#1324
jonpryor added a commit to jonpryor/java.interop that referenced this issue May 22, 2025
Context: dotnet/android#9913
Context: dotnet/android@de49d96

Fixes: dotnet#1324

dotnet/android#9913 updated `JNIEnv.GetJniName(Type)` to use
`JniRuntime.JniTypeManager.GetTypeSignature(Type)`.

It promptly failed on CI:

	E NUnit   :  : System.NotSupportedException : 'Java.InteropTests.GenericHolder`1[T]' contains a generic type definition. This is not supported.
	E NUnit   :    at Java.Interop.JniRuntime.JniTypeManager.GetTypeSignature(Type )
	E NUnit   :    at Android.Runtime.JNIEnv.GetJniName(Type )
	E NUnit   :    at Java.Interop.TypeManager.RegisterType(String , Type )
	E NUnit   :    at Android.Runtime.JNIEnvInit.RegisterJniNatives(IntPtr , Int32 , IntPtr , IntPtr , Int32 )
	E NUnit   :    at Java.Interop.JniEnvironment.Object.AllocObject(JniObjectReference )
	E NUnit   :    at Java.Interop.JniType.AllocObject()
	E NUnit   :    at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String , Type , JniArgumentValue* )
	E NUnit   :    at Java.Lang.Object..ctor()
	E NUnit   :    at Java.InteropTests.GenericHolder`1[[System.Int32, System.Private.CoreLib, Version=10.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]..ctor()
	E NUnit   :    at Java.InteropTests.JnienvTest.NewClosedGenericTypeWorks()
	E NUnit   :    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
	E NUnit   :    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

because of this check within
`JniRuntime.JniTypeManager.GetTypeSignature()`:

	if (type.ContainsGenericParameters)
	  throw new NotSupportedException ($"'{type}' contains a generic type definition. This is not supported.");

Why is `type.ContainsGenericParameters` true?

`type.ContainsGenericParameters` is true because XAJavaInterop1-style
Java Callable Wrappers contain an Assembly Qualified Type Name, and
if the underlying type is generic, then the assembly qualified type
name mentions the type definition.  Consider:

	// C#
	class GenericHolder<T> : Java.Lang.Object {
	}

results in this Java Callable Wrapper snippet:

	// Java
	public /* partial */ class GenericHolder_1
	{
	  public static final String __md_methods;
	  static {
	    __md_methods = "";
	    mono.android.Runtime.register ("Java.InteropTests.GenericHolder`1, Mono.Android-Tests", GenericHolder_1.class, __md_methods);
	  }
	}

Note ``Java.InteropTests.GenericHolder`1, Mono.Android-Tests``:
``Type.GetType("Java.InteropTests.GenericHolder`1, Mono.Android-Tests")``
results in a type definition, in which `type.ContainsGenericParameters`
is true.

*Is this a problem*?  No, it is not a problem *in type registration*,
as this has worked in dotnet/android for many years at this point.

Is this a problem *elsewhere*?  Yes:

 1. Instances of `GenericHolder_1` cannot be created *from Java*,
    because Java doesn't know what type parameters to provide,
    because of type erasure.

 2. `[JavaCallable]` methods on generic types also cannot work,
    as only *one* method can be registered, while semantically a
    generic method is *N* methods (per type).

Update `JniRuntime.JniTypeManager.GetTypeSignature()` to no longer
check for `type.ContainsGenericParameters`.

Update `JniTypeManagerTests` to assert that constructing instances
from Java results in an exception.

A "funny" thing happened when adding that test: it didn't assert!
There are two reasons for this.  The first is that the Java Callable
Wrapper for `Java.InteropTests.GenericHolder<T>` *did not* have a
Java constructor!  Without a constructor, it had the default
constructor, which has no such verification check.

The cause of the missing constructor is that
`Java.Interop.Tools.JavaCallableWrappers` needs to know the JNI
signature of the constructor to generate, and the `JavaObject`
default constructor didn't have `[JniConstructorSignature]`.

Update the `JavaObject` default constructor appropriately:

	partial class JavaObject {
	  [JniConstructorSignature ("()V")]
	  public JavaObject () …
	}

This causes the Java Callable Wrapper for `GenericHolder<T>` to have
the desired constructor which calls `ManagedPeer.construct()`,
which in turn is a prerequisite for throwing `NotSupportedException`.

The second issue is that the `CannotCreateGenericHolderFromJava()`
test *must* use `JNIEnv::AllocObject()` +
`JNIEnv::CallNonvirtualVoidMethod()` to construct the instance,
***not*** `JNIEnv::NewObject()`, because we special-case
`JNIEnv::NewObject()`; see also commit 1c99956.
jonathanpeppers pushed a commit that referenced this issue May 22, 2025
Context: dotnet/android#9913
Context: dotnet/android@de49d96

Fixes: #1324

dotnet/android#9913 updated `JNIEnv.GetJniName(Type)` to use
`JniRuntime.JniTypeManager.GetTypeSignature(Type)`.

It promptly failed on CI:

	E NUnit   :  : System.NotSupportedException : 'Java.InteropTests.GenericHolder`1[T]' contains a generic type definition. This is not supported.
	E NUnit   :    at Java.Interop.JniRuntime.JniTypeManager.GetTypeSignature(Type )
	E NUnit   :    at Android.Runtime.JNIEnv.GetJniName(Type )
	E NUnit   :    at Java.Interop.TypeManager.RegisterType(String , Type )
	E NUnit   :    at Android.Runtime.JNIEnvInit.RegisterJniNatives(IntPtr , Int32 , IntPtr , IntPtr , Int32 )
	E NUnit   :    at Java.Interop.JniEnvironment.Object.AllocObject(JniObjectReference )
	E NUnit   :    at Java.Interop.JniType.AllocObject()
	E NUnit   :    at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String , Type , JniArgumentValue* )
	E NUnit   :    at Java.Lang.Object..ctor()
	E NUnit   :    at Java.InteropTests.GenericHolder`1[[System.Int32, System.Private.CoreLib, Version=10.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]..ctor()
	E NUnit   :    at Java.InteropTests.JnienvTest.NewClosedGenericTypeWorks()
	E NUnit   :    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
	E NUnit   :    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

because of this check within
`JniRuntime.JniTypeManager.GetTypeSignature()`:

	if (type.ContainsGenericParameters)
	  throw new NotSupportedException ($"'{type}' contains a generic type definition. This is not supported.");

Why is `type.ContainsGenericParameters` true?

`type.ContainsGenericParameters` is true because XAJavaInterop1-style
Java Callable Wrappers contain an Assembly Qualified Type Name, and
if the underlying type is generic, then the assembly qualified type
name mentions the type definition.  Consider:

	// C#
	class GenericHolder<T> : Java.Lang.Object {
	}

results in this Java Callable Wrapper snippet:

	// Java
	public /* partial */ class GenericHolder_1
	{
	  public static final String __md_methods;
	  static {
	    __md_methods = "";
	    mono.android.Runtime.register ("Java.InteropTests.GenericHolder`1, Mono.Android-Tests", GenericHolder_1.class, __md_methods);
	  }
	}

Note ``Java.InteropTests.GenericHolder`1, Mono.Android-Tests``:
``Type.GetType("Java.InteropTests.GenericHolder`1, Mono.Android-Tests")``
results in a type definition, in which `type.ContainsGenericParameters`
is true.

*Is this a problem*?  No, it is not a problem *in type registration*,
as this has worked in dotnet/android for many years at this point.

Is this a problem *elsewhere*?  Yes:

 1. Instances of `GenericHolder_1` cannot be created *from Java*,
    because Java doesn't know what type parameters to provide,
    because of type erasure.

 2. `[JavaCallable]` methods on generic types also cannot work,
    as only *one* method can be registered, while semantically a
    generic method is *N* methods (per type).

Update `JniRuntime.JniTypeManager.GetTypeSignature()` to no longer
check for `type.ContainsGenericParameters`.

Update `JniTypeManagerTests` to assert that constructing instances
from Java results in an exception.

A "funny" thing happened when adding that test: it didn't assert!
There are two reasons for this.  The first is that the Java Callable
Wrapper for `Java.InteropTests.GenericHolder<T>` *did not* have a
Java constructor!  Without a constructor, it had the default
constructor, which has no such verification check.

The cause of the missing constructor is that
`Java.Interop.Tools.JavaCallableWrappers` needs to know the JNI
signature of the constructor to generate, and the `JavaObject`
default constructor didn't have `[JniConstructorSignature]`.

Update the `JavaObject` default constructor appropriately:

	partial class JavaObject {
	  [JniConstructorSignature ("()V")]
	  public JavaObject () …
	}

This causes the Java Callable Wrapper for `GenericHolder<T>` to have
the desired constructor which calls `ManagedPeer.construct()`,
which in turn is a prerequisite for throwing `NotSupportedException`.

The second issue is that the `CannotCreateGenericHolderFromJava()`
test *must* use `JNIEnv::AllocObject()` +
`JNIEnv::CallNonvirtualVoidMethod()` to construct the instance,
***not*** `JNIEnv::NewObject()`, because we special-case
`JNIEnv::NewObject()`; see also commit 1c99956.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant