From a15f6973534c0a5b7c5cc29bbaf7ce5598c5e062 Mon Sep 17 00:00:00 2001
From: Eric Chung <eric.chung@mediatek.com>
Date: Mon, 9 Sep 2019 22:57:52 +0800
Subject: [PATCH] Refine freeBuffer in Mapper 2.0

When users call Mapper:freeBuffer, Mapper will erase buffer handle from mBufferHandles.
No matter the result of free buffer handle returned from gralloc, buffer handle is removed from mBufferHandles.
This means that a buffer handle can not be freed twice, even if it fail to be freed at the first time.
Because users will receive nullptr when they call freeBuffer to free the same bufferHandle at the second time.

When freeBuffer is called, Mapper only looks for input buffer in mBufferHandles instead of erasing it from mBufferHandles.
If the result of freeBuffer returned by gralloc is NONE, then remove the buffer handle from mBufferHandles.

Test: Manual
Bug: 141145482

Change-Id: I4e27f54eb219f23a5844d6b440d7160b296c31e2
---
 .../utils/hal/include/mapper-hal/2.0/Mapper.h | 20 +++++++++++++------
 .../mapper-passthrough/2.0/GrallocLoader.h    | 15 ++++++++++++--
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/graphics/mapper/2.0/utils/hal/include/mapper-hal/2.0/Mapper.h b/graphics/mapper/2.0/utils/hal/include/mapper-hal/2.0/Mapper.h
index 5ad2a6564c..81341742c7 100644
--- a/graphics/mapper/2.0/utils/hal/include/mapper-hal/2.0/Mapper.h
+++ b/graphics/mapper/2.0/utils/hal/include/mapper-hal/2.0/Mapper.h
@@ -80,17 +80,21 @@ class MapperImpl : public Interface {
     }
 
     Return<Error> freeBuffer(void* buffer) override {
-        native_handle_t* bufferHandle = removeImportedBuffer(buffer);
+        native_handle_t* bufferHandle = getImportedBuffer(buffer);
         if (!bufferHandle) {
             return Error::BAD_BUFFER;
         }
 
-        return mHal->freeBuffer(bufferHandle);
+        Error error = mHal->freeBuffer(bufferHandle);
+        if (error == Error::NONE) {
+            removeImportedBuffer(buffer);
+        }
+        return error;
     }
 
     Return<void> lock(void* buffer, uint64_t cpuUsage, const V2_0::IMapper::Rect& accessRegion,
                       const hidl_handle& acquireFence, IMapper::lock_cb hidl_cb) override {
-        const native_handle_t* bufferHandle = getImportedBuffer(buffer);
+        const native_handle_t* bufferHandle = getConstImportedBuffer(buffer);
         if (!bufferHandle) {
             hidl_cb(Error::BAD_BUFFER, nullptr);
             return Void();
@@ -112,7 +116,7 @@ class MapperImpl : public Interface {
     Return<void> lockYCbCr(void* buffer, uint64_t cpuUsage, const V2_0::IMapper::Rect& accessRegion,
                            const hidl_handle& acquireFence,
                            IMapper::lockYCbCr_cb hidl_cb) override {
-        const native_handle_t* bufferHandle = getImportedBuffer(buffer);
+        const native_handle_t* bufferHandle = getConstImportedBuffer(buffer);
         if (!bufferHandle) {
             hidl_cb(Error::BAD_BUFFER, YCbCrLayout{});
             return Void();
@@ -132,7 +136,7 @@ class MapperImpl : public Interface {
     }
 
     Return<void> unlock(void* buffer, IMapper::unlock_cb hidl_cb) override {
-        const native_handle_t* bufferHandle = getImportedBuffer(buffer);
+        const native_handle_t* bufferHandle = getConstImportedBuffer(buffer);
         if (!bufferHandle) {
             hidl_cb(Error::BAD_BUFFER, nullptr);
             return Void();
@@ -160,7 +164,11 @@ class MapperImpl : public Interface {
         return static_cast<native_handle_t*>(buffer);
     }
 
-    virtual const native_handle_t* getImportedBuffer(void* buffer) const {
+    virtual native_handle_t* getImportedBuffer(void* buffer) const {
+        return static_cast<native_handle_t*>(buffer);
+    }
+
+    virtual const native_handle_t* getConstImportedBuffer(void* buffer) const {
         return static_cast<const native_handle_t*>(buffer);
     }
 
diff --git a/graphics/mapper/2.0/utils/passthrough/include/mapper-passthrough/2.0/GrallocLoader.h b/graphics/mapper/2.0/utils/passthrough/include/mapper-passthrough/2.0/GrallocLoader.h
index e8b1b4b837..85a91c3ada 100644
--- a/graphics/mapper/2.0/utils/passthrough/include/mapper-passthrough/2.0/GrallocLoader.h
+++ b/graphics/mapper/2.0/utils/passthrough/include/mapper-passthrough/2.0/GrallocLoader.h
@@ -68,7 +68,14 @@ class GrallocImportedBufferPool {
         return mBufferHandles.erase(bufferHandle) == 1 ? bufferHandle : nullptr;
     }
 
-    const native_handle_t* get(void* buffer) {
+    native_handle_t* get(void* buffer) {
+        auto bufferHandle = static_cast<native_handle_t*>(buffer);
+
+        std::lock_guard<std::mutex> lock(mMutex);
+        return mBufferHandles.count(bufferHandle) == 1 ? bufferHandle : nullptr;
+    }
+
+    const native_handle_t* getConst(void* buffer) {
         auto bufferHandle = static_cast<const native_handle_t*>(buffer);
 
         std::lock_guard<std::mutex> lock(mMutex);
@@ -92,9 +99,13 @@ class GrallocMapper : public T {
         return GrallocImportedBufferPool::getInstance().remove(buffer);
     }
 
-    const native_handle_t* getImportedBuffer(void* buffer) const override {
+    native_handle_t* getImportedBuffer(void* buffer) const override {
         return GrallocImportedBufferPool::getInstance().get(buffer);
     }
+
+    const native_handle_t* getConstImportedBuffer(void* buffer) const override {
+        return GrallocImportedBufferPool::getInstance().getConst(buffer);
+    }
 };
 
 class GrallocLoader {