Skip to content

Commit

Permalink
Audit JNIEnv::GetObjectClass() invocations
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jonpryor
jonpryor committed Jan 26, 2024
1 parent 7b46391 commit b4ca587
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 9 deletions.
17 changes: 14 additions & 3 deletions src/Mono.Android/Java.Interop/TypeManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,14 @@ internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership
[UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "TypeManager.CreateProxy() does not statically know the value of the 'type' local variable.")]
internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership transfer, Type? targetType)
{
if (handle == IntPtr.Zero) {
throw new NotSupportedException (
FormattableString.Invariant ($"Internal error: `handle` is NULL!"),
CreateJavaLocationException ());
}
Type? type = null;
IntPtr class_ptr = JNIEnv.GetObjectClass (handle);
string class_name = GetClassName (class_ptr);
string? class_name = GetClassName (class_ptr);
lock (TypeManagerMapDictionaries.AccessLock) {
while (class_ptr != IntPtr.Zero && !TypeManagerMapDictionaries.JniToManaged.TryGetValue (class_name, out type)) {

Expand All @@ -279,12 +284,18 @@ internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership

IntPtr super_class_ptr = JNIEnv.GetSuperclass (class_ptr);
JNIEnv.DeleteLocalRef (class_ptr);
class_name = null;
class_ptr = super_class_ptr;
class_name = GetClassName (class_ptr);
if (class_ptr != IntPtr.Zero) {
class_name = GetClassName (class_ptr);
}
}
}

JNIEnv.DeleteLocalRef (class_ptr);
if (class_ptr != IntPtr.Zero) {
JNIEnv.DeleteLocalRef (class_ptr);
class_ptr = IntPtr.Zero;
}

if (type == null) {
JNIEnv.DeleteRef (handle, transfer);
Expand Down
9 changes: 3 additions & 6 deletions src/monodroid/jni/osbridge.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -657,15 +657,13 @@ OSBridge::add_reference_jobject (JNIEnv *env, jobject handle, jobject reffed_han

java_class = env->GetObjectClass (handle);
add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V");
env->DeleteLocalRef (java_class);
if (add_method_id) {
env->CallVoidMethod (handle, add_method_id, reffed_handle);
env->DeleteLocalRef (java_class);

return 1;
}

env->ExceptionClear ();
env->DeleteLocalRef (java_class);
return 0;
}

Expand Down Expand Up @@ -910,7 +908,6 @@ OSBridge::gc_cleanup_after_java_collection (JNIEnv *env, int num_sccs, MonoGCBri
MonoClass *klass;
#endif
MonoObject *obj;
jclass java_class;
jobject jref;
jmethodID clear_method_id;
int i, j, total, alive, refs_added;
Expand Down Expand Up @@ -942,8 +939,9 @@ OSBridge::gc_cleanup_after_java_collection (JNIEnv *env, int num_sccs, MonoGCBri
sccs [i]->is_alive = 1;
mono_field_get_value (obj, bridge_info->refs_added, &refs_added);
if (refs_added) {
java_class = env->GetObjectClass (jref);
jclass java_class = env->GetObjectClass (jref);
clear_method_id = env->GetMethodID (java_class, "monodroidClearReferences", "()V");
env->DeleteLocalRef (java_class);
if (clear_method_id) {
env->CallVoidMethod (jref, clear_method_id);
} else {
Expand All @@ -957,7 +955,6 @@ OSBridge::gc_cleanup_after_java_collection (JNIEnv *env, int num_sccs, MonoGCBri
}
#endif
}
env->DeleteLocalRef (java_class);
}
} else {
abort_unless (!sccs [i]->is_alive, "Bridge SCC at index %d must NOT be alive", i);
Expand Down
1 change: 1 addition & 0 deletions tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Environment.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
debug.mono.log=lref+,gref+
4 changes: 4 additions & 0 deletions tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Mono.Android.NET-Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@
<Compile Remove="..\Mono.Data.Sqlite\SqliteTests.cs" />
</ItemGroup>

<ItemGroup>
<AndroidEnvironment Include="Environment.txt" />
</ItemGroup>

<ItemGroup Condition=" '$(AndroidPackageFormat)' != 'aab' ">
<TestApk Include="$(OutputPath)$(_MonoAndroidTestPackage)-Signed.apk">
<Package>$(_MonoAndroidTestPackage)</Package>
Expand Down

0 comments on commit b4ca587

Please sign in to comment.