commit da2e609eb07b5afb00e72655c5b3d24c313027e4
author Yin-Chia Yeh <yinchiayeh@google.com> Wed Jul 29 15:00:07 2020 -0700
committer Yin-Chia Yeh <yinchiayeh@google.com> Thu Jul 30 11:02:40 2020 -0700
tree 810253fd31564621ad8fbfc647ebeb78f5284426
parent b96ea771e0c1916eeaee6b979a5f9d7848c0ed59

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

diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp
index cf269b3..e1a17db 100644
--- a/libs/gui/Surface.cpp
+++ b/libs/gui/Surface.cpp
@@ -732,6 +732,8 @@
         mSharedBufferHasBeenQueued = false;
     }
 
+    mDequeuedSlots.insert(buf);
+
     return OK;
 }
 
@@ -760,6 +762,8 @@
         mSharedBufferHasBeenQueued = true;
     }
 
+    mDequeuedSlots.erase(i);
+
     return OK;
 }
 
@@ -895,6 +899,8 @@
         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 @@
         mRemovedBuffers.push_back(mSlots[attachedSlot].buffer);
     }
     mSlots[attachedSlot].buffer = graphicBuffer;
+    mDequeuedSlots.insert(attachedSlot);
 
     return NO_ERROR;
 }
@@ -1926,6 +1933,10 @@
 }
 
 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 @@
             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;
     }