commit 128b012cc8c2369136bb8450bc91c81aebe18506
author Chong Zhang <chz@google.com> Sat Mar 01 18:04:13 2014 -0800
committer Chong Zhang <chz@google.com> Mon Mar 03 12:29:49 2014 -0800
tree c0054ed0e2473117e4aa7def54a6eedc638199d3
parent 6ef3c2cbbb72201e9b2216fb96d071e0fac654d1

fix MediaCodec release deadlock and resource leak

Bug: 12910147
Change-Id: I5f2c02b6ad5bfcd88797c6aa79dcaa23d030eacf

media/jni/android_media_MediaCodec.cpp

diff --git a/media/jni/android_media_MediaCodec.cpp b/media/jni/android_media_MediaCodec.cpp
index 3ce483d..b2fb2df 100644
--- a/media/jni/android_media_MediaCodec.cpp
+++ b/media/jni/android_media_MediaCodec.cpp
@@ -112,12 +112,35 @@
     mLooper->registerHandler(this);
 }
 
-JMediaCodec::~JMediaCodec() {
+void JMediaCodec::release() {
     if (mCodec != NULL) {
         mCodec->release();
         mCodec.clear();
     }
 
+    if (mLooper != NULL) {
+        mLooper->unregisterHandler(id());
+        mLooper->stop();
+        mLooper.clear();
+    }
+}
+
+JMediaCodec::~JMediaCodec() {
+    if (mCodec != NULL || mLooper != NULL) {
+        /* MediaCodec and looper should have been released explicitly already
+         * in setMediaCodec() (see comments in setMediaCodec()).
+         *
+         * Otherwise JMediaCodec::~JMediaCodec() might be called from within the
+         * message handler, doing release() there risks deadlock as MediaCodec::
+         * release() post synchronous message to the same looper.
+         *
+         * Print a warning and try to proceed with releasing.
+         */
+        ALOGW("try to release MediaCodec from JMediaCodec::~JMediaCodec()...");
+        release();
+        ALOGW("done releasing MediaCodec from JMediaCodec::~JMediaCodec().");
+    }
+
     JNIEnv *env = AndroidRuntime::getJNIEnv();
 
     env->DeleteWeakGlobalRef(mObject);
@@ -432,6 +455,12 @@
         codec->incStrong(thiz);
     }
     if (old != NULL) {
+        /* release MediaCodec and stop the looper now before decStrong.
+         * otherwise JMediaCodec::~JMediaCodec() could be called from within
+         * its message handler, doing release() from there will deadlock
+         * (as MediaCodec::release() post synchronous message to the same looper)
+         */
+        old->release();
         old->decStrong(thiz);
     }
     env->SetLongField(thiz, gFields.context, (jlong)codec.get());

media/jni/android_media_MediaCodec.h

diff --git a/media/jni/android_media_MediaCodec.h b/media/jni/android_media_MediaCodec.h
index 53254c9..2f2ea96 100644
--- a/media/jni/android_media_MediaCodec.h
+++ b/media/jni/android_media_MediaCodec.h
@@ -42,6 +42,7 @@
     status_t initCheck() const;
 
     void registerSelf();
+    void release();
 
     status_t configure(
             const sp<AMessage> &format,