Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 349b8b4c authored by Chong Zhang's avatar Chong Zhang
Browse files

stagefright: fix potential codec leaks in async stop/reset 

Currently if stop is followed by a reset from another thread,
we unblock the stop first (to avoid ANR) without waiting for
kWhatShutdownCompleted, and proceed with the reset request.
But this could cause release to return too early when the
kWhatShutdownCompleted for the stop is received. When this
happens, the reset() will unregister ACodec's handlers before
the real shutdown with deallocation is processed, causing
a codec node instance to be leaked.

To solve this problem, separate kWhatShutdownCompleted into
two messages kWhatStopCompleted and kWhatReleaseCompleted.
Release should only return upon kWhatReleaseCompleted.

Also initializing the fence fd in the metadata buffer (even
though it's not used) to avoid warning in freeBuffer.

bug: 32750454
bug: 32971559

Change-Id: If218c853955729a2a2f2e8356908115a79c442e3
parent 4d2bf460

cmds/stagefright/sf2.cpp

+2 −1
Original line number Diff line number Diff line
@@ -230,7 +230,8 @@ protected: 
                    mCodec->signalResume();



                    (new AMessage(kWhatSeek, this))->post(5000000ll);

                } else if (what == CodecBase::kWhatShutdownCompleted) {

                } else if (what == CodecBase::kWhatStopCompleted ||

                        what == CodecBase::kWhatReleaseCompleted) {

                    mDecodeLooper->unregisterHandler(mCodec->id());



                    if (mDecodeLooper != looper()) {

include/media/stagefright/CodecBase.h

+2 −1
Original line number Diff line number Diff line
@@ -43,7 +43,8 @@ struct CodecBase : public AHandler, /* static */ ColorUtils { 
        kWhatFillThisBuffer      = 'fill',

        kWhatDrainThisBuffer     = 'drai',

        kWhatEOS                 = 'eos ',

        kWhatShutdownCompleted   = 'scom',

        kWhatStopCompleted       = 'scom',

        kWhatReleaseCompleted    = 'rcom',

        kWhatFlushCompleted      = 'fcom',

        kWhatError               = 'erro',

        kWhatComponentAllocated  = 'cAll',

media/libstagefright/ACodec.cpp

+11 −7
Original line number Diff line number Diff line
@@ -1201,6 +1201,10 @@ status_t ACodec::allocateOutputMetadataBuffers() { 
        info.mDequeuedAt = mDequeueCounter;



        info.mData = new MediaCodecBuffer(mOutputFormat, new ABuffer(bufferSize));



        // Initialize fence fd to -1 to avoid warning in freeBuffer().

        ((VideoNativeMetadata *)info.mData->base())->nFenceFd = -1;



        info.mCodecData = info.mData;



        err = mOMXNode->useBuffer(kPortIndexOutput, OMXBuffer::sPreset, &info.mBufferID);
@@ -5258,7 +5262,7 @@ bool ACodec::BaseState::onMessageReceived(const sp<AMessage> &msg) { 
            ALOGE_IF("[%s] failed to release codec instance: err=%d",

                       mCodec->mComponentName.c_str(), err);

            sp<AMessage> notify = mCodec->mNotify->dup();

            notify->setInt32("what", CodecBase::kWhatShutdownCompleted);

            notify->setInt32("what", CodecBase::kWhatReleaseCompleted);

            notify->post();

            break;

        }
@@ -6147,7 +6151,8 @@ bool ACodec::UninitializedState::onMessageReceived(const sp<AMessage> &msg) { 
                     "cannot keep component allocated on shutdown in Uninitialized state");



            sp<AMessage> notify = mCodec->mNotify->dup();

            notify->setInt32("what", CodecBase::kWhatShutdownCompleted);

            notify->setInt32("what", keepComponentAllocated ?

                    CodecBase::kWhatStopCompleted : CodecBase::kWhatReleaseCompleted);

            notify->post();



            handled = true;
@@ -6344,7 +6349,8 @@ void ACodec::LoadedState::onShutdown(bool keepComponentAllocated) { 


    if (mCodec->mExplicitShutdown) {

        sp<AMessage> notify = mCodec->mNotify->dup();

        notify->setInt32("what", CodecBase::kWhatShutdownCompleted);

        notify->setInt32("what", keepComponentAllocated ?

                CodecBase::kWhatStopCompleted : CodecBase::kWhatReleaseCompleted);

        notify->post();

        mCodec->mExplicitShutdown = false;

    }
@@ -7346,8 +7352,7 @@ bool ACodec::ExecutingToIdleState::onMessageReceived(const sp<AMessage> &msg) { 


        case kWhatShutdown:

        {

            // We're already doing that...



            mCodec->deferMessage(msg);

            handled = true;

            break;

        }
@@ -7456,8 +7461,7 @@ bool ACodec::IdleToLoadedState::onMessageReceived(const sp<AMessage> &msg) { 
    switch (msg->what()) {

        case kWhatShutdown:

        {

            // We're already doing that...



            mCodec->deferMessage(msg);

            handled = true;

            break;

        }

media/libstagefright/MediaCodec.cpp

+16 −9
Original line number Diff line number Diff line
@@ -1560,19 +1560,26 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) { 
                    break;

                }



                case CodecBase::kWhatShutdownCompleted:

                case CodecBase::kWhatStopCompleted:

                {

                    if (mState == UNINITIALIZED) {

                        // Ignore shutdown complete if we're already released.

                    if (mState != STOPPING) {

                        ALOGW("Received kWhatStopCompleted in state %d", mState);

                        break;

                    }

                    if (mState == STOPPING) {

                    setState(INITIALIZED);

                    } else {

                        CHECK_EQ(mState, RELEASING);

                    (new AMessage)->postReply(mReplyID);

                    break;

                }



                case CodecBase::kWhatReleaseCompleted:

                {

                    if (mState != RELEASING) {

                        ALOGW("Received kWhatReleaseCompleted in state %d", mState);

                        break;

                    }

                    setState(UNINITIALIZED);

                    mComponentName.clear();

                    }



                    mFlags &= ~kFlagIsComponentAllocated;



                    mResourceManagerService->removeResource(getId(mResourceManagerClient));

media/libstagefright/filters/MediaFilter.cpp

+2 −1
Original line number Diff line number Diff line
@@ -676,7 +676,8 @@ void MediaFilter::onShutdown(const sp<AMessage> &msg) { 
    }



    sp<AMessage> notify = mNotify->dup();

    notify->setInt32("what", CodecBase::kWhatShutdownCompleted);

    notify->setInt32("what", keepComponentAllocated ?

            CodecBase::kWhatStopCompleted : CodecBase::kWhatReleaseCompleted);

    notify->post();

}