don't return the current buffer from dequeueBuffer

we were not reseting mCurrentTexture in some situations
which in turn caused dequeueBuffers() return a
"FREE" buffer that was also current.
Very often it was harmless, but it created a race with
updateTexImage() which could cause the following
queueBuffers() to fail.

Bug: 5156325
Change-Id: If15a31dc869117543d220d6e5562c57116cbabdb

2 files changed, 24 insertions, 5 deletions

diff --git a/include/gui/SurfaceTexture.h b/include/gui/SurfaceTexture.h
index a6fb12e..493993d 100644
--- a/include/gui/SurfaceTexture.h
+++ b/include/gui/SurfaceTexture.h
@@ -275,7 +275,7 @@ private:
         enum BufferState {
             // FREE indicates that the buffer is not currently being used and
             // will not be used in the future until it gets dequeued and
-            // subseqently queued by the client.
+            // subsequently queued by the client.
             FREE = 0,
 
             // DEQUEUED indicates that the buffer has been dequeued by the
diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp
index b2441ce..ac9b33b 100644
--- a/libs/gui/SurfaceTexture.cpp
+++ b/libs/gui/SurfaceTexture.cpp
@@ -36,6 +36,10 @@
 #include <utils/Log.h>
 #include <utils/String8.h>
 
+
+#define ALLOW_DEQUEUE_CURRENT_BUFFER    false
+
+
 namespace android {
 
 // Transform matrices
@@ -294,9 +298,22 @@ status_t SurfaceTexture::dequeueBuffer(int *outBuf, uint32_t w, uint32_t h,
             if (state == BufferSlot::DEQUEUED) {
                 dequeuedCount++;
             }
-            if (state == BufferSlot::FREE /*|| i == mCurrentTexture*/) {
-                foundSync = i;
-                if (i != mCurrentTexture) {
+
+            // if buffer is FREE it CANNOT be current
+            LOGW_IF((state == BufferSlot::FREE) && (mCurrentTexture==i),
+                    "dequeueBuffer: buffer %d is both FREE and current!", i);
+
+            if (ALLOW_DEQUEUE_CURRENT_BUFFER) {
+                if (state == BufferSlot::FREE || i == mCurrentTexture) {
+                    foundSync = i;
+                    if (i != mCurrentTexture) {
+                        found = i;
+                        break;
+                    }
+                }
+            } else {
+                if (state == BufferSlot::FREE) {
+                    foundSync = i;
                     found = i;
                     break;
                 }
@@ -325,7 +342,7 @@ status_t SurfaceTexture::dequeueBuffer(int *outBuf, uint32_t w, uint32_t h,
         }
 
         // we're in synchronous mode and didn't find a buffer, we need to wait
-        // for for some buffers to be consumed
+        // for some buffers to be consumed
         tryAgain = mSynchronousMode && (foundSync == INVALID_BUFFER_SLOT);
         if (tryAgain) {
             mDequeueCondition.wait(mMutex);
@@ -846,6 +863,7 @@ void SurfaceTexture::freeBufferLocked(int i) {
 void SurfaceTexture::freeAllBuffersLocked() {
     LOGW_IF(!mQueue.isEmpty(),
             "freeAllBuffersLocked called but mQueue is not empty");
+    mCurrentTexture = INVALID_BUFFER_SLOT;
     for (int i = 0; i < NUM_BUFFER_SLOTS; i++) {
         freeBufferLocked(i);
     }
@@ -859,6 +877,7 @@ void SurfaceTexture::freeAllBuffersExceptHeadLocked() {
         Fifo::iterator front(mQueue.begin());
         head = *front;
     }
+    mCurrentTexture = INVALID_BUFFER_SLOT;
     for (int i = 0; i < NUM_BUFFER_SLOTS; i++) {
         if (i != head) {
             freeBufferLocked(i);