Fix a race in BufferQueue

BufferQueue::dequeueBuffer() could incorrectly return
WOULD_BLOCK while in "cannot block" mode if it happened
while a consumer acquired the last allowed buffer
before releasing the old one (which is a valid thing
to do).

Change-Id: I318e5408871ba85e068ea9ef4dc9b578f1bb1043
This commit is contained in:
Mathias Agopian 2013-07-23 21:50:20 -07:00
parent 7ffaa7c60d
commit 6bac363cbd

View File

@ -268,7 +268,6 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>* outFence, bool async
usage |= mConsumerUsageBits; usage |= mConsumerUsageBits;
int found = -1; int found = -1;
int dequeuedCount = 0;
bool tryAgain = true; bool tryAgain = true;
while (tryAgain) { while (tryAgain) {
if (mAbandoned) { if (mAbandoned) {
@ -299,23 +298,28 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>* outFence, bool async
// look for a free buffer to give to the client // look for a free buffer to give to the client
found = INVALID_BUFFER_SLOT; found = INVALID_BUFFER_SLOT;
dequeuedCount = 0; int dequeuedCount = 0;
int acquiredCount = 0;
for (int i = 0; i < maxBufferCount; i++) { for (int i = 0; i < maxBufferCount; i++) {
const int state = mSlots[i].mBufferState; const int state = mSlots[i].mBufferState;
if (state == BufferSlot::DEQUEUED) { switch (state) {
dequeuedCount++; case BufferSlot::DEQUEUED:
} dequeuedCount++;
break;
if (state == BufferSlot::FREE) { case BufferSlot::ACQUIRED:
/* We return the oldest of the free buffers to avoid acquiredCount++;
* stalling the producer if possible. This is because break;
* the consumer may still have pending reads of the case BufferSlot::FREE:
* buffers in flight. /* We return the oldest of the free buffers to avoid
*/ * stalling the producer if possible. This is because
if ((found < 0) || * the consumer may still have pending reads of the
mSlots[i].mFrameNumber < mSlots[found].mFrameNumber) { * buffers in flight.
found = i; */
} if ((found < 0) ||
mSlots[i].mFrameNumber < mSlots[found].mFrameNumber) {
found = i;
}
break;
} }
} }
@ -348,7 +352,13 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>* outFence, bool async
// the max buffer count to change. // the max buffer count to change.
tryAgain = found == INVALID_BUFFER_SLOT; tryAgain = found == INVALID_BUFFER_SLOT;
if (tryAgain) { if (tryAgain) {
if (mDequeueBufferCannotBlock) { // return an error if we're in "cannot block" mode (producer and consumer
// are controlled by the application) -- however, the consumer is allowed
// to acquire briefly an extra buffer (which could cause us to have to wait here)
// and that's okay because we know the wait will be brief (it happens
// if we dequeue a buffer while the consumer has acquired one but not released
// the old one yet -- for e.g.: see GLConsumer::updateTexImage()).
if (mDequeueBufferCannotBlock && (acquiredCount <= mMaxAcquiredBufferCount)) {
ST_LOGE("dequeueBuffer: would block! returning an error instead."); ST_LOGE("dequeueBuffer: would block! returning an error instead.");
return WOULD_BLOCK; return WOULD_BLOCK;
} }