From 78b7220f542826771223f9b5c0b876874dd398d2 Mon Sep 17 00:00:00 2001
From: Marissa Wall <marissaw@google.com>
Date: Fri, 15 Mar 2019 14:58:34 -0700
Subject: blast: drop buffer from SF's cache when destroyed

When an app drops its reference to an AHardwareBuffer, the buffer
should be removed from the client and SurfaceFlinger caches.

Ideally, both caches would have wp to the buffer and the buffer
would automatically be removed from the cache.

Unfortunately, GraphicBuffers are refcounted per process. If SurfaceFlinger
just has a wp to the GraphicBuffer, the buffer's destructor will
be called and SurfaceFlinger will lose access to the buffer.
SurfaceFlinger can't just hold onto a sp to the buffer because
then the buffer wouldn't be destoryed when the app drops its reference.

Instead, when the app process drops its last strong reference,
GraphicBuffer will send a callback to the client side cache.
The cache will send a Transaction to SurfaceFlinger to drop its sp
to the buffer.

Bug: 127689853
Test: SurfaceFlinger_test

Change-Id: I2182578ed33d7c731945cb88cd1decb2892266b0
---
 services/surfaceflinger/BufferStateLayerCache.cpp | 39 +++++++++++++++--------
 1 file changed, 25 insertions(+), 14 deletions(-)

(limited to 'services/surfaceflinger/BufferStateLayerCache.cpp')

diff --git a/services/surfaceflinger/BufferStateLayerCache.cpp b/services/surfaceflinger/BufferStateLayerCache.cpp
index cb02d16007..51ca45c53a 100644
--- a/services/surfaceflinger/BufferStateLayerCache.cpp
+++ b/services/surfaceflinger/BufferStateLayerCache.cpp
@@ -23,21 +23,14 @@
 
 #include "BufferStateLayerCache.h"
 
-#define VALID_CACHE_ID(id) ((id) >= 0 || (id) < (BUFFER_CACHE_MAX_SIZE))
-
 namespace android {
 
 ANDROID_SINGLETON_STATIC_INSTANCE(BufferStateLayerCache);
 
 BufferStateLayerCache::BufferStateLayerCache() : mDeathRecipient(new CacheDeathRecipient) {}
 
-void BufferStateLayerCache::add(sp<IBinder> processToken, int32_t id,
+void BufferStateLayerCache::add(const sp<IBinder>& processToken, uint64_t id,
                                 const sp<GraphicBuffer>& buffer) {
-    if (!VALID_CACHE_ID(id)) {
-        ALOGE("failed to cache buffer: invalid buffer id");
-        return;
-    }
-
     if (!processToken) {
         ALOGE("failed to cache buffer: invalid process token");
         return;
@@ -61,15 +54,33 @@ void BufferStateLayerCache::add(sp<IBinder> processToken, int32_t id,
     }
 
     auto& processBuffers = mBuffers[processToken];
+
+    if (processBuffers.size() > BUFFER_CACHE_MAX_SIZE) {
+        ALOGE("failed to cache buffer: cache is full");
+        return;
+    }
+
     processBuffers[id] = buffer;
 }
 
-sp<GraphicBuffer> BufferStateLayerCache::get(sp<IBinder> processToken, int32_t id) {
-    if (!VALID_CACHE_ID(id)) {
-        ALOGE("failed to get buffer: invalid buffer id");
-        return nullptr;
+void BufferStateLayerCache::erase(const sp<IBinder>& processToken, uint64_t id) {
+    if (!processToken) {
+        ALOGE("failed to uncache buffer: invalid process token");
+        return;
+    }
+
+    std::lock_guard lock(mMutex);
+
+    if (mBuffers.find(processToken) == mBuffers.end()) {
+        ALOGE("failed to uncache buffer: process token not found");
+        return;
     }
 
+    auto& processBuffers = mBuffers[processToken];
+    processBuffers.erase(id);
+}
+
+sp<GraphicBuffer> BufferStateLayerCache::get(const sp<IBinder>& processToken, uint64_t id) {
     if (!processToken) {
         ALOGE("failed to cache buffer: invalid process token");
         return nullptr;
@@ -82,8 +93,8 @@ sp<GraphicBuffer> BufferStateLayerCache::get(sp<IBinder> processToken, int32_t i
         return nullptr;
     }
 
-    if (id >= itr->second.size()) {
-        ALOGE("failed to get buffer: id outside the bounds of the cache");
+    if (itr->second.find(id) == itr->second.end()) {
+        ALOGE("failed to get buffer: buffer not found");
         return nullptr;
     }
 
-- 
cgit v1.2.3-59-g8ed1b