[Java.Interop] Typemap support for JavaObject & [JniTypeSignature]
#1181
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Context: dotnet/android#8543 Context: dotnet/android#8625 Context: #1168 Context: def5bc0 Context: 005c914 dotnet/android#8543 tested PR #1168, was Totally Green™ -- finding no issues -- and so we merged PR #1168 into 005c914. Enter dotnet/android#8625, which bumps xamarin-android to use def5bc0, which includes 005c914. dotnet/android#8625 contains *failing unit tests* (?!), including `Java.InteropTests.InvokeVirtualFromConstructorTests()`: Java.Lang.LinkageError : net.dot.jni.test.CallVirtualFromConstructorDerived ----> System.NotSupportedException : Could not find System.Type corresponding to Java type JniTypeSignature(TypeName=net/dot/jni/test/CallVirtualFromConstructorDerived ArrayRank=0 Keyword=False) . at Java.Interop.JniEnvironment.StaticMethods.GetStaticMethodID(JniObjectReference , String , String ) at Java.Interop.JniType.GetStaticMethod(String , String ) at Java.Interop.JniPeerMembers.JniStaticMethods.GetMethodInfo(String , String ) at Java.Interop.JniPeerMembers.JniStaticMethods.GetMethodInfo(String ) at Java.Interop.JniPeerMembers.JniStaticMethods.InvokeObjectMethod(String , JniArgumentValue* ) at Java.InteropTests.CallVirtualFromConstructorDerived.NewInstance(Int32 value) at Java.InteropTests.InvokeVirtualFromConstructorTests.ActivationConstructor() at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags ) --- End of managed Java.Lang.LinkageError stack trace --- java.lang.NoClassDefFoundError: net.dot.jni.test.CallVirtualFromConstructorDerived at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method) at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:35) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2189) Caused by: android.runtime.JavaProxyThrowable: [System.NotSupportedException]: Could not find System.Type corresponding to Java type JniTypeSignature(TypeName=net/dot/jni/test/CallVirtualFromConstructorDerived ArrayRank=0 Keyword=False) . at Java.Interop.ManagedPeer.GetTypeFromSignature(Unknown Source:0) at Java.Interop.ManagedPeer.RegisterNativeMembers(Unknown Source:0) at net.dot.jni.ManagedPeer.registerNativeMembers(Native Method) at net.dot.jni.test.CallVirtualFromConstructorDerived.<clinit>(CallVirtualFromConstructorDerived.java:12) ... 3 more --NotSupportedException at Java.Interop.ManagedPeer.GetTypeFromSignature(JniTypeManager , JniTypeSignature , String ) at Java.Interop.ManagedPeer.RegisterNativeMembers(IntPtr jnienv, IntPtr klass, IntPtr n_nativeClass, IntPtr n_methods) :shocked-pikachu-face: (But dotnet/android#8543 was green!) The problem is twofold: 1. 005c914 now requires the presence of typemap entries from e.g. `Java.InteropTests.CallVirtualFromConstructorDerived` to `net.dot.jni.test.CallVirtualFromConstructorDerived`. 2. `Java.Interop.Tools.JavaCallableWrappers` et al doesn't create typemap entries for `Java.Interop.JavaObject` subclasses which have `[JniTypeSignature]`. Consequently, our units tests fail (and apparently weren't *run* on dotnet/android#8543?! Still not what happened.) Fix typemap generation by adding a new `TypeDefinition.HasJavaPeer()` extension method to replace all the `.IsSubclassOf("Java.Lang.Object")` and similar checks, extending it to also check for `Java.Interop.JavaObject` and `Java.Interop.JavaException` base types. (Continuing to use base type checks is done instead of just relying on implementation of `Java.Interop.IJavaPeerable` as a performance optimization, as there could be *lots* of interface types to check.) Additionally, @jonathanpeppers -- while trying to investigate all this -- ran across a build failure: obj\Debug\net9.0-android\android\src\java\lang\Object.java(7,15): javac.exe error JAVAC0000: error: cyclic inheritance involving Object This suggests that `Java.Interop.Tools.JavaCallableWrappers` was encountering `Java.Interop.JavaObject` -- or some other type which has `[JniTypeSignature("java/lang/Object")]` -- which is why `java/lang/Object.java` was being generated. Audit all `[JniTypeSignature]` attributes, and add `GenerateJavaPeer=false` to all types which should *not* hava a Java Callable Wrapper generated for them. This includes nearly everything within `Java.Interop-Tests.dll`. (We want the typemaps! We *don't* want generated Java source, as we have hand-written Java peer types for those tests.) --- Aside: this project includes [T4 Text Templates][0]. To regenerate the output files *without involving Visual Studio*, you can install the [`dotnet-t4`][1] tool: $ dotnet tool install --global dotnet-t4 then run it separately for each `.tt` file: $HOME/.dotnet/tools/t4 -o src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs \ src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt [0]: https://learn.microsoft.com/visualstudio/modeling/code-generation-and-t4-text-templates?view=vs-2022 [1]: https://www.nuget.org/packages/dotnet-t4/
|
TODO: update typemap generation unit tests to assert/verify that |
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Jan 24, 2024
Context: dotnet/java-interop#1181 Does It Build™?
|
For local verification -- before dotnet/android#8625 PR tests finish -- after applying these changes to my local xamarin-android checkout, I run: Before these changes, It looks like this should fix things, though I still want to update the typemap unit tests before merging this PR. |
|
We may also want to review & update xamarin-android to use |
jonathanpeppers
approved these changes
Jan 24, 2024
| @@ -92,6 +92,25 @@ public static bool IsSubclassOf (this TypeDefinition type, string typeName, IMet | |||
| return false; | |||
| } | |||
|
|
|||
| public static bool HasJavaPeer (this TypeDefinition type, IMetadataResolver resolver) | |||
There was a problem hiding this comment.
Is the idea we'd call this same method from xamarin-android as well?
So we don't have duplicate of this logic running around:
A funny thing happened when b9aa5f7 ran on xamarin-android: unit tests started crashing! E monodroid-assembly: typemap: unable to load assembly 'Java.Interop-Tests' when looking up managed type corresponding to Java type 'java/lang/Object' What appears to be happening is an Unfortunate Interaction™: 1. `Java.Interop-Tests.dll` contained *multiple bindings* for `java/lang/Object`. e.g. [JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)] partial class JavaDisposedObject : JavaObject { } 2. The typemap generator has no functionality to "prioritize" one binding vs. another; it's random. As such, there is nothing to cause `Java.Lang.Object` to be used as the preferred binding for `java/lang/Object`. This meant that when we hit the typemap codepath in .NET Android, we looked for the C# type that corresponded to `java/lang/Object`, found *some random type* from `Java.Interop-Tests`, and… …and then we hit another oddity: that codepath only supported looking for C# types in assemblies which had already been loaded. This was occurring during startup, so `Java.Interop-Tests` had not yet been loaded yet, so it errored out, returned `nullptr`, and later Android just aborts things: F droid.NET_Test: runtime.cc:638] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x79 Just…eep! This didn't happen before because `Java.Interop.JavaObject` subclasses *didn't* participate in typemap generation. b9aa5f7 *added* that support, introducing this unforeseen interaction. Fix this by *removing* all "alternate bindings" for `java/lang/Object`: - [JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)] + [JniTypeSignature (JniTypeName)] partial class JavaDisposedObject : JavaObject { internal const string JniTypeName = "net/dot/jni/test/JavaDisposedObject"; } This implicitly requires that we now have a Java Callable Wrapper for this type, so update `Java.Interop-Tests.csproj` to run `jcw-gen` as part of the build process. This ensures that we create the JCW for e.g. `JavaDisposedObject`. Finally, update `JavaVMFixture` to add the required typemap entries. These changes should allow .NET Android unit tests to run w/o crashing.
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Jan 26, 2024
Context: dotnet/java-interop@6b3637d The story so far is that some of our unit tests are crashing, and have been in one form or another since 4332ea0 (the bump to dotnet/java-interop@def5bc0). The current crash, from the PR build for 7b46391: D monodroid-assembly: typemap: assembly 'Java.Interop-Tests' hasn't been loaded yet, attempting a full load W monodroid-assembly: typemap: failed to load managed assembly 'Java.Interop-Tests.dll'. No such file or directory E monodroid-assembly: typemap: unable to load assembly 'Java.Interop-Tests' when looking up managed type corresponding to Java type 'java/lang/Object' I monodroid-timing: [1/5] Typemap.java_to_managed: end, total time; elapsed: 0:0::260000 W monodroid-assembly: typemap: called from W monodroid-assembly: at Java.Interop.TypeManager.GetJavaToManagedType(String ) W monodroid-assembly: at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) W monodroid-assembly: at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type ) W monodroid-assembly: at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership ) W monodroid-assembly: at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer) W monodroid-assembly: at Android.Runtime.JavaSet.Iterator() W monodroid-assembly: at Android.Runtime.JavaSet`1[[System.String, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetEnumerator() W monodroid-assembly: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ProcessArguments() W monodroid-assembly: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnCreate(Bundle arguments) W monodroid-assembly: at Android.App.Instrumentation.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_arguments) W monodroid-assembly: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) E droid.NET_Test: JNI ERROR (app bug): accessed stale Local 0x75 (index 7 in a table of size 7) F droid.NET_Test: java_vm_ext.cc:570] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x75 There are *three* "concerning" items here: 1. typemaps are trying to load `Java.Interop-Tests`, and failing: typemap: failed to load managed assembly 'Java.Interop-Tests.dll'. No such file or directory @grendello is looking into this. 2. The binding for `java/lang/Object` is coming from Java.Interop-Tests, not Mono.Android (?!) typemap: unable to load assembly 'Java.Interop-Tests' when looking up managed type corresponding to Java type 'java/lang/Object' dotnet/java-interop#1181 has a fix for this, and we're not applying the fix yet because we believe that it will hide (1). 3. The JNI error, which crashes the process: F droid.NET_Test: java_vm_ext.cc:570] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x75 F droid.NET_Test: java_vm_ext.cc:570] from void crc643df67da7b13bb6b1.TestInstrumentation_1.n_onCreate(android.os.Bundle) F droid.NET_Test: runtime.cc:630] Runtime aborting... F droid.NET_Test: runtime.cc:630] Dumping all threads without mutator lock held F droid.NET_Test: runtime.cc:630] All threads: F droid.NET_Test: runtime.cc:630] DALVIK THREADS (14): F droid.NET_Test: runtime.cc:630] "main" prio=5 tid=1 Runnable F droid.NET_Test: runtime.cc:630] | group="" sCount=0 dsCount=0 flags=0 obj=0x729e9d98 self=0x7567e0f51000 F droid.NET_Test: runtime.cc:630] | sysTid=9143 nice=0 cgrp=default sched=0/0 handle=0x7567e14daed8 F droid.NET_Test: runtime.cc:630] | state=R schedstat=( 1270418000 334229000 139 ) utm=16 stm=110 core=0 HZ=100 F droid.NET_Test: runtime.cc:630] | stack=0x7ffcbb3e4000-0x7ffcbb3e6000 stackSize=8192KB F droid.NET_Test: runtime.cc:630] | held mutexes= "abort lock" "mutator lock"(shared held) F droid.NET_Test: runtime.cc:630] native: #00 pc 000000000048df4e /apex/com.android.runtime/lib64/libart.so (art::DumpNativeStack(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, int, BacktraceMap*, char const*, art::ArtMethod*, void*, bool)+126) F droid.NET_Test: runtime.cc:630] native: #1 pc 00000000005a77c3 /apex/com.android.runtime/lib64/libart.so (art::Thread::DumpStack(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, bool, BacktraceMap*, bool) const+675) F droid.NET_Test: runtime.cc:630] native: #2 pc 00000000005c49cb /apex/com.android.runtime/lib64/libart.so (art::DumpCheckpoint::Run(art::Thread*)+859) F droid.NET_Test: runtime.cc:630] native: #3 pc 00000000005bcf28 /apex/com.android.runtime/lib64/libart.so (art::ThreadList::RunCheckpoint(art::Closure*, art::Closure*)+456) F droid.NET_Test: runtime.cc:630] native: #4 pc 00000000005bc2e1 /apex/com.android.runtime/lib64/libart.so (art::ThreadList::Dump(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, bool)+1601) F droid.NET_Test: runtime.cc:630] native: #5 pc 0000000000552eb9 /apex/com.android.runtime/lib64/libart.so (art::Runtime::Abort(char const*)+1529) F droid.NET_Test: runtime.cc:630] native: #6 pc 000000000000c873 /system/lib64/libbase.so (android::base::LogMessage::~LogMessage()+611) F droid.NET_Test: runtime.cc:630] native: #7 pc 00000000003ede84 /apex/com.android.runtime/lib64/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+1604) F droid.NET_Test: runtime.cc:630] native: #8 pc 00000000003ee18a /apex/com.android.runtime/lib64/libart.so (art::JavaVMExt::JniAbortF(char const*, char const*, ...)+234) F droid.NET_Test: runtime.cc:630] native: #9 pc 00000000005adf31 /apex/com.android.runtime/lib64/libart.so (art::Thread::DecodeJObject(_jobject*) const+785) F droid.NET_Test: runtime.cc:630] native: #10 pc 00000000003def9b /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::CheckInstance(art::ScopedObjectAccess&, art::(anonymous namespace)::ScopedCheck::InstanceKind, _jobject*, bool)+91) F droid.NET_Test: runtime.cc:630] native: #11 pc 00000000003de205 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::CheckPossibleHeapValue(art::ScopedObjectAccess&, char, art::(anonymous namespace)::JniValueType)+485) F droid.NET_Test: runtime.cc:630] native: #12 pc 00000000003dd732 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::Check(art::ScopedObjectAccess&, bool, char const*, art::(anonymous namespace)::JniValueType*)+690) F droid.NET_Test: runtime.cc:630] native: #13 pc 00000000003ce865 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::GetObjectClass(_JNIEnv*, _jobject*)+837) F droid.NET_Test: runtime.cc:630] native: #14 pc 0000000000017196 /data/app/Mono.Android.NET_Tests-LUUW792fOvX745oAS4jeRQ==/split_config.x86_64.apk (offset 331000) (???) F droid.NET_Test: runtime.cc:630] at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onCreate(Native method) As per `native #13`, we're (somehow) passing an invalid JNI reference to `JNIEnv::GetObjectClass()`. ***HOW*** are we passing an invalid JNI reference to `JNIEnv::GetObjectClass()`? Attempt to investigate (3) further, by: 1. Reviewing all calls to `JNIEnv::GetObjectClass()` within this repo to see if we could potentially be passing an invalid value. The "most obvious" candidate is `TypeManager.CreateInstance()`, which calls `JNIEnv::GetObjectClass()` in a loop. I'm still not sure if that could possibly be the cause, but Just In Case™… "Cleanup" some C++ code so that calls to `JNIEnv::DeleteLocalReference()` are closer to the `JNIEnv::GetObjectClass()` calls. 2. Update `tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Mono.Android.NET-Tests.csproj` to add an `@(AndroidEnvironment)` item, which sets `debug.mono.log=gref+,lref+`. This will enable LREF and GREF logging within `adb logcat`, which *may* allow us to track down where "local reference 0x75" came from.
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Jan 26, 2024
Context: dotnet/java-interop#1181 Context: #8625 Context: dotnet/java-interop@005c914 Does It Build™? PR #8625 has turned into a cluster, *probably* because of dotnet/java-interop@005c9141, which implicitly requires typemap support for *non*-`Java.Lang.Object` subclasses such as the `CallVirtualFromConstructorDerived` test type. Java.Interop#1181 updates typemap and JCW generation to support `Java.Interop.JavaObject` and `Java.Interop.JavaException` subclasses, which will *hopefully* allow the `CallVirtualFromConstructorDerived`-using tests to work.
This was referenced Jan 26, 2024
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Jan 27, 2024
Context: xamarin/monodroid@e3e4f123d8 Context: dotnet/java-interop@005c9141 Context: dotnet/java-interop#1181 We've been trying to track down a JNI error which occurs when trying to use dotnet/java-interop@005c9141, resembling: I monodroid-lref: +l+ lrefc 1 handle 0x71/L from thread '(null)'(1) D monodroid-gref: at Android.Runtime.AndroidObjectReferenceManager.CreatedLocalReference(JniObjectReference , Int32& ) D monodroid-gref: at Java.Interop.JniRuntime.JniObjectReferenceManager.CreatedLocalReference(JniEnvironmentInfo , JniObjectReference ) D monodroid-gref: at Java.Interop.JniEnvironment.LogCreateLocalRef(JniObjectReference ) D monodroid-gref: at Java.Interop.JniEnvironment.LogCreateLocalRef(IntPtr ) D monodroid-gref: at Java.Interop.JniEnvironment.InstanceMethods.CallObjectMethod(JniObjectReference , JniMethodInfo ) D monodroid-gref: at Android.Runtime.JNIEnv.CallObjectMethod(IntPtr , IntPtr ) D monodroid-gref: at Android.Runtime.JavaSet.Iterator() D monodroid-gref: at Android.Runtime.JavaSet`1[[System.String, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetEnumerator() D monodroid-gref: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ProcessArguments() D monodroid-gref: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnCreate(Bundle arguments) D monodroid-gref: at Android.App.Instrumentation.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_arguments) D monodroid-gref: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) … I monodroid-lref: -l- lrefc 0 handle 0x71/L from thread '(null)'(1) D monodroid-gref: at Android.Runtime.AndroidObjectReferenceManager.DeleteLocalReference(JniObjectReference& , Int32& ) D monodroid-gref: at Java.Interop.JniRuntime.JniObjectReferenceManager.DeleteLocalReference(JniEnvironmentInfo , JniObjectReference& ) D monodroid-gref: at Java.Interop.JniObjectReference.Dispose(JniObjectReference& reference) D monodroid-gref: at Android.Runtime.JNIEnv.DeleteLocalRef(IntPtr ) D monodroid-gref: at Android.Runtime.JNIEnv.DeleteRef(IntPtr , JniHandleOwnership ) D monodroid-gref: at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) D monodroid-gref: at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type ) D monodroid-gref: at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership ) D monodroid-gref: at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer) D monodroid-gref: at Android.Runtime.JavaSet.Iterator() D monodroid-gref: at Android.Runtime.JavaSet`1[[System.String, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetEnumerator() D monodroid-gref: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ProcessArguments() D monodroid-gref: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnCreate(Bundle arguments) D monodroid-gref: at Android.App.Instrumentation.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_arguments) D monodroid-gref: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) D monodroid-gref: E droid.NET_Test: JNI ERROR (app bug): accessed stale Local 0x71 (index 7 in a table of size 7) F droid.NET_Test: java_vm_ext.cc:570] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x71 … F droid.NET_Test: runtime.cc:630] native: #13 pc 00000000003ce865 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::GetObjectClass(_JNIEnv*, _jobject*)+837) This has been "fun". The problem: 1. dotnet/java-interop@005c9141 relies on/requires additional typemaps in order to "fix" some linker warnings. This felt "fine" at the time. 2. However, the Java.Interop *unit tests* which test (1) involve "hand-written" typemap entries to allow things to work. 3. In .NET Android, those hand-written typemap entries aren't used; instead, the normal .NET Android typemaps are used. 4. .NET Android typemaps did not contain entries for the types introduced in (2), so various tests started failing. 5. dotnet/java-interop#1181 attempts to fix this by extending Java Callable Wrappers and associated typemaps to support `Java.Interop.JavaObject` subclasses, which brings the new types in (2) into the "normal" typemap fold. 6. However, some of those types "alias" `java.lang.Object`, and -- for some "bizarre" random ordering reason -- a type within `Java.Interop-Tests.dll` becomes the preferred `System.Type` to return when looking up `java/lang/Object`. 7. Which would *probably* be okay (if *really* weird), except that `GetJavaToManagedType()` returns null when the binding is within an assembly that hasn't been loaded yet. As this codepath is getting hit during app startup, `Java.Interop-Tests` hasn't been loaded, so `GetJavaToManagedType()` returns null. 8. Which means we're now in the scenario of being unable to find a binding/"wrapper class" for `java/lang/Object`, which we consider to be an error state. Because it's an error state, we dutifully throw. …except we've never actually hit this error state before -- HOW COULD WE?! -- which means we've found a bug in our error handling. Quick, find the problem! JNIEnv.DeleteRef (handle, transfer); throw new NotSupportedException ( FormattableString.Invariant ($"Internal error finding wrapper class for '{JNIEnv.GetClassNameFromInstance (handle)}'. (Where is the Java.Lang.Object wrapper?!)"), CreateJavaLocationException ()); The problem is a "use after free" bug: `JNIEnv.DeleteRef(handle, transfer)` *invalidates `handle`*, and then *immediately* afterward we call `JNIEnv.GetClassNameFromInstance(handle)`, on the now invalid value. BOOM goes the Android runtime. (The `DeleteRef()` call was introduced in xamarin/monodroid@e3e4f123d8, on 2011-Oct-19. Over 12 years to encounter this scenario!) Unfortunately, *just* fixing the "use-after-free" bug is insufficient; if we throw that `NotSupportedException`, things *will* break elsewhere. We'll just have an "elegant unhandled exception" app crash instead of a "THE WORLD IS ENDING" failed assertion crash. We could go with the simple fix for the crash, but this means that in order to integrate dotnet/java-interop@005c9141 & dotnet/java-interop#1181 we'd have to figure out how to *ensure* that `java/lang/Object` is bound as `Java.Lang.Object, Mono.Android`, not `Java.InteropTests.JavaLangRemappingTestObject, Java.Interop-Tests`. There may be a *slightly* more complicated fix which fixes both issues: consider the `-l-` callstack: at Android.Runtime.JNIEnv.DeleteRef(IntPtr , JniHandleOwnership ) at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type ) at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership ) at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer) at Android.Runtime.JavaSet.Iterator() This is part of a generic `Object.GetObject<IIterator>()` invocation! Additionally, because `IIterator` is an interface, in *normal* use the `type` variable within `TypeManager.CreateInstance()` would be `Java.Lang.Object, Mono.Android` and then ~immediately "discarded" because `Java.Lang.Object` cannot be assigned to `IIterator`. If we move the type compatibility check to *before* the `type == null` check, we *may* also fix the "`java/lang/Object` is bound as some unloadable type" issue. Let's try that!
This type mapping isn't *actually* required, but it *is* used in `JavaVMFixture`, and it confuses people (me!) if things are inconsistent.
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Jan 30, 2024
`acw-map.txt` contains mappings from .NET types to Java types, and implicitly vice-versa; see (TODO commit). *Normally* it contains three entries: 1. The fully-qualified .NET type name 2. The .NET type name, no assembly 3. (2) with a lowercased namespace name. For example: Mono.Android_Test.Library.CustomTextView, Mono.Android-Test.Library.NET;crc6456ab8145c81c4100.CustomTextView Mono.Android_Test.Library.CustomTextView;crc6456ab8145c81c4100.CustomTextView mono.android_test.library.CustomTextView;crc6456ab8145c81c4100.CustomTextView However, when XA4214 is emitted, there is a "collision" on the .NET side (but *not* the Java side); (2) and (3) are *ambiguous*, so one .NET type is arbitrarily chosen. The first line is still possible, because of assembly qualification. Enter ``Java.InteropTests.GenericHolder`1``: this type is present in *both* `Java.Interop-Tests.dll` *and* `Mono.Android-Tests.dll`. Before dotnet/java-interop#1181, this was "fine" because the `GenericHolder<T>` within `Java.Interop-Tests.dll` did not participate in typemap generation. Now it does, resulting in the XA4214. XA4214 *also* means that instead of three lines, it's *one* line: Java.InteropTests.GenericHolder`1, Mono.Android.NET-Tests;crc641855b07eca6dcc03.GenericHolder_1 Enter `<R8/>`, which parses `acw-map.txt` to create a `proguard_project_primary.cfg` file. `<R8/>` did it's *own* parsing of `acw-map.txt`, parsing only *one of every three lines*, on the assumption that *all* entries took three lines. This breaks in the presence of XA4214, because some entries only take one line, not three lines. Update `<R8/>` to instead use `MonoAndroidHelper.LoadMapFile()`, which reads all lines within `acw-map.txt`. This should result in a `proguard_project_primary.cfg` file which properly contains a `-keep` entry for `crc641855b07eca6dcc03.GenericHolder_1`.
CI is failing with: Java.Interop-Tests -> /Users/runner/work/1/s/bin/TestRelease-net7.0/Java.Interop-Tests.dll Could not execute because the specified command or file was not found. Possible reasons for this include: * You misspelled a built-in dotnet command. * You intended to execute a .NET program, but dotnet-/Users/runner/work/1/s/bin/Release-net7.0//jcw-gen.dll does not exist. * You intended to run a global tool, but a dotnet-prefixed executable with this name could not be found on the PATH. ##[error]tests/Java.Interop-Tests/Java.Interop-Tests.targets(26,5): Error MSB3073: The command "dotnet "/Users/runner/work/1/s/bin/Release-net7.0//jcw-gen.dll" -v "/Users/runner/work/1/s/bin/TestRelease-net7.0/Java.Interop-Tests.dll" --codegen-target JavaInterop1 -o "obj/Release-net7.0//java" -L "/Users/runner/work/1/s/bin/Release-net7.0/ref/." -L "/Users/runner/work/1/s/bin/TestRelease-net7.0/ref/." -L "/Users/runner/hostedtoolcache/dotnet/packs/Microsoft.NETCore.App.Ref/7.0.15/ref/net7.0/." -L "/Users/runner/.nuget/packages/microsoft.testplatform.testhost/17.5.0-preview-20221003-04/lib/netcoreapp3.1/." -L "/Users/runner/.nuget/packages/microsoft.codecoverage/17.5.0-preview-20221003-04/lib/netcoreapp3.1/." -L "/Users/runner/.nuget/packages/mono.linq.expressions/2.0.0/lib/netstandard2.0/." -L "/Users/runner/.nuget/packages/mono.options/6.12.0.148/lib/netstandard2.0/." -L "/Users/runner/.nuget/packages/newtonsoft.json/13.0.1/lib/netstandard2.0/." -L "/Users/runner/.nuget/packages/nuget.frameworks/5.11.0/lib/netstandard2.0/." -L "/Users/runner/.nuget/packages/nunit/3.13.2/lib/netstandard2.0/." -L "/Users/runner/work/1/s/external/xamarin-android-tools/bin/Release/net6.0/ref/."" exited with code 1. /Users/runner/work/1/s/tests/Java.Interop-Tests/Java.Interop-Tests.targets(26,5): error MSB3073: The command "dotnet "/Users/runner/work/1/s/bin/Release-net7.0//jcw-gen.dll" -v "/Users/runner/work/1/s/bin/TestRelease-net7.0/Java.Interop-Tests.dll" --codegen-target JavaInterop1 -o "obj/Release-net7.0//java" -L "/Users/runner/work/1/s/bin/Release-net7.0/ref/." -L "/Users/runner/work/1/s/bin/TestRelease-net7.0/ref/." -L "/Users/runner/hostedtoolcache/dotnet/packs/Microsoft.NETCore.App.Ref/7.0.15/ref/net7.0/." -L "/Users/runner/.nuget/packages/microsoft.testplatform.testhost/17.5.0-preview-20221003-04/lib/netcoreapp3.1/." -L "/Users/runner/.nuget/packages/microsoft.codecoverage/17.5.0-preview-20221003-04/lib/netcoreapp3.1/." -L "/Users/runner/.nuget/packages/mono.linq.expressions/2.0.0/lib/netstandard2.0/." -L "/Users/runner/.nuget/packages/mono.options/6.12.0.148/lib/netstandard2.0/." -L "/Users/runner/.nuget/packages/newtonsoft.json/13.0.1/lib/netstandard2.0/." -L "/Users/runner/.nuget/packages/nuget.frameworks/5.11.0/lib/netstandard2.0/." -L "/Users/runner/.nuget/packages/nunit/3.13.2/lib/netstandard2.0/." -L "/Users/runner/work/1/s/external/xamarin-android-tools/bin/Release/net6.0/ref/."" exited with code 1. [/Users/runner/work/1/s/tests/Java.Interop-Tests/Java.Interop-Tests.csproj] Add a `@(ProjectReference)` from `Java.Interop-Tests.csproj` to `jcw-gen.csproj` to ensure that `jcw-gen.csproj` is built first.
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Jan 31, 2024
Shouldn't change anything on this side…
Fix typo. (What's a "cit"?)
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Feb 2, 2024
Context: dotnet/java-interop#1165 Context: dotnet/java-interop@005c914 Context: #8543 Context: dotnet/java-interop@07c7300 Context: #8625 Context: xamarin/monodroid@e3e4f12 Context: xamarin/monodroid@a04b73b Context: efbec22 Changes: dotnet/java-interop@8b85462...07c7300 * dotnet/java-interop@07c73009: [Java.Interop] Typemap support for JavaObject & `[JniTypeSignature]` (dotnet/java-interop#1181) * dotnet/java-interop@d529f3be: Bump to xamarin/xamarin-android-tools/main@ed102fc (dotnet/java-interop#1182) * dotnet/java-interop@def5bc0d: [ci] Add API Scan job (dotnet/java-interop#1178) * dotnet/java-interop@d5afa0af: [invocation-overhead] Add generated source files (dotnet/java-interop#1175) * dotnet/java-interop@473ef74c: Bump to xamarin/xamarin-android-tools/main@4889bf0 (dotnet/java-interop#1172) * dotnet/java-interop@005c9141: [Java.Interop] Avoid `Type.GetType()` in `ManagedPeer` (dotnet/java-interop#1168) * dotnet/java-interop@0f1efebd: [Java.Interop] Use PublicApiAnalyzers to ensure we do not break API (dotnet/java-interop#1170) (From the "infinite scream" department…) It started with a desire to remove some linker warnings (dotnet/java-interop#1165): external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs(93,19,93,112): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type. dotnet/java-interop@005c9141 attempted to fix this by requiring the use of "typemaps" mapping Java type signatures to managed types, replacing e.g.: Type type = Type.GetType ("Example.Type, AssemblyName", throwOnError: true)!; Type[] parameterTypes = GetParameterTypes ("System.Int32:System.Int32"); ConstructorInfo ctor = type.GetConstructor (ptypes); // ctor=Example.Type(int, int) constructor with (not exactly, but for expository purposes): Type type = GetTypeFromSignature("crc64…/Type"); Type[] parameterTypes = GetConstructorCandidateParameterTypes ("(II)V"); ConstructorInfo ctor = type.GetConstructor (ptypes); // ctor=Example.Type(int, int) constructor among other changes. This was a *significant* change that would alter *Java.Interop* semantics but *not* .NET Android semantics -- .NET Android uses `Java.Interop.TypeManager.n_Activate()` (in this repo) for Java-side "activation" scenarios, not `Java.Interop.ManagedPeer` -- so in an abundance of caution we did a manual integration test in #8543 to make sure nothing broke before merging it. Something was apparently "off" in that integration. (We're still not sure what was off, or why it was completely green.) Ever since dotnet/java-interop@005c9141 was merged, every attempt to bump xamarin/Java.Interop has failed, in a number of ways described below. However, instead of reverting dotnet/java-interop@005c9141 we took this as an opportunity to understand *how and why* things were failing, as apparently we had encountered some *long-standing* corner cases in How Things Work. The oversights and failures include: 1. In order to make the Java.Interop unit tests work in .NET Android, the (largely hand-written) Java.Interop test types *also* need to participate with .NET Android typemap support, so that there is a typemap entry mapping `net/dot/jni/test/GenericHolder` to `Java.InteropTests.GenericHolder<T>` and vice-versa. dotnet/java-interop@07c73009 updates `Java.Interop.Tools.JavaCallableWrappers` to support creating typemap entries for `Java.Interop.JavaObject` subclasses, introducing a new `TypeDefinition.HasJavaPeer()` extension method. 2. (1) meant that, for the first time ever, types in `Java.Interop-Tests` participated in .NET Android type mapping. This *sounds* fine, except that `Java.Interop-Tests` contains "competing bindings" for `java.lang.Object`: [JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)] partial class JavaLangRemappingTestObject : JavaObject { } 3. (2) means that, for the first time ever, we *could* have the typemap entry for `java/lang/Object` map to `Java.InteropTests.JavaLangRemappingTestObject, Java.Interop-Tests`, *not* `Java.Lang.Object, Mono.Android`. Arguably a bug, arguably "meh", but this setup triggered some never previously encountered error conditions: 4. `EmbeddedAssemblies::typemap_java_to_managed()` within `libmonodroid.so` returns a `System.Type` that corresponds to a JNI type. `typemap_java_to_managed()` has a bug/corner case wherein it will only provide `Type` instances from assemblies which have already been loaded. Early in startup, `Java.Interop-Tests` hasn't been loaded yet, so when `java/lang/Object` was mapped to `Java.InteropTests.JavaLangRemappingTestObject, Java.Interop-Tests`, `typemap_java_to_managed()` would return `null`. This is a bug/corner case, which is being investigated in #8625. 5. Calls to `Java.Lang.Object.GetObject<T>()` call `Java.Interop.TypeManager.CreateInstance()`, which loops through the type and all base types to find a known binding/wrapper. Because of (3)+(4), if (when) we try to find the wrapper for `java/lang/Object`, we would find *no* mapping. This would cause an `JNI DETECTED ERROR IN APPLICATION` *crash*. This was due to a "use after free" bug. See the "TypeManager.CreateInstance() Use After Free Bug" section. 6. Once (5) is fixed we encounter our next issue: the `Java.InteropTests.JnienvTest.NewOpenGenericTypeThrows()` unit test started failing because `crc641855b07eca6dcc03.GenericHolder_1` couldn't be found. This was caused by a bug in `acw-map.txt` parsing within `<R8/>`. See the "`<R8/>` and `acw-map.txt` parsing.`" section. 7. Once (6) was fixed, (3) caused a *new* set of failures: multiple tests started failing because `java/lang/Object` was being mapped to the wrong managed type. (3) becomes less "meh" and more "definitely a bug". See the "Correct `java/lang/Object` mappings" section. *Now* things should work reliably. ~~ TypeManager.CreateInstance() Use After Free Bug ~~ On 2011-Oct-19, xamarin/monodroid@e3e4f123d8 introduced a use-after-free bug within `TypeManager.CreateInstance()`: JNIEnv.DeleteRef (handle, transfer); throw new NotSupportedException ( FormattableString.Invariant ($"Internal error finding wrapper class for '{JNIEnv.GetClassNameFromInstance (handle)}'. (Where is the Java.Lang.Object wrapper?!)"), CreateJavaLocationException ()); `handle` *cannot be used* after `JNIEnv.DeleteRef(handle)`. Failure to do so results in a `JNI DETECTED ERROR IN APPLICATION` crash; with `adb shell setprop debug.mono.log lref+` set, we see: I monodroid-lref: +l+ lrefc 1 handle 0x71/L from thread '(null)'(1) D monodroid-gref: at Android.Runtime.AndroidObjectReferenceManager.CreatedLocalReference(JniObjectReference , Int32& ) D monodroid-gref: at Java.Interop.JniRuntime.JniObjectReferenceManager.CreatedLocalReference(JniEnvironmentInfo , JniObjectReference ) D monodroid-gref: at Java.Interop.JniEnvironment.LogCreateLocalRef(JniObjectReference ) D monodroid-gref: at Java.Interop.JniEnvironment.LogCreateLocalRef(IntPtr ) D monodroid-gref: at Java.Interop.JniEnvironment.InstanceMethods.CallObjectMethod(JniObjectReference , JniMethodInfo ) D monodroid-gref: … … I monodroid-lref: -l- lrefc 0 handle 0x71/L from thread '(null)'(1) D monodroid-gref: at Android.Runtime.AndroidObjectReferenceManager.DeleteLocalReference(JniObjectReference& , Int32& ) D monodroid-gref: at Java.Interop.JniRuntime.JniObjectReferenceManager.DeleteLocalReference(JniEnvironmentInfo , JniObjectReference& ) D monodroid-gref: at Java.Interop.JniObjectReference.Dispose(JniObjectReference& reference) D monodroid-gref: at Android.Runtime.JNIEnv.DeleteLocalRef(IntPtr ) D monodroid-gref: at Android.Runtime.JNIEnv.DeleteRef(IntPtr , JniHandleOwnership ) D monodroid-gref: at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) D monodroid-gref: at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type ) D monodroid-gref: at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership ) D monodroid-gref: at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer) D monodroid-gref: … D monodroid-gref: E droid.NET_Test: JNI ERROR (app bug): accessed stale Local 0x71 (index 7 in a table of size 7) F droid.NET_Test: java_vm_ext.cc:570] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x71 … F droid.NET_Test: runtime.cc:630] native: #13 pc 00000000003ce865 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::GetObjectClass(_JNIEnv*, _jobject*)+837) The immediate fix is Don't Do That™; use a temporary: class_name = JNIEnv.GetClassNameFromInstance (handle); JNIEnv.DeleteRef (handle, transfer); throw new NotSupportedException ( FormattableString.Invariant ($"Internal error finding wrapper class for '{class_name}'. (Where is the Java.Lang.Object wrapper?!)"), CreateJavaLocationException ()); Unfortunately, *just* fixing the "use-after-free" bug is insufficient; if we throw that `NotSupportedException`, things *will* break elsewhere. We'll just have an "elegant unhandled exception" app crash instead of a "THE WORLD IS ENDING" failed assertion crash. We could go with the simple fix for the crash, but this means that in order to integrate dotnet/java-interop@005c9141 & dotnet/java-interop@07c73009 we'd have to figure out how to *ensure* that `java/lang/Object` is bound as `Java.Lang.Object, Mono.Android`, not `Java.InteropTests.JavaLangRemappingTestObject, Java.Interop-Tests`. (We actually need to do this *anyway*; see the "Correct `java/lang/Object` mappings" section. At the time we I was trying to *avoid* special-casing `Mono.Android.dll`…) There is a*slightly* more complicated approach which fixes (5) while supporting (4) `typemap_java_to_managed()` returning null; consider the `-l-` callstack: at Android.Runtime.JNIEnv.DeleteRef(IntPtr , JniHandleOwnership ) at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type ) at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership ) at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer) at Android.Runtime.JavaSet.Iterator() This is part of a generic `Object.GetObject<IIterator>()` invocation! Additionally, because `IIterator` is an interface, in *normal* use the `type` variable within `TypeManager.CreateInstance()` would be `Java.Lang.Object, Mono.Android` and then *immediately discarded* because `Java.Lang.Object` cannot be assigned to `IIterator`. Moving the type compatibility check to *before* the `type == null` check fixes *an* issue with `typemap_java_to_managed()` returning null. ~~ `<R8/>` and `acw-map.txt` parsing.` ~~ There are many ways for Android+Java code to refer to managed types. For example, consider the following View subclass: namespace Example { partial class MyCoolView : Android.Views.View { // … } } Within layout `.axml` files, you can mention an `Android.Views.View` subclass by: * Using the .NET Full Class Name as an element name. <Example.MyCoolView /> * Using the .NET Full Class Name with a *lowercased* namespace name as the element name. <example.MyCoolView /> * Use the Java-side name directly. <crc64….NiftyView /> Within Fragments, you can also use the *assembly-qualified name*: <fragment class="Example.MyCoolView, AssemblyName" /> At build time, all instances of the .NET type names will be *replaced* with the Java type names before the Android toolchain processes the files. The association between .NET type names and Java names is stored within `$(IntermediateOutputPath)acw-map.txt`, which was introduced in xamarin/monodroid@a04b73b3. *Normally* `acw-map.txt` contains three entries: 1. The fully-qualified .NET type name 2. The .NET type name, no assembly 3. (2) with a lowercased namespace name, *or* the `[Register]` value, if provided. For example: Mono.Android_Test.Library.CustomTextView, Mono.Android-Test.Library.NET;crc6456ab8145c81c4100.CustomTextView Mono.Android_Test.Library.CustomTextView;crc6456ab8145c81c4100.CustomTextView mono.android_test.library.CustomTextView;crc6456ab8145c81c4100.CustomTextView Java.InteropTests.GenericHolder`1, Java.Interop-Tests;net.dot.jni.test.tests.GenericHolder Java.InteropTests.GenericHolder`1;net.dot.jni.test.tests.GenericHolder net.dot.jni.test.tests.GenericHolder;net.dot.jni.test.tests.GenericHolder However, when warning XA4214 is emitted (efbec22), there is a "collision" on the .NET side (but *not* the Java side); (2) and (3) are potentially *ambiguous*, so one .NET type is arbitrarily chosen. (Collisions on the Java side result in XA4215 *errors*.) The first line is still possible, because of assembly qualification. Enter ``Java.InteropTests.GenericHolder`1``: this type is present in *both* `Java.Interop-Tests.dll` *and* `Mono.Android-Tests.dll`. dotnet/java-interop@07c73009, this was "fine" because the `GenericHolder<T>` within `Java.Interop-Tests.dll` did not participate in typemap generation. Now it does, resulting in the XA4214 warning. XA4214 *also* means that instead of three lines, it's *one* line: Java.InteropTests.GenericHolder`1, Mono.Android.NET-Tests;crc641855b07eca6dcc03.GenericHolder_1 Enter `<R8/>`, which parses `acw-map.txt` to create a `proguard_project_primary.cfg` file. `<R8/>` did it's *own* parsing of `acw-map.txt`, parsing only *one of every three lines*, on the assumption that *all* entries took three lines. This breaks in the presence of XA4214, because some entries only take one line, not three lines. This in turn meant that `proguard_project_primary.cfg` could *miss* types, which could mean that `r8` would *remove* the unspecified types, resulting in `ClassNotFoundException` at runtime: Java.Lang.ClassNotFoundException : crc641855b07eca6dcc03.GenericHolder_1 ----> Java.Lang.ClassNotFoundException : Didn't find class "crc641855b07eca6dcc03.GenericHolder_1" on path: DexPathList[[zip file "/data/app/Mono.Android.NET_Tests-2stBqO43ov5F6bHfYemJHQ==/base.apk", zip file "/data/app/Mono.Android.NET_Tests-2stBqO43ov5F6bHfYemJHQ==/split_config.x86_64.apk", zip file "/data/app/Mono.Android.NET_Tests-2stBqO43ov5F6bHfYemJHQ==/split_config.xxhdpi.apk"],nativeLibraryDirectories=[/data/app/Mono.Android.NET_Tests-2stBqO43ov5F6bHfYemJHQ==/lib/x86_64, /system/fake-libs64, /data/app/Mono.Android.NET_Tests-2stBqO43ov5F6bHfYemJHQ==/base.apk!/lib/x86_64, /data/app/Mono.Android.NET_Tests-2stBqO43ov5F6bHfYemJHQ==/split_config.x86_64.apk!/lib/x86_64, /data/app/Mono.Android.NET_Tests-2stBqO43ov5F6bHfYemJHQ==/split_config.xxhdpi.apk!/lib/x86_64, /system/lib64, /system/product/lib64]] at Java.Interop.JniEnvironment.StaticMethods.CallStaticObjectMethod(JniObjectReference , JniMethodInfo , JniArgumentValue* ) at Android.Runtime.JNIEnv.FindClass(String ) Update `<R8/>` to instead use `MonoAndroidHelper.LoadMapFile()`, which reads all lines within `acw-map.txt`. This results in a `proguard_project_primary.cfg` file which properly contains a `-keep` entry for XA4214-related types, such as `crc641855b07eca6dcc03.GenericHolder_1`. ~~ Correct `java/lang/Object` mappings ~~` Previous valiant efforts to allow `java/lang/Object` to be mapped to "anything", not just `Java.Lang.Object, Mono.Android`, eventually resulted in lots of unit test failures, e.g.: `Android.RuntimeTests.XmlReaderPullParserTest.ToLocalJniHandle()`: System.NotSupportedException : Unable to activate instance of type Java.InteropTests.JavaLangRemappingTestObject from native handle 0x19 (key_handle 0x2408476). ----> System.MissingMethodException : No constructor found for Java.InteropTests.JavaLangRemappingTestObject::.ctor(System.IntPtr, Android.Runtime.JniHandleOwnership) ----> Java.Interop.JavaLocationException : Exception_WasThrown, Java.Interop.JavaLocationException at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership ) at Android.Runtime.XmlResourceParserReader.FromNative(IntPtr , JniHandleOwnership ) at Android.Runtime.XmlResourceParserReader.FromJniHandle(IntPtr handle, JniHandleOwnership transfer) at Android.Content.Res.Resources.GetXml(Int32 ) at Android.RuntimeTests.XmlReaderPullParserTest.ToLocalJniHandle() at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags ) --MissingMethodException at Java.Interop.TypeManager.CreateProxy(Type , IntPtr , JniHandleOwnership ) at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) With a partially heavy heart, we need to special-case typemap entries by processing `Mono.Android.dll` *first*, so that it gets first dibs at bindings for `java/lang/Object` and other types. Update `NativeTypeMappingData` to process types from `Mono.Android` before processing any other module. Note that the special-casing needs to happen in `NativeTypeMappingData` because typemaps were formerly processed in *sorted module order*, in which the sort order is based on the *byte representation* of the module's MVID (a GUID). Additionally, *linking changes the MVID*, which means module order is *effectively random*. Consequently, trying to special case typemap ordering anywhere else is ineffective. ~~ Other ~~ Update `JavaCompileToolTask` to log the contents of its response file. Update LLVM-IR -related types within `src/Xamarin.Android.Build.Tasks/Utilities` to use `TaskLoggingHelper` for logging purposes, *not* `Action<string>`. Update related types to accept `TaskLoggingHelper`, so that we can more easily add diagnostic messages to these types in the future.
jonpryor
added a commit
to dotnet/android
that referenced
this pull request
Feb 2, 2024
Context: dotnet/java-interop@005c914 Context: dotnet/java-interop#1181 Context: 25d1f00 When attempting to bump to dotnet/java-interop@005c9141, multiple unit tests would fail, e.g. Java.Lang.LinkageError : net.dot.jni.test.CallVirtualFromConstructorDerived ----> System.NotSupportedException : Could not find System.Type corresponding to Java type JniTypeSignature(TypeName=net/dot/jni/test/CallVirtualFromConstructorDerived ArrayRank=0 Keyword=False) . This happened because dotnet/java-interop@005c9141 implicitly required that typemaps exist for `Java.Interop.JavaObject` subclasses. Fair enough; enter xamasrin/java.interop#1181, which added support to `Java.Interop.Tools.JavaCallableWrappers` to emit typemaps for `Java.Interop.JavaObject` subclasses. That caused *crashes* in `tests/Mono.Android-Tests`: E droid.NET_Test: JNI ERROR (app bug): accessed stale Local 0x75 (index 7 in a table of size 7) F droid.NET_Test: java_vm_ext.cc:570] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x75 … F droid.NET_Test: runtime.cc:630] native: #13 pc 00000000003ce865 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::GetObjectClass(_JNIEnv*, _jobject*)+837) The immediate cause of the crash was a "use after free" bug within `TypeManager.CreateInstance()` in a never-hit-before error path; the "use after free" bug was fixed in 25d1f00. However, the cause of the hitting a never-hit-before error path is because `EmbeddedAssemblies::typemap_java_to_managed()` would only map Java types to `System.Type` instances for assemblies that have *already been loaded*. If the assembly had not yet been loaded, then `EmbeddedAssemblies::typemap_java_to_managed()` would return `null`, and if the binding it couldn't find happens to be for `java.lang.Object`, we hit the (buggy!) "Where is the Java.Lang.Object wrapper" error condition. Commit 25d1f00 fixes that and a great many other related issues. What's left is `EmbeddedAssemblies::typemap_java_to_managed()`: it should *never* return null *unless* there is no typemap at all. Whether the target assembly has been loaded or not should be irrelevant. Update `EmbeddedAssemblies::typemap_java_to_managed()` so that it will load the target assembly if necessary. Additionally, before we figured out that we had a "use after free" bug, all we had to go on was that *something* related to `JNIEnv::GetObjectClass()` was involved. Review JNI usage around `JNIEnv::GetObjectClass()` and related invocations, and cleanup: * Simplify logic related to `JNIEnv::DeleteLocalRef()`. * Decrease scope of local variables. * Clear variables passed to `JNIEnv.DeleteLocalRef()`. Co-authored-by: Jonathan Pryor <jonpryor@vt.edu> Co-authored-by: Marek Habersack <grendel@twistedcode.net>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: dotnet/android#8543
Context: dotnet/android#8625
Context: #1168
Context: def5bc0
Context: 005c914
dotnet/android#8543 tested PR #1168, was Totally Green™ -- finding no issues -- and so we merged PR #1168 into 005c914.
Enter dotnet/android#8625, which bumps xamarin-android to use def5bc0, which includes 005c914. dotnet/android#8625 contains failing unit tests (?!), including
Java.InteropTests.InvokeVirtualFromConstructorTests()::shocked-pikachu-face: (But dotnet/android#8543 was green!)
The problem is twofold:
005c914 now requires the presence of typemap entries from e.g.
Java.InteropTests.CallVirtualFromConstructorDerivedtonet.dot.jni.test.CallVirtualFromConstructorDerived.Java.Interop.Tools.JavaCallableWrapperset al doesn't create typemap entries forJava.Interop.JavaObjectsubclasses which have[JniTypeSignature].Consequently, our units tests fail (and apparently weren't run on dotnet/android#8543?! Still not what happened.)
Fix typemap generation by adding a new
TypeDefinition.HasJavaPeer()extension method to replace all the.IsSubclassOf("Java.Lang.Object")and similar checks, extending it to also check forJava.Interop.JavaObjectandJava.Interop.JavaExceptionbase types. (Continuing to use base type checks is done instead of just relying on implementation ofJava.Interop.IJavaPeerableas a performance optimization, as there could be lots of interface types to check.)Additionally, @jonathanpeppers -- while trying to investigate all this -- ran across a build failure:
This suggests that
Java.Interop.Tools.JavaCallableWrapperswas encounteringJava.Interop.JavaObject-- or some other type which has[JniTypeSignature("java/lang/Object")]-- which is whyjava/lang/Object.javawas being generated.Audit all
[JniTypeSignature]attributes, and addGenerateJavaPeer=falseto all types which should not hava a Java Callable Wrapper generated for them. This includes nearly everything withinJava.Interop-Tests.dll. (We want the typemaps! We don't want generated Java source, as we have hand-written Java peer types for those tests.)Aside: this project includes T4 Text Templates. To regenerate the output files without involving Visual Studio, you can install the
dotnet-t4tool:then run it separately for each
.ttfile: