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

[monodroid] typemaps may need to load assemblies #8625

Merged
merged 11 commits into from
Feb 2, 2024

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jan 11, 2024

Bumps external/Java.Interop from 8b85462 to def5bc0.

Commits

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Bumps [external/Java.Interop](https://github.com/xamarin/java.interop) from `8b85462` to `def5bc0`.
- [Commits](dotnet/java-interop@8b85462...def5bc0)

---
updated-dependencies:
- dependency-name: external/Java.Interop
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot requested a review from jonpryor as a code owner January 11, 2024 02:05
@dependabot dependabot bot added dependencies Pull requests that update a dependency file. submodules Pull requests that update Submodules code labels Jan 11, 2024
@jonpryor
Copy link
Member

Mono.Android.NET_Tests, Java.InteropTests.InvokeVirtualFromConstructorTests fails:

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)

…because we don't have a type mapping from net/dot/jni/test/CallVirtualFromConstructorDerived to Java.InteropTests.CallVirtualFromConstructorDerived, which happens because before dotnet/java-interop@005c914 ManagedPeer.RegisterNativeMembers() would use Type.GetType() to lookup the type corresponding to a Java Callable Wrapper, but with dotnet/java-interop@005c914 we now go through JniRuntime.JniTypeManager.GetType(JniTypeSignature), which goes through the typemap infrastructure in xamarin-android, and there is no type mapping for CallVirtualFromConstructorDerived:

% grep 'Java.InteropTests\.'  tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/obj/Debug/net9.0-android/android/typemaps.x86_64.ll
@.TypeMapEntry.10487_from = private unnamed_addr constant [66 x i8] c"Java.InteropTests.ContainsExportedMethods, Mono.Android.NET-Tests\00", align 16
@.TypeMapEntry.10489_from = private unnamed_addr constant [58 x i8] c"Java.InteropTests.GenericHolder`1, Mono.Android.NET-Tests\00", align 16
@.TypeMapEntry.10491_from = private unnamed_addr constant [56 x i8] c"Java.InteropTests.IAndroidInterface, Java.Interop-Tests\00", align 16
@.TypeMapEntry.10493_from = private unnamed_addr constant [47 x i8] c"Java.InteropTests.MyCb, Mono.Android.NET-Tests\00", align 16
@.TypeMapEntry.10495_from = private unnamed_addr constant [51 x i8] c"Java.InteropTests.MyObject, Mono.Android.NET-Tests\00", align 16
@.TypeMapEntry.10497_from = private unnamed_addr constant [50 x i8] c"Java.InteropTests.MyPaint, Mono.Android.NET-Tests\00", align 16
@.TypeMapEntry.10499_from = private unnamed_addr constant [63 x i8] c"Java.InteropTests.MyRegistrationThread, Mono.Android.NET-Tests\00", align 16
@.TypeMapEntry.10501_from = private unnamed_addr constant [67 x i8] c"Java.InteropTests.RegisterMeOnNewThreadOne, Mono.Android.NET-Tests\00", align 16
@.TypeMapEntry.10503_from = private unnamed_addr constant [67 x i8] c"Java.InteropTests.RegisterMeOnNewThreadTwo, Mono.Android.NET-Tests\00", align 16
@.TypeMapEntry.10505_from = private unnamed_addr constant [69 x i8] c"Java.InteropTests.ThrowableActivatedFromJava, Mono.Android.NET-Tests\00", align 16

(Note: no mention of CallVirtualFromConstructorDerived, or any other type within Java.Interop-Tests.dll.)

Next question: why aren't we producing type mappings for anything within Java.Interop-Tests.dll? jcw-gen appears to support [JniTypeSignature]:

@jonpryor
Copy link
Member

Syncing from Discord

@jonathanpeppers asked:

Is this IsInterface check wrong?

type.IsInterface && type.ImplementsInterface ("Java.Interop.IJavaPeerable", cache)

Yes, it's wrong; the type.IsInterface && should be removed.

and also various .IsSubclassOf() checks in Java.Interop also need updating/rethinking (among others?):

Basically, any code which assumes/requires Java.Lang.Object or Java.Lang.Throwable subclasses will fail wrt CallVirtualFromConstructorDerived and other JavaObject or JavaException subclasses. All of thses should probably just be updated to check for "implements IJavaPeerable".

jonpryor added a commit to dotnet/java-interop that referenced this pull request Jan 24, 2024
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/
@jonpryor
Copy link
Member

Tests still fail, just differently so. We haveJava.InteropTests.JavaObjectTest.Ctor_Exceptions() failing:

https://github.com/xamarin/java.interop/blob/def5bc0df68a5908fac85e5dd4f02f1e27c75882/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs#L168

Presumably because we're now creating a JCW for the type, so I'll need to update the declaration…?

Then we have "unspecified crashes", a'la: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8967688&view=ms.vss-test-web.build-test-results-tab&runId=92698252&paneView=debug&resultId=100000

01-24 03:44:25.617  1743  1743 F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x14 in tid 1743 ([email protected]), pid 1743 ([email protected])
01-24 03:44:25.761  2403  2403 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
01-24 03:44:25.761  2403  2403 F DEBUG   : Build fingerprint: 'Android/sdk_phone_x86_64/generic_x86_64:10/QSR1.210820.001/7663313:userdebug/test-keys'
01-24 03:44:25.761  2403  2403 F DEBUG   : Revision: '0'
01-24 03:44:25.761  2403  2403 F DEBUG   : ABI: 'x86_64'
01-24 03:44:25.761  2403  2403 F DEBUG   : Timestamp: 2024-01-24 03:44:25+0000
01-24 03:44:25.761  2403  2403 F DEBUG   : pid: 1743, tid: 1743, name: [email protected]  >>> /vendor/bin/hw/[email protected] <<<
01-24 03:44:25.761  2403  2403 F DEBUG   : uid: 1000
01-24 03:44:25.761  2403  2403 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x14
01-24 03:44:25.761  2403  2403 F DEBUG   : Cause: null pointer dereference
01-24 03:44:25.761  2403  2403 F DEBUG   :     rax 00007285421ba020  rbx 0000000000000000  rcx 0000000000000000  rdx 0000000000000001
01-24 03:44:25.762  2403  2403 F DEBUG   :     r8  0000000000000003  r9  00007ffcb1de1230  r10 0000000000000000  r11 0000000000000000
01-24 03:44:25.762  2403  2403 F DEBUG   :     r12 00007ffcb1de1200  r13 0000728543650400  r14 00007285436507a8  r15 0000728543621140
01-24 03:44:25.762  2403  2403 F DEBUG   :     rdi 00007285421b9008  rsi 0000000000000000
01-24 03:44:25.762  2403  2403 F DEBUG   :     rbp 00007285425922f0  rsp 00007ffcb1de1018  rip 00007285421b3ec0
01-24 03:44:25.912  2403  2403 F DEBUG   :
01-24 03:44:25.912  2403  2403 F DEBUG   : backtrace:
01-24 03:44:25.912  2403  2403 F DEBUG   :       #00 pc 000000000000aec0  /vendor/lib64/libOpenglSystemCommon.so (GoldfishGralloc::getHostHandle(native_handle const*)) (BuildId: 9e1f1713753fc3439af85bf4faea017d)
01-24 03:44:25.912  2403  2403 F DEBUG   :       #01 pc 000000000000e672  /vendor/lib64/hw/hwcomposer.ranchu.so (android::EmuHWC2::Display::present(int*)+354) (BuildId: f71c058ba159f115677b7d79af7d1a56)
01-24 03:44:25.912  2403  2403 F DEBUG   :       #02 pc 000000000000aa35  /vendor/lib64/hw/[email protected] (android::hardware::graphics::composer::V2_1::passthrough::detail::HwcHalImpl[android::hardware::graphics::composer::V2_1::hal::ComposerHal](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8967688&view=ms.vss-test-web.build-test-results-tab&runId=92698252&paneView=debug&resultId=100000)::presentDisplay(unsigned long, int*, std::__1::vector<unsigned long, std::__1::allocator<unsigned long>>, std::__1::vector<int, std::__1::allocator<int>>)+53) (BuildId: 128e287ebf4eef7e499d1951b9466c5b)
01-24 03:44:25.912  2403  2403 F DEBUG   :       #03 pc 000000000001362a  /vendor/lib64/hw/[email protected] (android::hardware::graphics::composer::V2_1::hal::ComposerCommandEngine::executePresentDisplay(unsigned short)+106) (BuildId: 128e287ebf4eef7e499d1951b9466c5b)
01-24 03:44:25.912  2403  2403 F DEBUG   :       #04 pc 000000000001243d  /vendor/lib64/hw/[email protected] (android::hardware::graphics::composer::V2_1::hal::ComposerCommandEngine::executeCommand(android::hardware::graphics::composer::V2_1::IComposerClient::Command, unsigned short)+221) (BuildId: 128e287ebf4eef7e499d1951b9466c5b)
01-24 03:44:25.912  2403  2403 F DEBUG   :       #05 pc 0000000000010e9b  /vendor/lib64/hw/[email protected] (android::hardware::graphics::composer::V2_1::hal::ComposerCommandEngine::execute(unsigned int, android::hardware::hidl_vec[android::hardware::hidl_handle](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8967688&view=ms.vss-test-web.build-test-results-tab&runId=92698252&paneView=debug&resultId=100000) const&, bool*, unsigned int*, android::hardware::hidl_vec[android::hardware::hidl_handle](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8967688&view=ms.vss-test-web.build-test-results-tab&runId=92698252&paneView=debug&resultId=100000)*)+139) (BuildId: 128e287ebf4eef7e499d1951b9466c5b)
01-24 03:44:25.912  2403  2403 F DEBUG   :       #06 pc 000000000000ddcc  /vend

…in OpenGL?!

There are other crashes as well, which don't trivially provide a backtrace, and thus presumbly aren't related to OpenGL…

@jonathanpeppers
Copy link
Member

The test failure I see now locally is:

Ctor_Exceptions 
 [FAIL] : Expected: <System.NotSupportedException>
  But was:  null
   at Java.InteropTests.JavaObjectTest.Ctor_Exceptions()
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)\
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

I think it's actually just able to create this object now?

https://github.com/xamarin/java.interop/blob/def5bc0df68a5908fac85e5dd4f02f1e27c75882/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs#L168

https://github.com/xamarin/java.interop/blob/def5bc0df68a5908fac85e5dd4f02f1e27c75882/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs#L204-L205

Even if I add this change, same result:

++[JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)]
class JavaObjectWithNoJavaPeer : JavaObject {

This is for situations when, for any reason, Java is calling us before
any of the managed types involved in the mapping had the chance to be
loaded - that is, before their containing assembly has been loaded.

For most apps the overhead of this solution will be negligible (a single
`nullptr` check).
@dependabot dependabot bot requested a review from grendello as a code owner January 25, 2024 12:10
grendello and others added 2 commits January 25, 2024 21:55
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 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.
@jonpryor
Copy link
Member

jonpryor commented Jan 26, 2024

The Story So Far; from b4ca587:

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'
    

    [Java.Interop] Typemap support for JavaObject & [JniTypeSignature] 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: #01 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: #02 pc 00000000005c49cb  /apex/com.android.runtime/lib64/libart.so (art::DumpCheckpoint::Run(art::Thread*)+859)
    F droid.NET_Test: runtime.cc:630]   native: #03 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: #04 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: #05 pc 0000000000552eb9  /apex/com.android.runtime/lib64/libart.so (art::Runtime::Abort(char const*)+1529)
    F droid.NET_Test: runtime.cc:630]   native: #06 pc 000000000000c873  /system/lib64/libbase.so (android::base::LogMessage::~LogMessage()+611)
    F droid.NET_Test: runtime.cc:630]   native: #07 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: #08 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: #09 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
Copy link
Member

Commit b4ca587 attempted to set debug.mono.log to include lref+,gref+, so we could track JNI handles.

Oddly, that failed; JNI handle info wasn't collected into adb logcat. (Feels like something to debug some other time.)

So…

  1. Create an emulator locally, by detaching my physical Android device and running:

    ./dotnet-local.sh build -c Release tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/*.csproj "-t:InstallAvdImage;AcquireAndroidTarget"
    

    This appears to create an API-28 emulator. (Not sure.)

  2. Download the .aab from CI.

  3. "Manipulate" the .aab and install it onto my emulator:

    % java -jar \
       …/xamarin-android/bin/Release/lib/packs/Microsoft.Android.Sdk.Darwin/34.99.0/tools/bundletool.jar \
       build-apks \
       --bundle=app.aab \
       --output=app.apks --connected-device
       --aapt2
       …/xamarin-android/bin/Release/lib/packs/Microsoft.Android.Sdk.Darwin/34.99.0/tools/Darwin/aapt2
    % java -jar \
       …/xamarin-android/bin/Release/lib/packs/Microsoft.Android.Sdk.Darwin/34.99.0/tools/bundletool.jar \
       install-apks \
       --apks=app.apks
    
  4. Run the tests:

     % adb logcat > log.txt &
     % adb shell setprop debug.mono.log "'gref+,lref+'"
     % adb shell am instrument -w Mono.Android.NET_Tests/xamarin.android.runtimetests.NUnitInstrumentation
    

We now have JNI Local & Global ref logs! Huzzah!

The crash:

F DEBUG   : Abort message: 'JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x75

Tracking backwards for reference 0x75, and we see in-order:

I monodroid-lref: +l+ lrefc 1 handle 0x75/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.Types.TryFindClass(String , Boolean )
D monodroid-gref:    at Java.Interop.JniEnvironment.Types.FindClass(String )
D monodroid-gref:    at Java.Interop.JniType..ctor(String )
D monodroid-gref:    at Java.Interop.JniType.GetCachedJniType(JniType& , String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.get_JniPeerType()
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.get_JniPeerType()
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.GetMethodInfo(String , String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.GetMethodInfo(String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeAbstractObjectMethod(String , IJavaPeerable , JniArgumentValue* )
D monodroid-gref:    at Android.OS.Bundle.KeySet()
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-gref: +g+ grefc 11 gwrefc 0 obj-handle 0x75/L -> new-handle 0x2ca6/G from thread '(null)'(1)
D monodroid-gref:    at Android.Runtime.AndroidObjectReferenceManager.CreateGlobalReference(JniObjectReference )
D monodroid-gref:    at Java.Interop.JniObjectReference.NewGlobalRef()
D monodroid-gref:    at Java.Interop.JniType.Initialize(JniObjectReference& , JniObjectReferenceOptions )
D monodroid-gref:    at Java.Interop.JniType..ctor(String )
D monodroid-gref:    at Java.Interop.JniType.GetCachedJniType(JniType& , String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.get_JniPeerType()
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.get_JniPeerType()
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.GetMethodInfo(String , String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.GetMethodInfo(String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeAbstractObjectMethod(String , IJavaPeerable , JniArgumentValue* )
D monodroid-gref:    at Android.OS.Bundle.KeySet()
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 0x75/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 Java.Interop.JniObjectReference.Dispose(JniObjectReference& , JniObjectReferenceOptions )
D monodroid-gref:    at Java.Interop.JniType.Initialize(JniObjectReference& , JniObjectReferenceOptions )
D monodroid-gref:    at Java.Interop.JniType..ctor(String )
D monodroid-gref:    at Java.Interop.JniType.GetCachedJniType(JniType& , String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.get_JniPeerType()
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.get_JniPeerType()
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.GetMethodInfo(String , String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.GetMethodInfo(String )
D monodroid-gref:    at Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeAbstractObjectMethod(String , IJavaPeerable , JniArgumentValue* )
D monodroid-gref:    at Android.OS.Bundle.KeySet()
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)

What's odd is two things:

  1. The above looks reasonable
  2. Time delay; the last -l- message I have is at 01-26 22:19:34.907. The crash is at 01-26 22:19:34.947, .04ms later.

Where is this offending handle coming from?

@jonpryor
Copy link
Member

@jonpryor asked:

Where is this offending handle coming from

Enter PR #8681, which attempts to just test "concerning item 2":

  1. The binding for java/lang/Object is coming from Java.Interop-Tests, not Mono.Android (?!)
    [Java.Interop] Typemap support for JavaObject & [JniTypeSignature] java-interop#1181 has a fix for this, and we're not applying the fix yet because we believe that it will hide (1).

It also crashed, which I didn't expect, but it crashed because dotnet/java-interop#1181 was incomplete; it still contained java/lang/Object bindings which were being used in preference to Java.Lang.Object, Mono.Android.

However, I think we've figured out the JNI crash: f0305ba

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. [Java.Interop] Typemap support for JavaObject & [JniTypeSignature] 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.

https://github.com/xamarin/xamarin-android/blob/9798fb889ee251b65264b0b4637454a3c68ebe91/src/Mono.Android/Java.Interop/TypeManager.cs#L289-L294

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.

Copy link
Contributor Author

dependabot bot commented on behalf of github Jan 29, 2024

A newer version of external/Java.Interop exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

Context: 9a7aee7
Context: 7b46391

Fake a MonoVM hook to try to load the assembly.
jonpryor added a commit to dotnet/java-interop that referenced this pull request Feb 2, 2024
…1181)

Context: dotnet/android#8543
Context: dotnet/android#8625
Context: dotnet/android#8681
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.
    `net.dot.jni.test.CallVirtualFromConstructorDerived` to
    `Java.InteropTests.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 sure what happened.)

Update 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.)

Add `[JniTypeSignature]` to `GenericHolder<T>`.  This type mapping
isn't *actually* required, but it *is* used in `JavaVMFixture`, and
it confuses people (me!) if things are inconsistent.  Additionally,
remove `tests/` from the Java-side name, for consistency.

~~ Avoid multiple java/lang/Object bindings ~~

A funny thing happened when in dotnet/android#8681 -- which
tested this commit -- when using an intermediate version of this
commit: 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, Mono.Android` 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.  This commit *adds* that
support, introducing this unforeseen interaction.

Fix this by *removing* most "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`.

Update `JavaVMFixture` to add the required typemap entries.

---

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/
jonpryor added a commit 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.
PR #8681 was merged, which fixed the Java.Interop bump issues.
@jonpryor
Copy link
Member

jonpryor commented Feb 2, 2024

WIP Commit Message:

[monodroid] typemaps may need to load assemblies (#8625)

Context: https://github.com/xamarin/java.interop/commit/005c914170a0af9069ff18fd4dd9d45463dd5dc6
Context: https://github.com/xamarin/java.interop/pull/1181
Context: 25d1f007a7bd1ca3ce960315fabd04b9a687224e

When attempting to bump to xamarin/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 xamarin/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 25d1f007.

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 25d1f007 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 <[email protected]>
Co-authored-by: Marek Habersack <[email protected]>

@jonpryor jonpryor changed the title Bump external/Java.Interop from 8b85462 to def5bc0 [monodroid] typemaps may need to load assemblies Feb 2, 2024
@jonpryor jonpryor merged commit 06b1d7f into main Feb 2, 2024
45 checks passed
@jonpryor jonpryor deleted the dependabot/submodules/external/Java.Interop-def5bc0 branch February 2, 2024 21:28
grendello added a commit that referenced this pull request Feb 5, 2024
* main:
  [monodroid] typemaps may need to load assemblies (#8625)
  Bump $(AndroidNetPreviousVersion) to 34.0.79 (#8693)
  Bump to xamarin/java.interop/main@07c73009 (#8681)
  Bump to dotnet/installer@1c496970b7 9.0.100-preview.2.24078.1 (#8685)
grendello added a commit that referenced this pull request Feb 5, 2024
* main:
  [monodroid] typemaps may need to load assemblies (#8625)
  Bump $(AndroidNetPreviousVersion) to 34.0.79 (#8693)
  Bump to xamarin/java.interop/main@07c73009 (#8681)
grendello added a commit that referenced this pull request Feb 7, 2024
* main:
  Bump to dotnet/installer@fb7b9a4b9e 9.0.100-preview.2.24106.6 (#8700)
  [Mono.Android] Cache `Profiles/api-34.xml` contents (#8679)
  [monodroid] typemaps may need to load assemblies (#8625)
  Bump $(AndroidNetPreviousVersion) to 34.0.79 (#8693)
  Bump to xamarin/java.interop/main@07c73009 (#8681)
  Bump to dotnet/installer@1c496970b7 9.0.100-preview.2.24078.1 (#8685)
  [GetAndroidDependencies] Add Jdk dependency info (#8651)
  [xaprepare] Add support for newer SparkyLinux (#8684)
grendello added a commit that referenced this pull request Feb 14, 2024
* main: (116 commits)
  [tmt] Update to work with current `libxamarin-app.so` (#8694)
  [Xamarin.Android.Build.Tasks] remove `$(AndroidSupportedAbis)` from `build.props` (#8717)
  [Xamarin.Android.Build.Tasks] BannedApiAnalyzers for Resolve() (#8715)
  Bump to xamarin/Java.Interop/main@dfcbd670 (#8714)
  [monodroid] C++ tweaks and legacy code cleanup (#8638)
  Bump to xamarin/xamarin-android-tools/main@a698a33 (#8710)
  [readme] Add `d17-8` download links. (#8709)
  Bump external/Java.Interop from `07c7300` to `7f08b77` (#8702)
  Bump to xamarin/monodroid@848d1277b7 (#8691)
  [Xamarin.Android.Build.Tasks] `FixAbstractMethodsStep` performance (#8650)
  Bump to dotnet/installer@fb7b9a4b9e 9.0.100-preview.2.24106.6 (#8700)
  [Mono.Android] Cache `Profiles/api-34.xml` contents (#8679)
  [monodroid] typemaps may need to load assemblies (#8625)
  Bump $(AndroidNetPreviousVersion) to 34.0.79 (#8693)
  Bump to xamarin/java.interop/main@07c73009 (#8681)
  Bump to dotnet/installer@1c496970b7 9.0.100-preview.2.24078.1 (#8685)
  [GetAndroidDependencies] Add Jdk dependency info (#8651)
  [xaprepare] Add support for newer SparkyLinux (#8684)
  Bump to dotnet/installer@5680e93cb2 9.0.100-preview.2.24073.12 (#8666)
  $(AndroidPackVersionSuffix)=preview.2; net9 is 34.99.0.preview.2 (#8678)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Pull requests that update a dependency file. submodules Pull requests that update Submodules code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants