commit 75ce1ea078444100db9f9eef06a9ef35ad8a0446
author Patrick Williams <pdwilliams@google.com> Thu Jan 19 15:02:30 2023 -0600
committer Patrick Williams <pdwilliams@google.com> Mon Jan 23 09:39:12 2023 -0600
tree e70f9648c7bf8cb2bc223bda1b026c94df45f8f9
parent 24b785d3bbd2e811a1d466f87a68fae539bd2a2d

Cache and uncache buffers in the same transaction

This removes a race condition where clients may attempt to cache a
buffer before an associated uncache buffer request has completed,
leading to full buffer cache errors in SurfaceFlinger.

GLSurfaceViewTest#testPauseResumeWithoutDelay is failing on barbet and
test logs show frequent `ClientCache::add - cache is full` messages.
This CL fixes the "cache is full" error but does not fix the broken
test.

Bug: 264892858
Test: presubmits
Test: GLSurfaceViewTest#testPauseResumeWithoutDelay on barbet
Change-Id: I9a075054d11c819307cd837fcfbdc03d8faf5086

libs/gui/ISurfaceComposer.cpp

diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index a77ca04..cefb9a7 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -63,7 +63,8 @@
                                  Vector<ComposerState>& state, const Vector<DisplayState>& displays,
                                  uint32_t flags, const sp<IBinder>& applyToken,
                                  const InputWindowCommands& commands, int64_t desiredPresentTime,
-                                 bool isAutoTimestamp, const client_cache_t& uncacheBuffer,
+                                 bool isAutoTimestamp,
+                                 const std::vector<client_cache_t>& uncacheBuffers,
                                  bool hasListenerCallbacks,
                                  const std::vector<ListenerCallbacks>& listenerCallbacks,
                                  uint64_t transactionId) override {
@@ -87,8 +88,11 @@
         SAFE_PARCEL(commands.write, data);
         SAFE_PARCEL(data.writeInt64, desiredPresentTime);
         SAFE_PARCEL(data.writeBool, isAutoTimestamp);
-        SAFE_PARCEL(data.writeStrongBinder, uncacheBuffer.token.promote());
-        SAFE_PARCEL(data.writeUint64, uncacheBuffer.id);
+        SAFE_PARCEL(data.writeUint32, static_cast<uint32_t>(uncacheBuffers.size()));
+        for (const client_cache_t& uncacheBuffer : uncacheBuffers) {
+            SAFE_PARCEL(data.writeStrongBinder, uncacheBuffer.token.promote());
+            SAFE_PARCEL(data.writeUint64, uncacheBuffer.id);
+        }
         SAFE_PARCEL(data.writeBool, hasListenerCallbacks);
 
         SAFE_PARCEL(data.writeVectorSize, listenerCallbacks);
@@ -158,11 +162,14 @@
             SAFE_PARCEL(data.readInt64, &desiredPresentTime);
             SAFE_PARCEL(data.readBool, &isAutoTimestamp);
 
-            client_cache_t uncachedBuffer;
+            SAFE_PARCEL_READ_SIZE(data.readUint32, &count, data.dataSize());
+            std::vector<client_cache_t> uncacheBuffers(count);
             sp<IBinder> tmpBinder;
-            SAFE_PARCEL(data.readNullableStrongBinder, &tmpBinder);
-            uncachedBuffer.token = tmpBinder;
-            SAFE_PARCEL(data.readUint64, &uncachedBuffer.id);
+            for (size_t i = 0; i < count; i++) {
+                SAFE_PARCEL(data.readNullableStrongBinder, &tmpBinder);
+                uncacheBuffers[i].token = tmpBinder;
+                SAFE_PARCEL(data.readUint64, &uncacheBuffers[i].id);
+            }
 
             bool hasListenerCallbacks = false;
             SAFE_PARCEL(data.readBool, &hasListenerCallbacks);
@@ -182,7 +189,7 @@
 
             return setTransactionState(frameTimelineInfo, state, displays, stateFlags, applyToken,
                                        inputWindowCommands, desiredPresentTime, isAutoTimestamp,
-                                       uncachedBuffer, hasListenerCallbacks, listenerCallbacks,
+                                       uncacheBuffers, hasListenerCallbacks, listenerCallbacks,
                                        transactionId);
         }
         default: {