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

Commit ebca5b98 authored by Jayant Chowdhary's avatar Jayant Chowdhary
Browse files

AImageReader: make sure ~AImageReader isn't called with FrameListener::mLock held. 



The following sequence of events is possible:

t1: FrameListener::onFrameAvailable callback is called, mReader is
    promoted from wp<> to sp<>, t1 holds mLock.
  t2: AImageReader_delete is called, decStrong is called on AImageReader,
      but since its refcount isn't 0, ~AImageReader isn't called
    t1: onFrameAvailable completes, ~AImageReader is called with mLock
        held, ~AImageReader->
        setImageListenerLocked->FrameListener::setImageListener->tries
        to lock mLock again, t1 deadlocks.

We move the locking mLock to after the promotion of mReader to sp<> so
that it gets destructed before ~AImageReader is called.

The same is done for BufferRemovedListener.

Bug: 136193631

Test: Auth; unlock

Change-Id: I8bb8c7d59f3711fd9fe434159095938eb5db9153
Signed-off-by: default avatarJayant Chowdhary <jchowdhary@google.com>
parent 265eb995

media/ndk/NdkImageReader.cpp

+2 −2
Original line number Diff line number Diff line
@@ -113,12 +113,12 @@ AImageReader::getNumPlanesForFormat(int32_t format) { 


void

AImageReader::FrameListener::onFrameAvailable(const BufferItem& /*item*/) {

    Mutex::Autolock _l(mLock);

    sp<AImageReader> reader = mReader.promote();

    if (reader == nullptr) {

        ALOGW("A frame is available after AImageReader closed!");

        return; // reader has been closed

    }

    Mutex::Autolock _l(mLock);

    if (mListener.onImageAvailable == nullptr) {

        return; // No callback registered

    }
@@ -143,12 +143,12 @@ AImageReader::FrameListener::setImageListener(AImageReader_ImageListener* listen 


void

AImageReader::BufferRemovedListener::onBufferFreed(const wp<GraphicBuffer>& graphicBuffer) {

    Mutex::Autolock _l(mLock);

    sp<AImageReader> reader = mReader.promote();

    if (reader == nullptr) {

        ALOGW("A frame is available after AImageReader closed!");

        return; // reader has been closed

    }

    Mutex::Autolock _l(mLock);

    if (mListener.onBufferRemoved == nullptr) {

        return; // No callback registered

    }

media/ndk/NdkImageReaderPriv.h

+2 −2
Original line number Diff line number Diff line
@@ -134,7 +134,7 @@ struct AImageReader : public RefBase { 


      private:

        AImageReader_ImageListener mListener = {nullptr, nullptr};

        wp<AImageReader>           mReader;

        const wp<AImageReader>     mReader;

        Mutex                      mLock;

    };

    sp<FrameListener> mFrameListener;
@@ -149,7 +149,7 @@ struct AImageReader : public RefBase { 


       private:

        AImageReader_BufferRemovedListener mListener = {nullptr, nullptr};

        wp<AImageReader>           mReader;

        const wp<AImageReader>     mReader;

        Mutex                      mLock;

    };

    sp<BufferRemovedListener> mBufferRemovedListener;