Skip to content
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

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

Open
jonpryor opened this issue Mar 18, 2025 · 0 comments

Comments

@jonpryor
Copy link
Member

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
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

No branches or pull requests

1 participant