commit 583b1b32191992d6ada58b3c61c71932a71c0c4b
author Pablo Ceballos <pceballos@google.com> Thu Sep 03 18:23:52 2015 -0700
committer Pablo Ceballos <pceballos@google.com> Fri Sep 18 11:43:09 2015 -0700
tree 2c8daa3613d25df62529580ec167879b6b0cc5c5
parent 643a34c3b4dcacf3303ec33b590ca9067e475169

BQ: Prevent operations on disconnected BQs

- Update unit tests to match

Bug 23763412

Change-Id: I77e59bf6b57b328433c3835450455f80a8fa454b

libs/gui/BufferQueueProducer.cpp

diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 8cc0cfa..06cdeab 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -57,6 +57,11 @@
         return NO_INIT;
     }
 
+    if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+        BQ_LOGE("requestBuffer: BufferQueue has no connected producer");
+        return NO_INIT;
+    }
+
     if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) {
         BQ_LOGE("requestBuffer: slot index %d out of range [0, %d)",
                 slot, BufferQueueDefs::NUM_BUFFER_SLOTS);
@@ -286,6 +291,16 @@
     { // Autolock scope
         Mutex::Autolock lock(mCore->mMutex);
         mConsumerName = mCore->mConsumerName;
+
+        if (mCore->mIsAbandoned) {
+            BQ_LOGE("dequeueBuffer: BufferQueue has been abandoned");
+            return NO_INIT;
+        }
+
+        if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+            BQ_LOGE("dequeueBuffer: BufferQueue has no connected producer");
+            return NO_INIT;
+        }
     } // Autolock scope
 
     BQ_LOGV("dequeueBuffer: async=%s w=%u h=%u format=%#x, usage=%#x",
@@ -453,6 +468,11 @@
         return NO_INIT;
     }
 
+    if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+        BQ_LOGE("detachBuffer(P): BufferQueue has no connected producer");
+        return NO_INIT;
+    }
+
     if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) {
         BQ_LOGE("detachBuffer(P): slot index %d out of range [0, %d)",
                 slot, BufferQueueDefs::NUM_BUFFER_SLOTS);
@@ -487,13 +507,19 @@
     }
 
     Mutex::Autolock lock(mCore->mMutex);
-    mCore->waitWhileAllocatingLocked();
 
     if (mCore->mIsAbandoned) {
         BQ_LOGE("detachNextBuffer: BufferQueue has been abandoned");
         return NO_INIT;
     }
 
+    if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+        BQ_LOGE("detachNextBuffer: BufferQueue has no connected producer");
+        return NO_INIT;
+    }
+
+    mCore->waitWhileAllocatingLocked();
+
     if (mCore->mFreeBuffers.empty()) {
         return NO_MEMORY;
     }
@@ -524,7 +550,16 @@
     }
 
     Mutex::Autolock lock(mCore->mMutex);
-    mCore->waitWhileAllocatingLocked();
+
+    if (mCore->mIsAbandoned) {
+        BQ_LOGE("attachBuffer(P): BufferQueue has been abandoned");
+        return NO_INIT;
+    }
+
+    if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+        BQ_LOGE("attachBuffer(P): BufferQueue has no connected producer");
+        return NO_INIT;
+    }
 
     if (buffer->getGenerationNumber() != mCore->mGenerationNumber) {
         BQ_LOGE("attachBuffer: generation number mismatch [buffer %u] "
@@ -533,6 +568,8 @@
         return BAD_VALUE;
     }
 
+    mCore->waitWhileAllocatingLocked();
+
     status_t returnFlags = NO_ERROR;
     int found;
     // TODO: Should we provide an async flag to attachBuffer? It seems
@@ -612,6 +649,11 @@
             return NO_INIT;
         }
 
+        if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+            BQ_LOGE("queueBuffer: BufferQueue has no connected producer");
+            return NO_INIT;
+        }
+
         const int maxBufferCount = mCore->getMaxBufferCountLocked(async);
 
         if (slot < 0 || slot >= maxBufferCount) {
@@ -748,27 +790,32 @@
     return NO_ERROR;
 }
 
-void BufferQueueProducer::cancelBuffer(int slot, const sp<Fence>& fence) {
+status_t BufferQueueProducer::cancelBuffer(int slot, const sp<Fence>& fence) {
     ATRACE_CALL();
     BQ_LOGV("cancelBuffer: slot %d", slot);
     Mutex::Autolock lock(mCore->mMutex);
 
     if (mCore->mIsAbandoned) {
         BQ_LOGE("cancelBuffer: BufferQueue has been abandoned");
-        return;
+        return NO_INIT;
+    }
+
+    if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+        BQ_LOGE("cancelBuffer: BufferQueue has no connected producer");
+        return NO_INIT;
     }
 
     if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) {
         BQ_LOGE("cancelBuffer: slot index %d out of range [0, %d)",
                 slot, BufferQueueDefs::NUM_BUFFER_SLOTS);
-        return;
+        return BAD_VALUE;
     } else if (mSlots[slot].mBufferState != BufferSlot::DEQUEUED) {
         BQ_LOGE("cancelBuffer: slot %d is not owned by the producer "
                 "(state = %d)", slot, mSlots[slot].mBufferState);
-        return;
+        return BAD_VALUE;
     } else if (fence == NULL) {
         BQ_LOGE("cancelBuffer: fence is NULL");
-        return;
+        return BAD_VALUE;
     }
 
     mCore->mFreeBuffers.push_front(slot);
@@ -776,6 +823,8 @@
     mSlots[slot].mFence = fence;
     mCore->mDequeueCondition.broadcast();
     mCore->validateConsistencyLocked();
+
+    return NO_ERROR;
 }
 
 int BufferQueueProducer::query(int what, int *outValue) {