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

Commit 1df8c345 authored by Jamie Gennis's avatar Jamie Gennis
Browse files

libgui: disallow NULL Fence pointers 

This change eliminates the uses of a NULL sp<Fence> indicating that no waiting
is required.  Instead we use a non-NULL but invalid Fence object for which the
wait methods will return immediately.

Bug: 7892871
Change-Id: I5360aebe3090422ef6920d56c99fc4eedc642e48
parent 351c2941

libs/gui/BufferItemConsumer.cpp

+1 −1
Original line number Original line Diff line number Diff line
@@ -62,7 +62,7 @@ status_t BufferItemConsumer::acquireBuffer(BufferItem *item, bool waitForFence) 
        return err;

        return err;

    }

    }





    if (waitForFence && item->mFence.get()) {

    if (waitForFence) {

        err = item->mFence->waitForever(1000, "BufferItemConsumer::acquireBuffer");

        err = item->mFence->waitForever(1000, "BufferItemConsumer::acquireBuffer");

        if (err != OK) {

        if (err != OK) {

            BI_LOGE("Failed to wait for fence of acquired buffer: %s (%d)",

            BI_LOGE("Failed to wait for fence of acquired buffer: %s (%d)",

libs/gui/BufferQueue.cpp

+14 −9
Original line number Original line Diff line number Diff line
@@ -372,8 +372,6 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>& outFence, 
            h = mDefaultHeight;

            h = mDefaultHeight;

        }

        }





        // buffer is now in DEQUEUED (but can also be current at the same time,

        // if we're in synchronous mode)

        mSlots[buf].mBufferState = BufferSlot::DEQUEUED;

        mSlots[buf].mBufferState = BufferSlot::DEQUEUED;





        const sp<GraphicBuffer>& buffer(mSlots[buf].mGraphicBuffer);

        const sp<GraphicBuffer>& buffer(mSlots[buf].mGraphicBuffer);
@@ -387,7 +385,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>& outFence, 
            mSlots[buf].mGraphicBuffer = NULL;

            mSlots[buf].mGraphicBuffer = NULL;

            mSlots[buf].mRequestBufferCalled = false;

            mSlots[buf].mRequestBufferCalled = false;

            mSlots[buf].mEglFence = EGL_NO_SYNC_KHR;

            mSlots[buf].mEglFence = EGL_NO_SYNC_KHR;

            mSlots[buf].mFence.clear();

            mSlots[buf].mFence = Fence::NO_FENCE;

            mSlots[buf].mEglDisplay = EGL_NO_DISPLAY;

            mSlots[buf].mEglDisplay = EGL_NO_DISPLAY;





            returnFlags |= IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION;

            returnFlags |= IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION;
@@ -397,7 +395,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>& outFence, 
        eglFence = mSlots[buf].mEglFence;

        eglFence = mSlots[buf].mEglFence;

        outFence = mSlots[buf].mFence;

        outFence = mSlots[buf].mFence;

        mSlots[buf].mEglFence = EGL_NO_SYNC_KHR;

        mSlots[buf].mEglFence = EGL_NO_SYNC_KHR;

        mSlots[buf].mFence.clear();

        mSlots[buf].mFence = Fence::NO_FENCE;

    }  // end lock scope

    }  // end lock scope





    if (returnFlags & IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION) {

    if (returnFlags & IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION) {
@@ -423,7 +421,6 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>& outFence, 
        }

        }

    }

    }







    if (eglFence != EGL_NO_SYNC_KHR) {

    if (eglFence != EGL_NO_SYNC_KHR) {

        EGLint result = eglClientWaitSyncKHR(dpy, eglFence, 0, 1000000000);

        EGLint result = eglClientWaitSyncKHR(dpy, eglFence, 0, 1000000000);

        // If something goes wrong, log the error, but return the buffer without

        // If something goes wrong, log the error, but return the buffer without
@@ -488,6 +485,11 @@ status_t BufferQueue::queueBuffer(int buf, 




    input.deflate(&timestamp, &crop, &scalingMode, &transform, &fence);

    input.deflate(&timestamp, &crop, &scalingMode, &transform, &fence);





    if (fence == NULL) {

        ST_LOGE("queueBuffer: fence is NULL");

        return BAD_VALUE;

    }



    ST_LOGV("queueBuffer: slot=%d time=%#llx crop=[%d,%d,%d,%d] tr=%#x "

    ST_LOGV("queueBuffer: slot=%d time=%#llx crop=[%d,%d,%d,%d] tr=%#x "

            "scale=%s",

            "scale=%s",

            buf, timestamp, crop.left, crop.top, crop.right, crop.bottom,

            buf, timestamp, crop.left, crop.top, crop.right, crop.bottom,
@@ -607,6 +609,9 @@ void BufferQueue::cancelBuffer(int buf, sp<Fence> fence) { 
        ST_LOGE("cancelBuffer: slot %d is not owned by the client (state=%d)",

        ST_LOGE("cancelBuffer: slot %d is not owned by the client (state=%d)",

                buf, mSlots[buf].mBufferState);

                buf, mSlots[buf].mBufferState);

        return;

        return;

    } else if (fence == NULL) {

        ST_LOGE("cancelBuffer: fence is NULL");

        return;

    }

    }

    mSlots[buf].mBufferState = BufferSlot::FREE;

    mSlots[buf].mBufferState = BufferSlot::FREE;

    mSlots[buf].mFrameNumber = 0;

    mSlots[buf].mFrameNumber = 0;
@@ -785,7 +790,7 @@ void BufferQueue::freeBufferLocked(int slot) { 
        eglDestroySyncKHR(mSlots[slot].mEglDisplay, mSlots[slot].mEglFence);

        eglDestroySyncKHR(mSlots[slot].mEglDisplay, mSlots[slot].mEglFence);

        mSlots[slot].mEglFence = EGL_NO_SYNC_KHR;

        mSlots[slot].mEglFence = EGL_NO_SYNC_KHR;

    }

    }

    mSlots[slot].mFence.clear();

    mSlots[slot].mFence = Fence::NO_FENCE;

}

}





void BufferQueue::freeAllBuffersLocked() {

void BufferQueue::freeAllBuffersLocked() {
@@ -843,7 +848,7 @@ status_t BufferQueue::acquireBuffer(BufferItem *buffer) { 
        mSlots[buf].mAcquireCalled = true;

        mSlots[buf].mAcquireCalled = true;

        mSlots[buf].mNeedsCleanupOnRelease = false;

        mSlots[buf].mNeedsCleanupOnRelease = false;

        mSlots[buf].mBufferState = BufferSlot::ACQUIRED;

        mSlots[buf].mBufferState = BufferSlot::ACQUIRED;

        mSlots[buf].mFence.clear();

        mSlots[buf].mFence = Fence::NO_FENCE;





        mQueue.erase(front);

        mQueue.erase(front);

        mDequeueCondition.broadcast();

        mDequeueCondition.broadcast();
@@ -863,8 +868,8 @@ status_t BufferQueue::releaseBuffer(int buf, EGLDisplay display, 




    Mutex::Autolock _l(mMutex);

    Mutex::Autolock _l(mMutex);





    if (buf == INVALID_BUFFER_SLOT) {

    if (buf == INVALID_BUFFER_SLOT || fence == NULL) {

        return -EINVAL;

        return BAD_VALUE;

    }

    }





    mSlots[buf].mEglDisplay = display;

    mSlots[buf].mEglDisplay = display;

libs/gui/ConsumerBase.cpp

+2 −2
Original line number Original line Diff line number Diff line
@@ -84,7 +84,7 @@ ConsumerBase::~ConsumerBase() { 
void ConsumerBase::freeBufferLocked(int slotIndex) {

void ConsumerBase::freeBufferLocked(int slotIndex) {

    CB_LOGV("freeBufferLocked: slotIndex=%d", slotIndex);

    CB_LOGV("freeBufferLocked: slotIndex=%d", slotIndex);

    mSlots[slotIndex].mGraphicBuffer = 0;

    mSlots[slotIndex].mGraphicBuffer = 0;

    mSlots[slotIndex].mFence = 0;

    mSlots[slotIndex].mFence = Fence::NO_FENCE;

}

}





// Used for refactoring, should not be in final interface

// Used for refactoring, should not be in final interface
@@ -228,7 +228,7 @@ status_t ConsumerBase::releaseBufferLocked(int slot, EGLDisplay display, 
        freeBufferLocked(slot);

        freeBufferLocked(slot);

    }

    }





    mSlots[slot].mFence.clear();

    mSlots[slot].mFence = Fence::NO_FENCE;





    return err;

    return err;

}

}

libs/gui/GLConsumer.cpp

+2 −1
Original line number Original line Diff line number Diff line
@@ -117,6 +117,7 @@ GLConsumer::GLConsumer(GLuint tex, bool allowSynchronousMode, 
        GLenum texTarget, bool useFenceSync, const sp<BufferQueue> &bufferQueue) :

        GLenum texTarget, bool useFenceSync, const sp<BufferQueue> &bufferQueue) :

    ConsumerBase(bufferQueue == 0 ? new BufferQueue(allowSynchronousMode) : bufferQueue),

    ConsumerBase(bufferQueue == 0 ? new BufferQueue(allowSynchronousMode) : bufferQueue),

    mCurrentTransform(0),

    mCurrentTransform(0),

    mCurrentFence(Fence::NO_FENCE),

    mCurrentTimestamp(0),

    mCurrentTimestamp(0),

    mFilteringEnabled(true),

    mFilteringEnabled(true),

    mTexName(tex),

    mTexName(tex),
@@ -823,7 +824,7 @@ status_t GLConsumer::doGLFenceWaitLocked() const { 
        return INVALID_OPERATION;

        return INVALID_OPERATION;

    }

    }





    if (mCurrentFence != NULL) {

    if (mCurrentFence->isValid()) {

        if (useWaitSync) {

        if (useWaitSync) {

            // Create an EGLSyncKHR from the current fence.

            // Create an EGLSyncKHR from the current fence.

            int fenceFd = mCurrentFence->dup();

            int fenceFd = mCurrentFence->dup();

libs/gui/IGraphicBufferProducer.cpp

+15 −35
Original line number Original line Diff line number Diff line
@@ -94,9 +94,11 @@ public: 
            return result;

            return result;

        }

        }

        *buf = reply.readInt32();

        *buf = reply.readInt32();

        fence.clear();

        bool fenceWasWritten = reply.readInt32();

        bool hasFence = reply.readInt32();

        if (fenceWasWritten) {

        if (hasFence) {

            // If the fence was written by the callee, then overwrite the

            // caller's fence here.  If it wasn't written then don't touch the

            // caller's fence.

            fence = new Fence();

            fence = new Fence();

            reply.read(*fence.get());

            reply.read(*fence.get());

        }

        }
@@ -121,13 +123,9 @@ public: 




    virtual void cancelBuffer(int buf, sp<Fence> fence) {

    virtual void cancelBuffer(int buf, sp<Fence> fence) {

        Parcel data, reply;

        Parcel data, reply;

        bool hasFence = fence.get() && fence->isValid();

        data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor());

        data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor());

        data.writeInt32(buf);

        data.writeInt32(buf);

        data.writeInt32(hasFence);

        if (hasFence) {

        data.write(*fence.get());

        data.write(*fence.get());

        }

        remote()->transact(CANCEL_BUFFER, data, &reply);

        remote()->transact(CANCEL_BUFFER, data, &reply);

    }

    }
@@ -218,10 +216,9 @@ status_t BnGraphicBufferProducer::onTransact( 
            int buf;

            int buf;

            sp<Fence> fence;

            sp<Fence> fence;

            int result = dequeueBuffer(&buf, fence, w, h, format, usage);

            int result = dequeueBuffer(&buf, fence, w, h, format, usage);

            bool hasFence = fence.get() && fence->isValid();

            reply->writeInt32(buf);

            reply->writeInt32(buf);

            reply->writeInt32(hasFence);

            reply->writeInt32(fence != NULL);

            if (hasFence) {

            if (fence != NULL) {

                reply->write(*fence.get());

                reply->write(*fence.get());

            }

            }

            reply->writeInt32(result);

            reply->writeInt32(result);
@@ -241,12 +238,8 @@ status_t BnGraphicBufferProducer::onTransact( 
        case CANCEL_BUFFER: {

        case CANCEL_BUFFER: {

            CHECK_INTERFACE(IGraphicBufferProducer, data, reply);

            CHECK_INTERFACE(IGraphicBufferProducer, data, reply);

            int buf = data.readInt32();

            int buf = data.readInt32();

            sp<Fence> fence;

            sp<Fence> fence = new Fence();

            bool hasFence = data.readInt32();

            if (hasFence) {

                fence = new Fence();

            data.read(*fence.get());

            data.read(*fence.get());

            }

            cancelBuffer(buf, fence);

            cancelBuffer(buf, fence);

            return NO_ERROR;

            return NO_ERROR;

        } break;

        } break;
@@ -289,10 +282,6 @@ status_t BnGraphicBufferProducer::onTransact( 




// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------





static bool isValid(const sp<Fence>& fence) {

    return fence.get() && fence->isValid();

}



IGraphicBufferProducer::QueueBufferInput::QueueBufferInput(const Parcel& parcel) {

IGraphicBufferProducer::QueueBufferInput::QueueBufferInput(const Parcel& parcel) {

    parcel.read(*this);

    parcel.read(*this);

}

}
@@ -303,29 +292,24 @@ size_t IGraphicBufferProducer::QueueBufferInput::getFlattenedSize() const 
         + sizeof(crop)

         + sizeof(crop)

         + sizeof(scalingMode)

         + sizeof(scalingMode)

         + sizeof(transform)

         + sizeof(transform)

         + sizeof(bool)

         + fence->getFlattenedSize();

         + (isValid(fence) ? fence->getFlattenedSize() : 0);

}

}





size_t IGraphicBufferProducer::QueueBufferInput::getFdCount() const

size_t IGraphicBufferProducer::QueueBufferInput::getFdCount() const

{

{

    return isValid(fence) ? fence->getFdCount() : 0;

    return fence->getFdCount();

}

}





status_t IGraphicBufferProducer::QueueBufferInput::flatten(void* buffer, size_t size,

status_t IGraphicBufferProducer::QueueBufferInput::flatten(void* buffer, size_t size,

        int fds[], size_t count) const

        int fds[], size_t count) const

{

{

    status_t err = NO_ERROR;

    status_t err = NO_ERROR;

    bool haveFence = isValid(fence);

    char* p = (char*)buffer;

    char* p = (char*)buffer;

    memcpy(p, &timestamp,   sizeof(timestamp));   p += sizeof(timestamp);

    memcpy(p, &timestamp,   sizeof(timestamp));   p += sizeof(timestamp);

    memcpy(p, &crop,        sizeof(crop));        p += sizeof(crop);

    memcpy(p, &crop,        sizeof(crop));        p += sizeof(crop);

    memcpy(p, &scalingMode, sizeof(scalingMode)); p += sizeof(scalingMode);

    memcpy(p, &scalingMode, sizeof(scalingMode)); p += sizeof(scalingMode);

    memcpy(p, &transform,   sizeof(transform));   p += sizeof(transform);

    memcpy(p, &transform,   sizeof(transform));   p += sizeof(transform);

    memcpy(p, &haveFence,   sizeof(haveFence));   p += sizeof(haveFence);

    if (haveFence) {

    err = fence->flatten(p, size - (p - (char*)buffer), fds, count);

    err = fence->flatten(p, size - (p - (char*)buffer), fds, count);

    }

    return err;

    return err;

}

}
@@ -333,17 +317,13 @@ status_t IGraphicBufferProducer::QueueBufferInput::unflatten(void const* buffer, 
        size_t size, int fds[], size_t count)

        size_t size, int fds[], size_t count)

{

{

    status_t err = NO_ERROR;

    status_t err = NO_ERROR;

    bool haveFence;

    const char* p = (const char*)buffer;

    const char* p = (const char*)buffer;

    memcpy(&timestamp,   p, sizeof(timestamp));   p += sizeof(timestamp);

    memcpy(&timestamp,   p, sizeof(timestamp));   p += sizeof(timestamp);

    memcpy(&crop,        p, sizeof(crop));        p += sizeof(crop);

    memcpy(&crop,        p, sizeof(crop));        p += sizeof(crop);

    memcpy(&scalingMode, p, sizeof(scalingMode)); p += sizeof(scalingMode);

    memcpy(&scalingMode, p, sizeof(scalingMode)); p += sizeof(scalingMode);

    memcpy(&transform,   p, sizeof(transform));   p += sizeof(transform);

    memcpy(&transform,   p, sizeof(transform));   p += sizeof(transform);

    memcpy(&haveFence,   p, sizeof(haveFence));   p += sizeof(haveFence);

    if (haveFence) {

    fence = new Fence();

    fence = new Fence();

    err = fence->unflatten(p, size - (p - (const char*)buffer), fds, count);

    err = fence->unflatten(p, size - (p - (const char*)buffer), fds, count);

    }

    return err;

    return err;

}

}