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
diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp
index 73bd488..320f4cf 100644
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -268,7 +268,6 @@
usage |= mConsumerUsageBits;
int found = -1;
- int dequeuedCount = 0;
bool tryAgain = true;
while (tryAgain) {
if (mAbandoned) {
@@ -299,23 +298,28 @@
// look for a free buffer to give to the client
found = INVALID_BUFFER_SLOT;
- dequeuedCount = 0;
+ int dequeuedCount = 0;
+ int acquiredCount = 0;
for (int i = 0; i < maxBufferCount; i++) {
const int state = mSlots[i].mBufferState;
- if (state == BufferSlot::DEQUEUED) {
- dequeuedCount++;
- }
-
- if (state == BufferSlot::FREE) {
- /* We return the oldest of the free buffers to avoid
- * stalling the producer if possible. This is because
- * the consumer may still have pending reads of the
- * buffers in flight.
- */
- if ((found < 0) ||
- mSlots[i].mFrameNumber < mSlots[found].mFrameNumber) {
- found = i;
- }
+ switch (state) {
+ case BufferSlot::DEQUEUED:
+ dequeuedCount++;
+ break;
+ case BufferSlot::ACQUIRED:
+ acquiredCount++;
+ break;
+ case BufferSlot::FREE:
+ /* We return the oldest of the free buffers to avoid
+ * stalling the producer if possible. This is because
+ * the consumer may still have pending reads of the
+ * buffers in flight.
+ */
+ if ((found < 0) ||
+ mSlots[i].mFrameNumber < mSlots[found].mFrameNumber) {
+ found = i;
+ }
+ break;
}
}
@@ -348,7 +352,13 @@
// the max buffer count to change.
tryAgain = found == INVALID_BUFFER_SLOT;
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.");
return WOULD_BLOCK;
}