JNI: C++ to Java method call crashes

1,895 views
Skip to first unread message

jordan juras

unread,
May 15, 2018, 11:20:42 AM5/15/18
to android-ndk
Hi all,

I have been trying different approaches to dealing with a JNI issue with little success and I would like to find out if anyone has faced similar threading issues in this forum.
We are building an Android app with integrated Crashlytics to log our native crashes. Unfortunately when the issue I am writing about appears in our crashlytics webview, we are given only a single line describing the method in which the crash was caused. Ie. no stack trace. I will highlight this crashing method in detail below.

In order to call Java methods from the native code base, I am following this 'standard' procedure:

i) initialize JNI, and cache a global JavaVM* jvm:
env->GetJavaVM(&jvm);
obj = env->NewGlobalRef(object);
jclass cls = env->GetObjectClass(obj);

ii) all of the Java methods needed to be accessed by the native side live in a single java class, from where the initialization of the JNI is invoked, so the jclass cls is used to cache jmethodIDs as follows:
(example for one jmethodID)
someMethodToBeCalledJAVA_CLASSNAME = env->GetMethodID(cls, "method1Name", "()V");

iii) when this method is called from the C++ code, I determine whether the current thread needs to be attached to the jvm, and call the method as follows:
void methodName() {

env_forMethod1 = checkJNIEnvStatus(&thread_status[method1Idx]);

if (env_forMethod1 != NULL) {

env_forMethod->CallVoidMethod(obj,someMethodToBeCalledJAVA_CLASSNAME);

if (thread_status[method1Idx] == THREAD_CONNECTION_STATUS_SUCCESS) {
jvm->DetachCurrentThread();
} else {
//LOGD("thread already connected, will be detached elsewhere");
}
}
}
The goal is to ALWAYS try to attach the thread to the jvm before making the call, and to ALWAYS detach the thread. In the case that the thread was already connected, we do not detach the thread because we can assume that if for some reason we are sharing the thread with another method that has already attached itself to the jvm, then it will detach itself in the same procedure.

v) checkJNIStatus(int* status) {} is a method that will check the thread state, and save the value in an array called thread_status[], of size equal to the number of different C++ to Java methods we have in our program. Each index of the array corresponds to a different env_forMethodx, and are indexed with a human readable enum :
typedef enum JNI_BROADCAST_GROUPS {

method1Idx,
method2Idx,
method3Idx,
ETCIdx

} JNI_BROADCAST_GROUPS;

It is within checkJNISatus() that we have our crash:
JNIEnv* checkJNIEnvStatus(int *status) {

JNIEnv* env;
int getEnvStatus = jvm->GetEnv((void**)&env, JNI_VERSION_1_6);

if (getEnvStatus == JNI_EDETACHED) {

//LOGD("SBP Thread detached, must attach now");

if (jvm->AttachCurrentThread(&env,NULL) != THREAD_CONNECTION_STATUS_SUCCESS) {
LOGE("Failed to attach env for JNI");
status[0] = THREAD_CONNECTION_FAILURE;
return NULL;
} else {
status[0] = THREAD_CONNECTION_STATUS_SUCCESS;
return env;
//LOGD("Attach Success");
}
} else if (getEnvStatus == JNI_OK) {
//LOGD("Thread Already Attached");
status[0] = THREAD_ALREADY_CONNECTED;
return env;

} else if (getEnvStatus == JNI_EVERSION) {
LOGE("Unsupported JNI version");
status[0] = THREAD_CONNECTION_FAILURE;
return NULL; ;
} else {
// default
LOGE("Default Thread Management return null");
status[0] = THREAD_CONNECTION_STATUS_UNDEFINED;
return NULL;
}
}

Unfortunately, we are only able to know that the crash happens within the checkJNIStatus() method. Furthermore the affects only about 2% of our users. Each method that invokes checkJNIStatus() is touched with much greater frequency than would lead to a conclusion that one single method call has a typo. Rather, the crash seems to be the result of some asynchronous threading problems.

The Crashlytics log is attached.


Any help would be fantastic. !!

Thanks,

JJ
checkJNIStatus() Crashlytics Log.png

Athos Fabio Bacchiocchi

unread,
May 15, 2018, 11:08:31 PM5/15/18
to andro...@googlegroups.com
You could try handling the thread detachment from the jvm automatically with a callback when the thread is destroyed, as adviced here:


Athos


Athos Bacchiocchi
Android Developer @ BandLab

--
You received this message because you are subscribed to the Google Groups "android-ndk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to android-ndk+unsubscribe@googlegroups.com.
To post to this group, send email to andro...@googlegroups.com.
Visit this group at https://groups.google.com/group/android-ndk.
To view this discussion on the web visit https://groups.google.com/d/msgid/android-ndk/0e78a46e-35b1-46d8-92b0-3dc80ab87ada%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Alex Cohn

unread,
May 24, 2018, 6:23:31 AM5/24/18
to android-ndk
On Wednesday, May 16, 2018 at 6:08:31 AM UTC+3, Athos Bacchiocchi wrote:
You could try handling the thread detachment from the jvm automatically with a callback when the thread is destroyed, as adviced here:


This is still a very good article, even though it was written many years ago. For me, the main takeaway is that the pattern of Attach/Detach on every call is not optimal for Android; instead, Attach the current thread on each JNI call unconditionally (it's NOP for attached thread), and use pthread_key_create to define a destructor function that will be called before the thread exits.

For me, this approach also avoids some strange corner cases, and is much more robust.
 
Athos
To unsubscribe from this group and stop receiving emails from it, send an email to android-ndk...@googlegroups.com.

jordan juras

unread,
May 24, 2018, 11:21:44 AM5/24/18
to andro...@googlegroups.com
Hey Alex, Athos,

Thanks for the suggestions. I have implemented the key-based thread destructors and in our testing environement it seems really stable. Furthermore, performance is improved as this implementation results in never unnecessarily detaching the thread.

Thanks ! This has been getting to me for a while.

JJ

To unsubscribe from this group and stop receiving emails from it, send an email to android-ndk+unsubscribe@googlegroups.com.

To post to this group, send email to andro...@googlegroups.com.
Visit this group at https://groups.google.com/group/android-ndk.

mic _

unread,
May 30, 2018, 4:37:09 AM5/30/18
to andro...@googlegroups.com
Is there a guarantee that a keyed pthread destructor won't be invoked on a Java thread being destroyed? It's not a big deal to call pthread_setspecific conditionally based on whether GetEnv returned EDETACHED, but I'd like to keep the code as simple as possible, so if I can eliminate the call to GetEnv then that's even better. I don't want to end up in a situation where on _some_ devices I _sometimes_ get a SIGABRT from trying to detach a Java thread.

/Michael



Alex Cohn

unread,
May 30, 2018, 8:46:46 AM5/30/18
to android-ndk
On Wednesday, May 30, 2018 at 11:37:09 AM UTC+3, mic wrote:
Is there a guarantee that a keyed pthread destructor won't be invoked on a Java thread being destroyed? It's not a big deal to call pthread_setspecific conditionally based on whether GetEnv returned EDETACHED, but I'd like to keep the code as simple as possible, so if I can eliminate the call to GetEnv then that's even better. I don't want to end up in a situation where on _some_ devices I _sometimes_ get a SIGABRT from trying to detach a Java thread.

/Michael

I'm afraid there is no such guarantee, and yours is a valid concern, and checking GetEnv() is the way to achieve this.

In most of my scenarios, I don't worry about this, because I know at compile time which threads are created as native, and I usually wrap them in some C++ class that takes care of this initialization.

Sincerely,
Alex
 

--
You received this message because you are subscribed to the Google Groups "android-ndk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to android-ndk...@googlegroups.com.
To post to this group, send email to andro...@googlegroups.com.
Visit this group at https://groups.google.com/group/android-ndk.
Reply all
Reply to author
Forward
0 new messages