Deleting the global ref of the observerCallback in JNI makes the offline activity to crash

[jfirebaugh]

A typical log is like the following:

art  F  art/runtime/java_vm_ext.cc:410] JNI DETECTED ERROR IN APPLICATION: use of deleted global reference 0x1005b6
     F  art/runtime/java_vm_ext.cc:410] "Offline Thread" prio=5 tid=33 Runnable
     F  art/runtime/java_vm_ext.cc:410]   | group="main" sCount=0 dsCount=0 obj=0x13261b80 self=0xd7837900
     F  art/runtime/java_vm_ext.cc:410]   | sysTid=10721 nice=0 cgrp=default sched=0/0 handle=0xdbcfa930
     F  art/runtime/java_vm_ext.cc:410]   | state=R schedstat=( 208107565 6797710 101 ) utm=8 stm=12 core=4 HZ=100
     F  art/runtime/java_vm_ext.cc:410]   | stack=0xdbbfe000-0xdbc00000 stackSize=1014KB
     F  art/runtime/java_vm_ext.cc:410]   | held mutexes= "mutator lock"(shared held)
     F  art/runtime/java_vm_ext.cc:410]   native: #00 pc 0035a55f  /system/lib/libart.so (art::DumpNativeStack(std::__1::basic_ostream<char,
         std::__1::char_traits<char> >&, int, char const*, art::ArtMethod*, void*)+126)
     F  art/runtime/java_vm_ext.cc:410]   native: #01 pc 0033b0e7  /system/lib/libart.so (art::Thread::Dump(std::__1::basic_ostream<char, st
        d::__1::char_traits<char> >&) const+138)
     F  art/runtime/java_vm_ext.cc:410]   native: #02 pc 0024dfcb  /system/lib/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)
        +750)
     F  art/runtime/java_vm_ext.cc:410]   native: #03 pc 0024e6d3  /system/lib/libart.so (art::JavaVMExt::JniAbortF(char const*, char const*
        , ...)+62)
     F  art/runtime/java_vm_ext.cc:410]   native: #04 pc 0033be33  /system/lib/libart.so (art::Thread::DecodeJObject(_jobject*) const+678)
     F  art/runtime/java_vm_ext.cc:410]   native: #05 pc 000ffa5f  /system/lib/libart.so (art::ScopedCheck::Check(art::ScopedObjectAccess&, 
        bool, char const*, art::JniValueType*) (.constprop.95)+890)
     F  art/runtime/java_vm_ext.cc:410]   native: #06 pc 0010e87d  /system/lib/libart.so (art::CheckJNI::CallMethodV(char const*, _JNIEnv*, 
        _jobject*, _jclass*, _jmethodID*, std::__va_list, art::Primitive::Type, art::InvokeType)+496)
     F  art/runtime/java_vm_ext.cc:410]   native: #07 pc 0010fa2f  /system/lib/libart.so (art::CheckJNI::CallVoidMethodV(_JNIEnv*, _jobject*
        , _jmethodID*, std::__va_list)+30)
     F  art/runtime/java_vm_ext.cc:410]   native: #08 pc 000e185c  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (_JNIEnv
        ::CallVoidMethod(_jobject*, _jmethodID*, ...)+56)
     F  art/runtime/java_vm_ext.cc:410]   native: #09 pc 000f6768  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (???)
     F  art/runtime/java_vm_ext.cc:410]   native: #10 pc 00214fe4  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (???)
     F  art/runtime/java_vm_ext.cc:410]   native: #11 pc 002090d8  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (???)
     F  art/runtime/java_vm_ext.cc:410]   native: #12 pc 001f3c84  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (mbgl::u
        til::RunLoop::process()+352)
     F  art/runtime/java_vm_ext.cc:410]   native: #13 pc 00373994  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (???)
     F  art/runtime/java_vm_ext.cc:410]   native: #14 pc 00373bc0  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (???)
     F  art/runtime/java_vm_ext.cc:410]   native: #15 pc 0037d18c  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (???)
     F  art/runtime/java_vm_ext.cc:410]   native: #16 pc 00374128  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (uv_run+
        428)
     F  art/runtime/java_vm_ext.cc:410]   native: #17 pc 002035e8  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (void mb
        gl::util::Thread<mbgl::DefaultFileSource::Impl>::run<std::__1::tuple<std::__1::basic_string<char, std::__1::char_traits<char>, std::
        __1::allocator<char> > const&, unsigned long long&>, 0u, 1u>(mbgl::util::ThreadContext, std::__1::tuple<std::__1::basic_string<char,
         std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long long&>&&, std::__1::integer_sequence<unsigned int, 0
        u, 1u>)+164)
     F  art/runtime/java_vm_ext.cc:410]   native: #18 pc 002034d0  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (mbgl::u
        til::Thread<mbgl::DefaultFileSource::Impl>::Thread<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<cha
        r> > const&, unsigned long long&>(mbgl::util::ThreadContext const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::_
        _1::allocator<char> > const&&&, unsigned long long&&&)::'lambda'()::operator()() const+236)
     F  art/runtime/java_vm_ext.cc:410]   native: #19 pc 00203394  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so (std::__
        1::__thread_proxy<std::__1::tuple<mbgl::util::Thread<mbgl::DefaultFileSource::Impl>::Thread<std::__1::basic_string<char, std::__1::c
        har_traits<char>, std::__1::allocator<char> > const&, unsigned long long&>(mbgl::util::ThreadContext const&, std::__1::basic_string<
        char, std::__1::char_traits<char>, std::__1::allocator<char> > const&&&, unsigned long long&&&)::'lambda'()> >(void*, void*)+88)
     F  art/runtime/java_vm_ext.cc:410]   native: #20 pc 0003f883  /system/lib/libc.so (__pthread_start(void*)+30)
     F  art/runtime/java_vm_ext.cc:410]   native: #21 pc 00019f75  /system/lib/libc.so (__start_thread+6)
     F  art/runtime/java_vm_ext.cc:410]   (no managed stack frames)
     F  art/runtime/java_vm_ext.cc:410]

I'm gonna remove the DeleteGlobalRef(observerCallback); calls for now while we figure out a better way to do this. This doesn't affect the same mechanism for lambdas.

/cc: @jfirebaugh

[jfirebaugh]

👀 @zugaldia

This should fix the leak, although in my tests I couldn't actually get ~Observer or destroyOfflineRegion to be called. Does the test app keep Java-side references to all OfflineRegions somewhere?

[zugaldia]

This should fix the leak, although in my tests I couldn't actually get ~Observer or destroyOfflineRegion to be called.

This is probably because it never reached GC, which we have no control over. If we need to be more deterministic about when finalize() is called, then we need to make that call explicit in the app, for example during the Activity's onDestroy().