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

Commit 9e0302fc authored by Jayant Chowdhary's avatar Jayant Chowdhary
Browse files

AImage: don't allow ~AImageReader to run before AImages are deleted. 



We can never be sure whether ~AImageReader() has run to completion or
not in AImage::close(). So we move clean up to another AImageReader
method and make sure that's called when in AImageReader_delete. AImage
now contains an sp<> to AImageReader so we can be sure that its members
wouldn't have been destroyed by ~AImageReader and we can check whether
AImageReader_delete has been called on it.

This also prevents us from deadlocking since AImage_delete could also cause
~AImageReader to run with AImageReader::mLock held in AImage::close().

Bug: 137571625
Bug: 137694217

Test: enroll; auth
Test: cts native AImageReader, camera, graphics tests

Change-Id: If5803cb6fcb6f4800032069872daaeac1cd36ed2
Signed-off-by: default avatarJayant Chowdhary <jchowdhary@google.com>
parent 4c6d9952

media/ndk/NdkImage.cpp

+5 −19
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ AImage::AImage(AImageReader* reader, int32_t format, uint64_t usage, BufferItem* 
        int64_t timestamp, int32_t width, int32_t height, int32_t numPlanes) :

        mReader(reader), mFormat(format), mUsage(usage), mBuffer(buffer), mLockedBuffer(nullptr),

        mTimestamp(timestamp), mWidth(width), mHeight(height), mNumPlanes(numPlanes) {

    LOG_FATAL_IF(reader == nullptr, "AImageReader shouldn't be null while creating AImage");

}



AImage::~AImage() {
@@ -57,14 +58,9 @@ AImage::close(int releaseFenceFd) { 
    if (mIsClosed) {

        return;

    }

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

    if (reader != nullptr) {

        reader->releaseImageLocked(this, releaseFenceFd);

    } else if (mBuffer != nullptr) {

        LOG_ALWAYS_FATAL("%s: parent AImageReader closed without releasing image %p",

                __FUNCTION__, this);

    if (!mReader->mIsClosed) {

        mReader->releaseImageLocked(this, releaseFenceFd);

    }



    // Should have been set to nullptr in releaseImageLocked

    // Set to nullptr here for extra safety only

    mBuffer = nullptr;
@@ -83,22 +79,12 @@ AImage::free() { 


void

AImage::lockReader() const {

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

    if (reader == nullptr) {

        // Reader has been closed

        return;

    }

    reader->mLock.lock();

    mReader->mLock.lock();

}



void

AImage::unlockReader() const {

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

    if (reader == nullptr) {

        // Reader has been closed

        return;

    }

    reader->mLock.unlock();

    mReader->mLock.unlock();

}



media_status_t

media/ndk/NdkImagePriv.h

+1 −1
Original line number Diff line number Diff line
@@ -72,7 +72,7 @@ struct AImage { 
    uint32_t getJpegSize() const;



    // When reader is close, AImage will only accept close API call

    wp<AImageReader>           mReader;

    const sp<AImageReader>     mReader;

    const int32_t              mFormat;

    const uint64_t             mUsage;  // AHARDWAREBUFFER_USAGE_* flags.

    BufferItem*                mBuffer;

media/ndk/NdkImageReader.cpp

+11 −1
Original line number Diff line number Diff line
@@ -272,6 +272,11 @@ AImageReader::AImageReader(int32_t width, 
      mFrameListener(new FrameListener(this)),

      mBufferRemovedListener(new BufferRemovedListener(this)) {}



AImageReader::~AImageReader() {

    Mutex::Autolock _l(mLock);

    LOG_FATAL_IF("AImageReader not closed before destruction", mIsClosed != true);

}



media_status_t

AImageReader::init() {

    PublicFormat publicFormat = static_cast<PublicFormat>(mFormat);
@@ -347,8 +352,12 @@ AImageReader::init() { 
    return AMEDIA_OK;

}



AImageReader::~AImageReader() {

void AImageReader::close() {

    Mutex::Autolock _l(mLock);

    if (mIsClosed) {

        return;

    }

    mIsClosed = true;

    AImageReader_ImageListener nullListener = {nullptr, nullptr};

    setImageListenerLocked(&nullListener);
@@ -741,6 +750,7 @@ EXPORT 
void AImageReader_delete(AImageReader* reader) {

    ALOGV("%s", __FUNCTION__);

    if (reader != nullptr) {

        reader->close();

        reader->decStrong((void*) AImageReader_delete);

    }

    return;

media/ndk/NdkImageReaderPriv.h

+2 −0
Original line number Diff line number Diff line
@@ -76,6 +76,7 @@ struct AImageReader : public RefBase { 
    int32_t        getHeight()    const { return mHeight; };

    int32_t        getFormat()    const { return mFormat; };

    int32_t        getMaxImages() const { return mMaxImages; };

    void           close();



  private:
@@ -165,6 +166,7 @@ struct AImageReader : public RefBase { 
    native_handle_t*           mWindowHandle = nullptr;



    List<AImage*>              mAcquiredImages;

    bool                       mIsClosed = false;



    Mutex                      mLock;

};