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

Commit 9504eb91 authored by Jesse Hall's avatar Jesse Hall
Browse files

Fix race condition in ConsumerBase::addReleaseFence() 

This needs the ConsumerBase mutex locked, but wasn't locking it. Two
of the four places that called it already held the lock so were fine.
Now addReleaseFence() takes the lock itself, and I added
addReleaseFenceLocked() for the two already-locked callers, since in
one of them dropping the lock would be inconvenient.

Bug: 7289269
Change-Id: I7a5628adb516f8eec782aa6c14128202f96d7b0a
parent 0e8fcc2c

include/gui/ConsumerBase.h

+3 −2
Original line number Original line Diff line number Diff line
@@ -150,16 +150,17 @@ protected: 
    // Derived classes should override this method to perform any cleanup that

    // Derived classes should override this method to perform any cleanup that

    // must take place when a buffer is released back to the BufferQueue.  If

    // must take place when a buffer is released back to the BufferQueue.  If

    // it is overridden the derived class's implementation must call

    // it is overridden the derived class's implementation must call

    // ConsumerBase::acquireBufferLocked.

    // ConsumerBase::releaseBufferLocked.

    virtual status_t releaseBufferLocked(int buf, EGLDisplay display,

    virtual status_t releaseBufferLocked(int buf, EGLDisplay display,

           EGLSyncKHR eglFence);

           EGLSyncKHR eglFence);





    // addReleaseFence adds the sync points associated with a fence to the set

    // addReleaseFence* adds the sync points associated with a fence to the set

    // of sync points that must be reached before the buffer in the given slot

    // of sync points that must be reached before the buffer in the given slot

    // may be used after the slot has been released.  This should be called by

    // may be used after the slot has been released.  This should be called by

    // derived classes each time some asynchronous work is kicked off that

    // derived classes each time some asynchronous work is kicked off that

    // references the buffer.

    // references the buffer.

    status_t addReleaseFence(int slot, const sp<Fence>& fence);

    status_t addReleaseFence(int slot, const sp<Fence>& fence);

    status_t addReleaseFenceLocked(int slot, const sp<Fence>& fence);





    // Slot contains the information and object references that

    // Slot contains the information and object references that

    // ConsumerBase maintains about a BufferQueue buffer slot.

    // ConsumerBase maintains about a BufferQueue buffer slot.

libs/gui/BufferItemConsumer.cpp

+1 −1
Original line number Original line Diff line number Diff line
@@ -82,7 +82,7 @@ status_t BufferItemConsumer::releaseBuffer(const BufferItem &item, 




    Mutex::Autolock _l(mMutex);

    Mutex::Autolock _l(mMutex);





    err = addReleaseFence(item.mBuf, releaseFence);

    err = addReleaseFenceLocked(item.mBuf, releaseFence);





    err = releaseBufferLocked(item.mBuf, EGL_NO_DISPLAY,

    err = releaseBufferLocked(item.mBuf, EGL_NO_DISPLAY,

            EGL_NO_SYNC_KHR);

            EGL_NO_SYNC_KHR);

libs/gui/ConsumerBase.cpp

+6 −1
Original line number Original line Diff line number Diff line
@@ -193,7 +193,12 @@ status_t ConsumerBase::acquireBufferLocked(BufferQueue::BufferItem *item) { 
}

}





status_t ConsumerBase::addReleaseFence(int slot, const sp<Fence>& fence) {

status_t ConsumerBase::addReleaseFence(int slot, const sp<Fence>& fence) {

    CB_LOGV("addReleaseFence: slot=%d", slot);

    Mutex::Autolock lock(mMutex);

    return addReleaseFenceLocked(slot, fence);

}



status_t ConsumerBase::addReleaseFenceLocked(int slot, const sp<Fence>& fence) {

    CB_LOGV("addReleaseFenceLocked: slot=%d", slot);





    if (!mSlots[slot].mFence.get()) {

    if (!mSlots[slot].mFence.get()) {

        mSlots[slot].mFence = fence;

        mSlots[slot].mFence = fence;

libs/gui/SurfaceTexture.cpp

+1 −1
Original line number Original line Diff line number Diff line
@@ -484,7 +484,7 @@ status_t SurfaceTexture::syncForReleaseLocked(EGLDisplay dpy) { 
                return UNKNOWN_ERROR;

                return UNKNOWN_ERROR;

            }

            }

            sp<Fence> fence(new Fence(fenceFd));

            sp<Fence> fence(new Fence(fenceFd));

            status_t err = addReleaseFence(mCurrentTexture, fence);

            status_t err = addReleaseFenceLocked(mCurrentTexture, fence);

            if (err != OK) {

            if (err != OK) {

                ST_LOGE("syncForReleaseLocked: error adding release fence: "

                ST_LOGE("syncForReleaseLocked: error adding release fence: "

                        "%s (%d)", strerror(-err), err);

                        "%s (%d)", strerror(-err), err);