Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash in wakelock.c regarding releasing JNI references. #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

calvarez-ov
Copy link

@calvarez-ov calvarez-ov commented Jul 28, 2016

This fixes a crash when a thread exits without detaching from jni.

The crash:

07-27 15:05:33.840   371   371 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
07-27 15:05:33.840   371   371 F DEBUG   : Build fingerprint: 'google/shamu/shamu:6.0.1/MMB29X/2704508:user/release-keys'
07-27 15:05:33.840   371   371 F DEBUG   : Revision: '0'
07-27 15:05:33.840   371   371 F DEBUG   : ABI: 'arm'
07-27 15:05:33.840   371   371 F DEBUG   : pid: 18852, tid: 18941, name: Thread-2067  >>> com.example.myapp <<<
07-27 15:05:33.840   371   371 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x9c
07-27 15:05:33.844  1375  1375 D PhoneStatusBar: disable: < expand icons* alerts system_info* back home recent clock search quick_settings >
07-27 15:05:33.876   371   371 F DEBUG   : Abort message: 'art/runtime/thread.cc:1237] Native thread exited without calling DetachCurrentThread: Thread[45,tid=18941,Native,Thread*=0x95cda500,peer=0x12f278
e0,"Thread-2067"]'
07-27 15:05:33.877   371   371 F DEBUG   :     r0 00000000  r1 00000010  r2 0000000a  r3 b4cf8bf8
07-27 15:05:33.877   371   371 F DEBUG   :     r4 00000000  r5 b4cfc928  r6 00000000  r7 9c6ba57c
07-27 15:05:33.877   371   371 F DEBUG   :     r8 fffff7dc  r9 fffff7d0  sl 9c6ba548  fp 9c6ba54c
07-27 15:05:33.878   371   371 F DEBUG   :     ip 00000000  sp 9c6ba4b8  lr b4989c89  pc b4bf1286  cpsr 400f0030
07-27 15:05:33.896   371   371 F DEBUG   :
07-27 15:05:33.896   371   371 F DEBUG   : backtrace:
07-27 15:05:33.897   371   371 F DEBUG   :     #00 pc 0035a286  /system/lib/libart.so (art::DumpCheckpoint::Run(art::Thread*)+205)
07-27 15:05:33.897   371   371 F DEBUG   :     #01 pc 0035af31  /system/lib/libart.so (art::ThreadList::RunCheckpoint(art::Closure*)+212)
07-27 15:05:33.897   371   371 F DEBUG   :     #02 pc 0035b45f  /system/lib/libart.so (art::ThreadList::Dump(std::__1::basic_ostream<char, std::__1::char_traits<char> >&)+142)
07-27 15:05:33.897   371   371 F DEBUG   :     #03 pc 00333a35  /system/lib/libart.so (art::Runtime::Abort()+424)
07-27 15:05:33.898   371   371 F DEBUG   :     #04 pc 000f45fb  /system/lib/libart.so (art::LogMessage::~LogMessage()+2226)
07-27 15:05:33.898   371   371 F DEBUG   :     #05 pc 0034d21d  /system/lib/libart.so (art::Thread::ThreadExitCallback(void*)+172)
07-27 15:05:33.898   371   371 F DEBUG   :     #06 pc 0003f9dd  /system/lib/libc.so (pthread_key_clean_all()+76)
07-27 15:05:33.898   371   371 F DEBUG   :     #07 pc 0003f73f  /system/lib/libc.so (pthread_exit+34)
07-27 15:05:33.898   371   371 F DEBUG   :     #08 pc 0003f461  /system/lib/libc.so (__pthread_start(void*)+32)
07-27 15:05:33.899   371   371 F DEBUG   :     #09 pc 00019b43  /system/lib/libc.so (__start_thread+6)

We only saw this crash when using the setLogHandler() api in java.

@@ -61,7 +62,7 @@ void belle_sip_wake_lock_uninit(JNIEnv *env) {
* @param data Unused.
*/
static void jni_key_cleanup(void *data) {
JNIEnv *env = pthread_getspecific(ctx.jniEnvKey);
JNIEnv *env = (JNIEnv*) data;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://linux.die.net/man/3/pthread_key_create

In the documentation of pthread_key_create, it says that upon thread destruction, the value is set to NULL and then the destructor (this function) is called with the value as an argument.

An optional destructor function may be associated with each key value. At thread exit, if a key value has a non-NULL destructor pointer, and the thread has a non-NULL value associated with that key, the value of the key is set to NULL, and then the function pointed to is called with the previously associated value as its sole argument. The order of destructor calls is unspecified if more than one destructor exists for a thread when it exits.

When I read this, I don't understand how this ever worked. It looks like pthread_getspecific() is supposed to return NULL when called from the destructor (this function).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Carmen,
The documentation for the pthread_key_create on Android (https://android.googlesource.com/platform/external/pthreads/+/master/pthread_key_create.c) in a bit different:

This function creates a thread-specific data key visible to all threads.
All existing and new threads have a value NULL for key until set using pthread_setspecific.
When any thread with a non-NULL value for key terminates, 'destructor' is called with key's current value for that thread.

@calvarez-ov calvarez-ov changed the title [ON-40378] Fix crash in wakelock.c regarding releasing JNI references. Fix crash in wakelock.c regarding releasing JNI references. Jul 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants