Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

media: fix race condition on async release

Browse Source 
Clear Java object state at releaseAsync(), and delete native object
upon async release complete.
Clearing CodecBase object only led to a rare race condition which
caused null pointer dereference.

Bug: 158501286
Test: atest CtsMediaTestCases:MediaCodecTest#testAsyncRelease (100 times)
Test: atest CtsMediaTestCases:MediaCodecCapabilitiesTest#testGetMaxSupportedInstances
Test: atest CtsMediaTestCases -- --module-arg CtsMediaTestCases:size:small
Change-Id: I691d39007c0ea770318f4038558ad338252bd2fb
This commit is contained in:
Wonsik Kim
2020-06-18 23:52:03 -07:00
parent 721a3ac836
commit e37ef4bc92
1 changed files with 15 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
media/jni/android_media_MediaCodec.cpp
View File

@@ -235,7 +235,10 @@ void JMediaCodec::release() {
void JMediaCodec::releaseAsync() { void JMediaCodec::releaseAsync() {
std::call_once(mAsyncReleaseFlag, [this] { std::call_once(mAsyncReleaseFlag, [this] {
if (mCodec != NULL) { if (mCodec != NULL) {
mCodec->releaseAsync(new AMessage(kWhatAsyncReleaseComplete, this)); sp<AMessage> notify = new AMessage(kWhatAsyncReleaseComplete, this);
// Hold strong reference to this until async release is complete
notify->setObject("this", this);
mCodec->releaseAsync(notify);
} }
mInitStatus = NO_INIT; mInitStatus = NO_INIT;
}); });
@@ -1088,8 +1091,11 @@ void JMediaCodec::onMessageReceived(const sp<AMessage> &msg) {
} }
case kWhatAsyncReleaseComplete: case kWhatAsyncReleaseComplete:
{ {
mCodec.clear(); if (mLooper != NULL) {
mLooper->stop(); mLooper->unregisterHandler(id());
mLooper->stop();
mLooper.clear();
}
break; break;
} }
default: default:
@@ -1104,7 +1110,7 @@ void JMediaCodec::onMessageReceived(const sp<AMessage> &msg) {
using namespace android; using namespace android;
static sp<JMediaCodec> setMediaCodec( static sp<JMediaCodec> setMediaCodec(
JNIEnv *env, jobject thiz, const sp<JMediaCodec> &codec) { JNIEnv *env, jobject thiz, const sp<JMediaCodec> &codec, bool release = true) {
sp<JMediaCodec> old = (JMediaCodec *)env->CallLongMethod(thiz, gFields.lockAndGetContextID); sp<JMediaCodec> old = (JMediaCodec *)env->CallLongMethod(thiz, gFields.lockAndGetContextID);
if (codec != NULL) { if (codec != NULL) {
codec->incStrong(thiz); codec->incStrong(thiz);
@@ -1115,7 +1121,9 @@ static sp<JMediaCodec> setMediaCodec(
* its message handler, doing release() from there will deadlock * its message handler, doing release() from there will deadlock
* (as MediaCodec::release() post synchronous message to the same looper) * (as MediaCodec::release() post synchronous message to the same looper)
*/ */
old->release(); if (release) {
old->release();
}
old->decStrong(thiz); old->decStrong(thiz);
} }
env->CallVoidMethod(thiz, gFields.setAndUnlockContextID, (jlong)codec.get()); env->CallVoidMethod(thiz, gFields.setAndUnlockContextID, (jlong)codec.get());
@@ -1130,7 +1138,8 @@ static sp<JMediaCodec> getMediaCodec(JNIEnv *env, jobject thiz) {
} }
static void android_media_MediaCodec_release(JNIEnv *env, jobject thiz) { static void android_media_MediaCodec_release(JNIEnv *env, jobject thiz) {
sp<JMediaCodec> codec = getMediaCodec(env, thiz); // Clear Java native reference.
sp<JMediaCodec> codec = setMediaCodec(env, thiz, nullptr, false /* release */);
if (codec != NULL) { if (codec != NULL) {
codec->releaseAsync(); codec->releaseAsync();
} }