From da2e609eb07b5afb00e72655c5b3d24c313027e4 Mon Sep 17 00:00:00 2001
From: Yin-Chia Yeh <yinchiayeh@google.com>
Date: Wed, 29 Jul 2020 15:00:07 -0700
Subject: Surface: do not evict outstanding buffer in onBuffersDiscarded

Due to the nature of async callback, onBufferDiscarded may
arrive after a discarded slot has been occupied by a new buffer.
Fix that issue by skipping currently dequeued slots.

Test: Stress test script in the bug
Bug: 161787265
Change-Id: Ia8fdd5f28c4b535bc5a7010b6ee0b16e375ec257
---
 libs/gui/Surface.cpp           | 15 +++++++++++++++
 libs/gui/include/gui/Surface.h |  8 ++++++++
 2 files changed, 23 insertions(+)

(limited to 'libs')

diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp
index cf269b33ba..e1a17db3d9 100644
--- a/libs/gui/Surface.cpp
+++ b/libs/gui/Surface.cpp
@@ -732,6 +732,8 @@ int Surface::dequeueBuffer(android_native_buffer_t** buffer, int* fenceFd) {
         mSharedBufferHasBeenQueued = false;
     }
 
+    mDequeuedSlots.insert(buf);
+
     return OK;
 }
 
@@ -760,6 +762,8 @@ int Surface::cancelBuffer(android_native_buffer_t* buffer,
         mSharedBufferHasBeenQueued = true;
     }
 
+    mDequeuedSlots.erase(i);
+
     return OK;
 }
 
@@ -895,6 +899,8 @@ int Surface::queueBuffer(android_native_buffer_t* buffer, int fenceFd) {
         ALOGE("queueBuffer: error queuing buffer to SurfaceTexture, %d", err);
     }
 
+    mDequeuedSlots.erase(i);
+
     if (mEnableFrameTimestamps) {
         mFrameEventHistory->applyDelta(output.frameTimestamps);
         // Update timestamps with the local acquire fence.
@@ -1660,6 +1666,7 @@ int Surface::attachBuffer(ANativeWindowBuffer* buffer)
         mRemovedBuffers.push_back(mSlots[attachedSlot].buffer);
     }
     mSlots[attachedSlot].buffer = graphicBuffer;
+    mDequeuedSlots.insert(attachedSlot);
 
     return NO_ERROR;
 }
@@ -1926,6 +1933,10 @@ Dataspace Surface::getBuffersDataSpace() {
 }
 
 void Surface::freeAllBuffers() {
+    if (!mDequeuedSlots.empty()) {
+        ALOGE("%s: %zu buffers were freed while being dequeued!",
+                __FUNCTION__, mDequeuedSlots.size());
+    }
     for (int i = 0; i < NUM_BUFFER_SLOTS; i++) {
         mSlots[i].buffer = nullptr;
     }
@@ -1947,6 +1958,10 @@ status_t Surface::getAndFlushBuffersFromSlots(const std::vector<int32_t>& slots,
             ALOGW("%s: Discarded slot %d doesn't contain buffer!", __FUNCTION__, i);
             continue;
         }
+        // Don't flush currently dequeued buffers
+        if (mDequeuedSlots.count(i) > 0) {
+            continue;
+        }
         outBuffers->push_back(mSlots[i].buffer);
         mSlots[i].buffer = nullptr;
     }
diff --git a/libs/gui/include/gui/Surface.h b/libs/gui/include/gui/Surface.h
index 49c83da319..55b4101908 100644
--- a/libs/gui/include/gui/Surface.h
+++ b/libs/gui/include/gui/Surface.h
@@ -30,6 +30,7 @@
 #include <utils/RefBase.h>
 
 #include <shared_mutex>
+#include <unordered_set>
 
 namespace android {
 
@@ -543,8 +544,15 @@ protected:
     int mMaxBufferCount;
 
     sp<IProducerListener> mListenerProxy;
+
+    // Get and flush the buffers of given slots, if the buffer in the slot
+    // is currently dequeued then it won't be flushed and won't be returned
+    // in outBuffers.
     status_t getAndFlushBuffersFromSlots(const std::vector<int32_t>& slots,
             std::vector<sp<GraphicBuffer>>* outBuffers);
+
+    // Buffers that are successfully dequeued/attached and handed to clients
+    std::unordered_set<int> mDequeuedSlots;
 };
 
 } // namespace android
-- 
cgit v1.2.3-59-g8ed1b