Skip to content

Commit

Permalink
Fix "JNI DETECTED ERROR IN APPLICATION: use of deleted local reference"
Browse files Browse the repository at this point in the history 
Context: xamarin/monodroid@e3e4f123d8
Context: dotnet/java-interop@005c9141
Context: dotnet/java-interop#1181

We've been trying to track down a JNI error which occurs when trying
to use dotnet/java-interop@005c9141, resembling:

	I monodroid-lref: +l+ lrefc 1 handle 0x71/L from thread '(null)'(1)
	D monodroid-gref:    at Android.Runtime.AndroidObjectReferenceManager.CreatedLocalReference(JniObjectReference , Int32& )
	D monodroid-gref:    at Java.Interop.JniRuntime.JniObjectReferenceManager.CreatedLocalReference(JniEnvironmentInfo , JniObjectReference )
	D monodroid-gref:    at Java.Interop.JniEnvironment.LogCreateLocalRef(JniObjectReference )
	D monodroid-gref:    at Java.Interop.JniEnvironment.LogCreateLocalRef(IntPtr )
	D monodroid-gref:    at Java.Interop.JniEnvironment.InstanceMethods.CallObjectMethod(JniObjectReference , JniMethodInfo )
	D monodroid-gref:    at Android.Runtime.JNIEnv.CallObjectMethod(IntPtr , IntPtr )
	D monodroid-gref:    at Android.Runtime.JavaSet.Iterator()
	D monodroid-gref:    at Android.Runtime.JavaSet`1[[System.String, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetEnumerator()
	D monodroid-gref:    at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ProcessArguments()
	D monodroid-gref:    at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnCreate(Bundle arguments)
	D monodroid-gref:    at Android.App.Instrumentation.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_arguments)
	D monodroid-gref:    at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0)
	…
	I monodroid-lref: -l- lrefc 0 handle 0x71/L from thread '(null)'(1)
	D monodroid-gref:    at Android.Runtime.AndroidObjectReferenceManager.DeleteLocalReference(JniObjectReference& , Int32& )
	D monodroid-gref:    at Java.Interop.JniRuntime.JniObjectReferenceManager.DeleteLocalReference(JniEnvironmentInfo , JniObjectReference& )
	D monodroid-gref:    at Java.Interop.JniObjectReference.Dispose(JniObjectReference& reference)
	D monodroid-gref:    at Android.Runtime.JNIEnv.DeleteLocalRef(IntPtr )
	D monodroid-gref:    at Android.Runtime.JNIEnv.DeleteRef(IntPtr , JniHandleOwnership )
	D monodroid-gref:    at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type )
	D monodroid-gref:    at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type )
	D monodroid-gref:    at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership )
	D monodroid-gref:    at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer)
	D monodroid-gref:    at Android.Runtime.JavaSet.Iterator()
	D monodroid-gref:    at Android.Runtime.JavaSet`1[[System.String, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetEnumerator()
	D monodroid-gref:    at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ProcessArguments()
	D monodroid-gref:    at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnCreate(Bundle arguments)
	D monodroid-gref:    at Android.App.Instrumentation.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_arguments)
	D monodroid-gref:    at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0)
	D monodroid-gref:
	E droid.NET_Test: JNI ERROR (app bug): accessed stale Local 0x71  (index 7 in a table of size 7)
	F droid.NET_Test: java_vm_ext.cc:570] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x71
	…
	F droid.NET_Test: runtime.cc:630]   native: #13 pc 00000000003ce865  /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::GetObjectClass(_JNIEnv*, _jobject*)+837)

This has been "fun".

The problem:

 1. dotnet/java-interop@005c9141 relies on/requires additional
    typemaps in order to "fix" some linker warnings.

    This felt "fine" at the time.

 2. However, the Java.Interop *unit tests* which test (1) involve
    "hand-written" typemap entries to allow things to work.

 3. In .NET Android, those hand-written typemap entries aren't used;
    instead, the normal .NET Android typemaps are used.

 4. .NET Android typemaps did not contain entries for the types
    introduced in (2), so various tests started failing.

 5. dotnet/java-interop#1181 attempts to fix this by extending
    Java Callable Wrappers and associated typemaps to support
    `Java.Interop.JavaObject` subclasses, which brings the new types
    in (2) into the "normal" typemap fold.

 6. However, some of those types "alias" `java.lang.Object`, and --
    for some "bizarre" random ordering reason -- a type within
    `Java.Interop-Tests.dll` becomes the preferred `System.Type`
    to return when looking up `java/lang/Object`.

 7. Which would *probably* be okay (if *really* weird), except that
    `GetJavaToManagedType()` returns null when the binding is within
    an assembly that hasn't been loaded yet.  As this codepath is
    getting hit during app startup, `Java.Interop-Tests` hasn't been
    loaded, so `GetJavaToManagedType()` returns null.

 8. Which means we're now in the scenario of being unable to find a
    binding/"wrapper class" for `java/lang/Object`, which we consider
    to be an error state.

Because it's an error state, we dutifully throw.

…except we've never actually hit this error state before --
HOW COULD WE?! -- which means we've found a bug in our error handling.

Quick, find the problem!

	JNIEnv.DeleteRef (handle, transfer);
	throw new NotSupportedException (
			FormattableString.Invariant ($"Internal error finding wrapper class for '{JNIEnv.GetClassNameFromInstance (handle)}'. (Where is the Java.Lang.Object wrapper?!)"),
			CreateJavaLocationException ());

The problem is a "use after free" bug:
`JNIEnv.DeleteRef(handle, transfer)` *invalidates `handle`*, and then
*immediately* afterward we call
`JNIEnv.GetClassNameFromInstance(handle)`, on the now invalid value.

BOOM goes the Android runtime.

(The `DeleteRef()` call was introduced in xamarin/monodroid@e3e4f123d8,
on 2011-Oct-19.  Over 12 years to encounter this scenario!)

Unfortunately, *just* fixing the "use-after-free" bug is insufficient;
if we throw that `NotSupportedException`, things *will* break
elsewhere.  We'll just have an "elegant unhandled exception" app crash
instead of a "THE WORLD IS ENDING" failed assertion crash.

We could go with the simple fix for the crash, but this means that in
order to integrate dotnet/java-interop@005c9141 &
dotnet/java-interop#1181 we'd have to figure out how to *ensure* that
`java/lang/Object` is bound as `Java.Lang.Object, Mono.Android`, not
`Java.InteropTests.JavaLangRemappingTestObject, Java.Interop-Tests`.

There may be a *slightly* more complicated fix which fixes both issues:
consider the `-l-` callstack:

	at Android.Runtime.JNIEnv.DeleteRef(IntPtr , JniHandleOwnership )
	at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type )
	at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type )
	at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership )
	at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer)
	at Android.Runtime.JavaSet.Iterator()

This is part of a generic `Object.GetObject<IIterator>()` invocation!
Additionally, because `IIterator` is an interface, in *normal* use
the `type` variable within `TypeManager.CreateInstance()` would be
`Java.Lang.Object, Mono.Android` and then ~immediately "discarded"
because `Java.Lang.Object` cannot be assigned to `IIterator`.

If we move the type compatibility check to *before* the
`type == null` check, we *may* also fix the
"`java/lang/Object` is bound as some unloadable type" issue.

Let's try that!
  • Loading branch information
@jonpryor
jonpryor committed Jan 27, 2024
1 parent 9798fb8 commit f0305ba
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions src/Mono.Android/Java.Interop/TypeManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,16 +286,20 @@ internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership

JNIEnv.DeleteLocalRef (class_ptr);

if (targetType != null &&
(type == null ||
!targetType.IsAssignableFrom (type))) {
type = targetType;
}

if (type == null) {
class_name = JNIEnv.GetClassNameFromInstance (handle);
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?!)"),
FormattableString.Invariant ($"Internal error finding wrapper class for '{class_name}'. (Where is the Java.Lang.Object wrapper?!)"),
CreateJavaLocationException ());
}

if (targetType != null && !targetType.IsAssignableFrom (type))
type = targetType;

if (type.IsInterface || type.IsAbstract) {
var invokerType = JavaObjectExtensions.GetInvokerType (type);
if (invokerType == null)
Expand Down

0 comments on commit f0305ba

Please sign in to comment.