Commit f0305ba
committed
Fix "JNI DETECTED ERROR IN APPLICATION: use of deleted local reference"
Context: https://github.com/xamarin/monodroid/commit/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!1 parent 9798fb8 commit f0305ba
1 file changed
Lines changed: 8 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
286 | 286 | | |
287 | 287 | | |
288 | 288 | | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
289 | 295 | | |
| 296 | + | |
290 | 297 | | |
291 | 298 | | |
292 | | - | |
| 299 | + | |
293 | 300 | | |
294 | 301 | | |
295 | 302 | | |
296 | | - | |
297 | | - | |
298 | | - | |
299 | 303 | | |
300 | 304 | | |
301 | 305 | | |
| |||
0 commit comments