BQ: Resend queued buffer handles after disconnect

When the producer disconnects from a BufferQueue, all of its slots are
cleared and buffers in the queue are marked as stale. When the slots
are cleared, we notify the consumer that buffers were released so that
it can clear its cache of slot-to-buffer mappings.

However, when we were marking queued buffers as stale, we were not
clearing the bit that dictates whether the BQ needs to send a buffer
handle upon acquire, so in the case where the producer queues a buffer
and disconnects, and then the consumer acquires that queued buffer,
the consumer doesn't get the buffer handle, nor can it retrieve it from
its cache, because it has (or should have) already cleared it.

This change clears that bit upon disconnect.

Bug: 36453246
Test: New test in libgui_tests + Vulkan CTS test from bug
Change-Id: Ic27f119a4164878257c3841b4e1d421694159774

diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp
index 9e3fecb..d653db8 100644 (file)
--- a/libs/gui/BufferQueueCore.cpp
+++ b/libs/gui/BufferQueueCore.cpp
@@ -261,6 +261,13 @@ void BufferQueueCore::freeAllBuffersLocked() {
 
     for (auto& b : mQueue) {
         b.mIsStale = true;
+
+        // We set this to false to force the BufferQueue to resend the buffer
+        // handle upon acquire, since if we're here due to a producer
+        // disconnect, the consumer will have been told to purge its cache of
+        // slot-to-buffer-handle mappings and will not be able to otherwise
+        // obtain a valid buffer handle.
+        b.mAcquireCalled = false;
     }
 
     VALIDATE_CONSISTENCY();

diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp
index 91ce531..907e049 100644 (file)
--- a/libs/gui/tests/BufferQueue_test.cpp
+++ b/libs/gui/tests/BufferQueue_test.cpp
@@ -1117,4 +1117,64 @@ TEST_F(BufferQueueTest, TestBufferReplacedInQueueBuffer) {
     ASSERT_EQ(true, output.bufferReplaced);
 }
 
+TEST_F(BufferQueueTest, TestStaleBufferHandleSentAfterDisconnect) {
+    createBufferQueue();
+    sp<DummyConsumer> dc(new DummyConsumer);
+    ASSERT_EQ(OK, mConsumer->consumerConnect(dc, true));
+    IGraphicBufferProducer::QueueBufferOutput output;
+    sp<IProducerListener> dummyListener(new DummyProducerListener);
+    ASSERT_EQ(OK, mProducer->connect(dummyListener, NATIVE_WINDOW_API_CPU,
+            true, &output));
+
+    int slot = BufferQueue::INVALID_BUFFER_SLOT;
+    sp<Fence> fence = Fence::NO_FENCE;
+    sp<GraphicBuffer> buffer = nullptr;
+    IGraphicBufferProducer::QueueBufferInput input(0ull, true,
+            HAL_DATASPACE_UNKNOWN, Rect::INVALID_RECT,
+            NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, Fence::NO_FENCE);
+
+    // Dequeue, request, and queue one buffer
+    status_t result = mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0,
+            nullptr);
+    ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, result);
+    ASSERT_EQ(OK, mProducer->requestBuffer(slot, &buffer));
+    ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
+
+    // Acquire and release the buffer. Upon acquiring, the buffer handle should
+    // be non-null since this is the first time we've acquired this slot.
+    BufferItem item;
+    ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+    ASSERT_EQ(slot, item.mSlot);
+    ASSERT_NE(nullptr, item.mGraphicBuffer.get());
+    ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
+            EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
+
+    // Dequeue and queue the buffer again
+    ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0, nullptr));
+    ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
+
+    // Acquire and release the buffer again. Upon acquiring, the buffer handle
+    // should be null since this is not the first time we've acquired this slot.
+    ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+    ASSERT_EQ(slot, item.mSlot);
+    ASSERT_EQ(nullptr, item.mGraphicBuffer.get());
+    ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
+            EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
+
+    // Dequeue and queue the buffer again
+    ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0, nullptr));
+    ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
+
+    // Disconnect the producer end. This should clear all of the slots and mark
+    // the buffer in the queue as stale.
+    ASSERT_EQ(OK, mProducer->disconnect(NATIVE_WINDOW_API_CPU));
+
+    // Acquire the buffer again. Upon acquiring, the buffer handle should not be
+    // null since the queued buffer should have been marked as stale, which
+    // should trigger the BufferQueue to resend the buffer handle.
+    ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+    ASSERT_EQ(slot, item.mSlot);
+    ASSERT_NE(nullptr, item.mGraphicBuffer.get());
+}
+
 } // namespace android