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
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 @@
}
}
- // 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 @@
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->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 (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 (output == NULL) {
+ BQ_LOGE("connect(P): output was NULL");
+ return BAD_VALUE;
+ }
- if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) {
- BQ_LOGE("connect(P): already connected (cur=%d req=%d)",
- mCore->mConnectedApi, api);
- return BAD_VALUE;
- }
-
- 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;
- }
-
- 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;