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

[Java.Interop] GetSimpleReferences():fallback for GetTypeSignatures() #1305

Merged

Conversation

jonpryor
Copy link
Member

Context: dotnet/android#9768

dotnet/android#9768 attempts to add types from Java.Interop.dll to the .NET for Android typemaps, in order to fix Debug warnings like:

I monodroid-assembly: typemap: unable to find mapping to a Java type from managed type 'Java.Interop.ManagedPeer, Java.Interop'

Unfortunately, this causes the assertion:

AssertGetJniTypeInfoForType (typeof (JavaArray<JavaObject>),    "[Ljava/lang/Object;",  false,  1);

within Java.InteropTests.JniTypeManagerTests.GetTypeSignature_Type() to fail:

Expected string length 19 but was 33. Strings differ at index 0.
Expected: "[Ljava/lang/Object;"
But was:  "crc64d5d92128469ae06d/JavaArray_1"
-----------^

The immediate cause of the failure is that
JniRuntime.JniTypeManager.GetTypeSignature() called JniRuntime.JniTypeManager.GetSimpleReference() before it tried to see if the type was JavaArray<T>. As Java.Interop.dll was now being processed for typemap purposes, and because JavaArray<T> did not have a [JniTypeSignatureAttribute], the typemap got the default behavior of crc64[hash…].

The broader cause is that GetSimpleReference() should be the fallback implementation, used after all other attempts to get a JNI name have failed.

Update GetTypeSignature() and GetTypeSignatures() so that GetSimpleReference() and/or GetSimpleReferences() are in fact treated as fallbacks.

Additionally, update AssertGetJniTypeInfoForType() to assert that the value returned by GetTypeSignature() is the same as the value return3ed by GetTypeSignatures().First(). This was commented as being the case, but we should verify that as well.

Finally, update AssertGetJniTypeInfoForType() so that when the assertion fails, the source type value is provided.

Context: dotnet/android#9768

dotnet/android#9768 attempts to add types from `Java.Interop.dll` to
the .NET for Android typemaps, in order to fix Debug warnings like:

	I monodroid-assembly: typemap: unable to find mapping to a Java type from managed type 'Java.Interop.ManagedPeer, Java.Interop'

Unfortunately, this causes the assertion:

	AssertGetJniTypeInfoForType (typeof (JavaArray<JavaObject>),    "[Ljava/lang/Object;",  false,  1);

within `Java.InteropTests.JniTypeManagerTests.GetTypeSignature_Type()`
to fail:

	Expected string length 19 but was 33. Strings differ at index 0.
	Expected: "[Ljava/lang/Object;"
	But was:  "crc64d5d92128469ae06d/JavaArray_1"
	-----------^

The immediate cause of the failure is that
`JniRuntime.JniTypeManager.GetTypeSignature()` called
`JniRuntime.JniTypeManager.GetSimpleReference()` *before* it tried
to see if the type was `JavaArray<T>`.  As `Java.Interop.dll` was now
being processed for typemap purposes, and because `JavaArray<T>` did
not have a `[JniTypeSignatureAttribute]`, the typemap got the default
behavior of `crc64[hash…]`.

The broader cause is that `GetSimpleReference()` should be the
*fallback* implementation, used after all other attempts to get a
JNI name have failed.

Update `GetTypeSignature()` and `GetTypeSignatures()` so that
`GetSimpleReference()` and/or `GetSimpleReferences()` are in fact
treated as fallbacks.

Additionally, update `AssertGetJniTypeInfoForType()` to assert that
the value returned by `GetTypeSignature()` is the same as the value
return3ed by` GetTypeSignatures().First()`.  This was *commented* as
being the case, but we should *verify* that as well.

Finally, update `AssertGetJniTypeInfoForType()` so that when the
assertion fails, the source `type` value is provided.
jonpryor added a commit to dotnet/android that referenced this pull request Feb 10, 2025
Does It Build™?
@jonpryor
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jonpryor
Copy link
Member Author

Draft commit message:

[Java.Interop] GetSimpleReferences(): fallback for GetTypeSignatures()

Context: https://github.com/dotnet/android/pull/9768

dotnet/android#9768 attempts to add types from `Java.Interop.dll` to
the .NET for Android typemaps, in order to fix Debug warnings like:

	I monodroid-assembly: typemap: unable to find mapping to a Java type from managed type 'Java.Interop.ManagedPeer, Java.Interop'

Unfortunately, the initial attempt to generate typemaps for
`Java.Interop.dll` caused the assertion:

	AssertGetJniTypeInfoForType (typeof (JavaArray<JavaObject>),    "[Ljava/lang/Object;",  false,  1);

within `Java.InteropTests.JniTypeManagerTests.GetTypeSignature_Type()`
to fail with:

	Expected string length 19 but was 33. Strings differ at index 0.
	Expected: "[Ljava/lang/Object;"
	But was:  "crc64d5d92128469ae06d/JavaArray_1"
	-----------^

The immediate cause of the failure is that
`JniRuntime.JniTypeManager.GetTypeSignature()` called
`JniRuntime.JniTypeManager.GetSimpleReference()` *before* it tried
to see if the type was `JavaArray<T>`.  As `Java.Interop.dll` was now
being processed for typemap purposes, and because `JavaArray<T>` did
not have a `[JniTypeSignatureAttribute]`, the typemap got the default
behavior of `crc64[hash…]`.

The broader cause is that `GetSimpleReference()` should be the
*fallback* implementation, used after all other attempts to get a
JNI name have failed.

Update `GetTypeSignature()` and `GetTypeSignatures()` so that
`GetSimpleReference()` and/or `GetSimpleReferences()` are in fact
treated as fallbacks.

Additionally, update `AssertGetJniTypeInfoForType()` to assert that
the value returned by `GetTypeSignature()` is the same as the value
return3ed by` GetTypeSignatures().First()`.  This was *commented* as
being the case, but we should *verify* that as well.

Finally, *move* the
`type.GetCustomAttribute<JniTypeSignatureAttribute()` and
`GetReplacementType()` logic into `GetSimpleReferences()`.  This 
This emphasizes the "fallback" nature of `GetSimpleReference()`,
adds an missing `GetReplacementType()` invocation from the
`GetTypeSignatures()` codepath, and this is what
[`AndroidTypeManager.GetSimpleReference()`][0] was already doing.

[0]: https://github.com/dotnet/android/blob/21c413195e300b6440eb437dade4f3a114e795f7/src/Mono.Android/Android.Runtime/AndroidRuntime.cs#L279-L289

@jonpryor jonpryor requested a review from Copilot February 11, 2025 20:34

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

@jonpryor jonpryor merged commit d62008d into main Feb 12, 2025
4 checks passed
@jonpryor jonpryor deleted the dev/jonp/jonp-GetTypeSignatures-when-Java.Interop-has-typemaps branch February 12, 2025 01:27
jonpryor added a commit to dotnet/android that referenced this pull request Feb 12, 2025
jonpryor added a commit to dotnet/android that referenced this pull request Feb 12, 2025
Changes: dotnet/java-interop@57f7bc8...d62008d

  * dotnet/java-interop@d62008d1: [Java.Interop] GetSimpleReferences(): fallback for GetTypeSignatures() (dotnet/java-interop#1305)

Context: 25d1f00

If you have a Debug build of .NET for Android, and you enable
assembly logging:

	% adb shell setprop debug.mono.log default,assembly

then `adb logcat` will contain messages such as:

	I monodroid-assembly: typemap: unable to find mapping to a Java type from managed type 'Java.Interop.ManagedPeer, Java.Interop'
	W monodroid: typemap: failed to map managed type to Java type: Java.Interop.ManagedPeer, Java.Interop, Version=10.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065 (Module ID
	W monodroid-assembly: typemap: called from
	W monodroid-assembly: at Android.Runtime.JNIEnv.TypemapManagedToJava(Type type)
	W monodroid-assembly:    at Android.Runtime.AndroidTypeManager.GetSimpleReference(Type type)
	W monodroid-assembly:    at Java.Interop.JniRuntime.JniTypeManager.GetTypeSignature(Type type)
	W monodroid-assembly:    at Java.Interop.JniPeerMembers..ctor(String jniPeerTypeName, Type managedPeerType, Boolean checkManagedPeerType, Boolean isInterface)
	W monodroid-assembly:    at Java.Interop.JniPeerMembers..ctor(String jniPeerTypeName, Type managedPeerType)
	W monodroid-assembly:    at Java.Interop.ManagedPeer..cctor()
	W monodroid-assembly:    at Java.Interop.JniRuntime..ctor(CreationOptions options)
	W monodroid-assembly:    at Android.Runtime.AndroidRuntime..ctor(IntPtr jnienv, IntPtr vm, IntPtr classLoader, IntPtr classLoader_loadClass, Boolean jniAddNativeMethodRegistrationAttributePresent)
	W monodroid-assembly:    at Android.Runtime.JNIEnvInit.Initialize(JnienvInitializeArgs* args)
	…
	I monodroid-assembly: typemap: unable to find mapping to a Java type from managed type 'Java.Interop.JavaObject, Java.Interop'
	W monodroid: typemap: failed to map managed type to Java type: Java.Interop.JavaObject, Java.Interop, Version=10.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065 (Module ID: 90427bf1-700a-4676-b23a-fb68e2e2e9da; Type token: 33554439)
	W monodroid-assembly: typemap: called from
	W monodroid-assembly: at Android.Runtime.JNIEnv.TypemapManagedToJava(Type type)
	W monodroid-assembly:    at Android.Runtime.AndroidTypeManager.GetSimpleReference(Type type)
	W monodroid-assembly:    at Java.Interop.JniRuntime.JniTypeManager.GetTypeSignature(Type type)
	W monodroid-assembly:    at Java.Interop.JniPeerMembers..ctor(String jniPeerTypeName, Type managedPeerType, Boolean checkManagedPeerType, Boolean isInterface)
	W monodroid-assembly:    at Java.Interop.JniPeerMembers..ctor(String jniPeerTypeName, Type managedPeerType)
	W monodroid-assembly:    at Java.Interop.JavaObject..cctor()
	W monodroid-assembly:    at Java.Interop.TypeManager.Activate(IntPtr jobject, ConstructorInfo cinfo, Object[] parms)
	W monodroid-assembly:    at Java.Interop.TypeManager.n_Activate(IntPtr jnienv, IntPtr jclass, IntPtr typename_ptr, IntPtr signature_ptr, IntPtr jobject, IntPtr parameters_ptr)
	W monodroid-assembly:    at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPLLLL_V(_JniMarshal_PPLLLL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1, IntPtr p2, IntPtr p3)

`ManagedPeer` and `JavaObject` are types within `Java.Interop.dll`
which uses `JniPeerMembers`, and [Debug builds of `JniPeerMembers`][0]
try to verify that the typemap entry is consistent:

	partial class JniPeerMembers {
	  JniPeerMembers string jniPeerTypeName, Type managedPeerType, bool checkManagedPeerType, bool isInterface = false)
	  {
	    // …
	#if DEBUG
	    var signatureFromType   = JniEnvironment.Runtime.TypeManager.GetTypeSignature (managedPeerType);
	    if (signatureFromType.SimpleReference != jniPeerTypeName) {
	      Debug.WriteLine ("WARNING-Java.Interop: ManagedPeerType <=> JniTypeName Mismatch! javaVM.GetJniTypeInfoForType(typeof({0})).JniTypeName=\"{1}\" != \"{2}\"",
	          managedPeerType.FullName,
	          signatureFromType.SimpleReference,
	          jniPeerTypeName);
	      Debug.WriteLine (new System.Diagnostics.StackTrace (true));
	    }
	#endif  // DEBUG
	  }
	}

However, `Java.Interop.dll` is not considered to be a .NET for Android
assembly, as it's built with e.g. `$(TargetFramework)=net9.0` and not
`$(TargetFramework)=net9.0-android`.  As such, it is skipped during
typemap processing.

Update `XAJavaTypeScanner.SpecialAssemblies` so that `Java.Interop.dll`
is explicitly processed.  This will ensure that there is a typemap
entry for `ManagedPeer` and that the `JniPeerMembers` checks pass.
This allows typemap entries for `JavaObject` and `ManagedPeer` to be
created, removing the above warning messages.

This also requires updating `XAJavaTypeScanner.GetJavaTypes()` so
that in Debug builds, `Mono.Android.dll` is processed *before* any
other assemblies, ensuring that `java/lang/Object` is typemap'd to
`Java.Lang.Object, Mono.Android.dll`.  See also 25d1f00, which did
the same thing for *Release* builds.

Add a new `ObjectTest.java_lang_Object_Is_Java_Lang_Object()` unit
test which explicitly verifies the mapping for `java/lang/Object`.

[0]: https://github.com/dotnet/java-interop/blob/6bc87e8b55bc00ae1423a5ae92cf5db573fc76ed/src/Java.Interop/Java.Interop/JniPeerMembers.cs#L45-L54
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 this pull request may close these issues.

2 participants