BufferQueue: Guard against unbounded queue growth

Adds logic to dequeueBuffer that blocks if there are currently too
many buffers in the queue. This prevents unbounded growth around
times where the slots are cleared but the queue is not (e.g.,
during rapid connect/disconnect or setBufferCount activity). This
replaces the fix from ag/377958 in a more general way.

Bug: 11293214
Change-Id: Ieb7adfcd076ff7ffe3d4d369397b2c29cf5099c3

2 files changed, 35 insertions, 37 deletions

diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp
index 995ed5e..872eae6 100644
--- a/libs/gui/BufferQueueConsumer.cpp
+++ b/libs/gui/BufferQueueConsumer.cpp
@@ -160,8 +160,10 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer,
     }
 
     mCore->mQueue.erase(front);
-    // TODO: Should this call be after we free a slot while dropping buffers?
-    // Simply acquiring the next buffer doesn't enable a producer to dequeue.
+
+    // We might have freed a slot while dropping old buffers, or the producer
+    // may be blocked waiting for the number of buffers in the queue to
+    // decrease.
     mCore->mDequeueCondition.broadcast();
 
     ATRACE_INT(mCore->mConsumerName.string(), mCore->mQueue.size());
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index ea37309..bd9b8b2 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -205,9 +205,21 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
             }
         }
 
-        // If no buffer is found, wait for a buffer to be released or for
-        // the max buffer count to change
-        tryAgain = (*found == BufferQueueCore::INVALID_BUFFER_SLOT);
+        // If we disconnect and reconnect quickly, we can be in a state where
+        // our slots are empty but we have many buffers in the queue. This can
+        // cause us to run out of memory if we outrun the consumer. Wait here if
+        // it looks like we have too many buffers queued up.
+        bool tooManyBuffers = mCore->mQueue.size() > maxBufferCount;
+        if (tooManyBuffers) {
+            BQ_LOGV("%s: queue size is %d, waiting", caller,
+                    mCore->mQueue.size());
+        }
+
+        // If no buffer is found, or if the queue has too many buffers
+        // outstanding, wait for a buffer to be acquired or released, or for the
+        // max buffer count to change.
+        tryAgain = (*found == BufferQueueCore::INVALID_BUFFER_SLOT) ||
+                   tooManyBuffers;
         if (tryAgain) {
             // Return an error if we're in non-blocking mode (producer and
             // consumer are controlled by the application).
@@ -663,41 +675,25 @@ status_t BufferQueueProducer::connect(const sp<IProducerListener>& listener,
     BQ_LOGV("connect(P): api=%d producerControlledByApp=%s", api,
             producerControlledByApp ? "true" : "false");
 
-    // If we disconnect and reconnect quickly, we can be in a state where our
-    // slots are empty but we have many buffers in the queue. This can cause us
-    // to run out of memory if we outrun the consumer. Wait here if it looks
-    // like we have too many buffers queued up.
-    while (true) {
-        if (mCore->mIsAbandoned) {
-            BQ_LOGE("connect(P): BufferQueue has been abandoned");
-            return NO_INIT;
-        }
-
-        if (mCore->mConsumerListener == NULL) {
-            BQ_LOGE("connect(P): BufferQueue has no consumer");
-            return NO_INIT;
-        }
-
-        if (output == NULL) {
-            BQ_LOGE("connect(P): output was NULL");
-            return BAD_VALUE;
-        }
+    if (mCore->mIsAbandoned) {
+        BQ_LOGE("connect(P): BufferQueue has been abandoned");
+        return NO_INIT;
+    }
 
-        if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) {
-            BQ_LOGE("connect(P): already connected (cur=%d req=%d)",
-                    mCore->mConnectedApi, api);
-            return BAD_VALUE;
-        }
+    if (mCore->mConsumerListener == NULL) {
+        BQ_LOGE("connect(P): BufferQueue has no consumer");
+        return NO_INIT;
+    }
 
-        size_t maxBufferCount = mCore->getMaxBufferCountLocked(false);
-        if (mCore->mQueue.size() <= maxBufferCount) {
-            // The queue size seems small enough to proceed
-            // TODO: Make this bound tighter?
-            break;
-        }
+    if (output == NULL) {
+        BQ_LOGE("connect(P): output was NULL");
+        return BAD_VALUE;
+    }
 
-        BQ_LOGV("connect(P): queue size is %d, waiting", mCore->mQueue.size());
-        mCore->mDequeueCondition.wait(mCore->mMutex);
+    if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) {
+        BQ_LOGE("connect(P): already connected (cur=%d req=%d)",
+                mCore->mConnectedApi, api);
+        return BAD_VALUE;
     }
 
     int status = NO_ERROR;