From ae3c3682333f25e860fe54e2bae3599bb466cdb6 Mon Sep 17 00:00:00 2001 From: Dan Stoza Date: Fri, 18 Apr 2014 15:43:35 -0700 Subject: 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 --- libs/gui/BufferQueueProducer.cpp | 66 +++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 35 deletions(-) (limited to 'libs/gui/BufferQueueProducer.cpp') diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index ea37309fdb..bd9b8b2733 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& 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; -- cgit v1.2.3-59-g8ed1b