Fix freeId() failed in ~BufferNode

And rename UniqueIdGenerator to BufferHubIdGenerator.

Created a static getter and Use scoped static initialization for
UniqueIdGenerator.

Test: run BufferHubBuffer_test, then "adb logcat | grep bufferhub -i" to
verify that there are no error messages.
Fix: 118844348
Change-Id: Ia75ae4dac13cc709f39161c803401bcbb90c70e2

9 files changed, 39 insertions, 28 deletions

diff --git a/services/bufferhub/Android.bp b/services/bufferhub/Android.bp
index b747dbca4b..f9aaa2050f 100644
--- a/services/bufferhub/Android.bp
+++ b/services/bufferhub/Android.bp
@@ -24,9 +24,9 @@ cc_library_shared {
     ],
     srcs: [
         "BufferClient.cpp",
+        "BufferHubIdGenerator.cpp",
         "BufferHubService.cpp",
         "BufferNode.cpp",
-        "UniqueIdGenerator.cpp",
     ],
     header_libs: [
         "libbufferhub_headers",
diff --git a/services/bufferhub/UniqueIdGenerator.cpp b/services/bufferhub/BufferHubIdGenerator.cpp
index 362a026f6c..6444a033e1 100644
--- a/services/bufferhub/UniqueIdGenerator.cpp
+++ b/services/bufferhub/BufferHubIdGenerator.cpp
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#include <bufferhub/UniqueIdGenerator.h>
+#include <bufferhub/BufferHubIdGenerator.h>
 
 namespace android {
 namespace frameworks {
@@ -22,9 +22,15 @@ namespace bufferhub {
 namespace V1_0 {
 namespace implementation {
 
-constexpr uint32_t UniqueIdGenerator::kInvalidId;
+constexpr uint32_t BufferHubIdGenerator::kInvalidId;
 
-uint32_t UniqueIdGenerator::getId() {
+BufferHubIdGenerator& BufferHubIdGenerator::getInstance() {
+    static BufferHubIdGenerator generator;
+
+    return generator;
+}
+
+uint32_t BufferHubIdGenerator::getId() {
     std::lock_guard<std::mutex> lock(mIdsInUseMutex);
 
     do {
@@ -37,7 +43,7 @@ uint32_t UniqueIdGenerator::getId() {
     return mLastId;
 }
 
-bool UniqueIdGenerator::freeId(uint32_t id) {
+bool BufferHubIdGenerator::freeId(uint32_t id) {
     std::lock_guard<std::mutex> lock(mIdsInUseMutex);
     auto iter = mIdsInUse.find(id);
     if (iter != mIdsInUse.end()) {
diff --git a/services/bufferhub/BufferHubService.cpp b/services/bufferhub/BufferHubService.cpp
index 6f97f0d8c0..6eaf0abe8e 100644
--- a/services/bufferhub/BufferHubService.cpp
+++ b/services/bufferhub/BufferHubService.cpp
@@ -35,7 +35,8 @@ Return<void> BufferHubService::allocateBuffer(const HardwareBufferDescription& d
 
     std::shared_ptr<BufferNode> node =
             std::make_shared<BufferNode>(desc.width, desc.height, desc.layers, desc.format,
-                                         desc.usage, userMetadataSize, nodeIdGenerator.getId());
+                                         desc.usage, userMetadataSize,
+                                         BufferHubIdGenerator::getInstance().getId());
     if (node == nullptr || !node->IsValid()) {
         ALOGE("%s: creating BufferNode failed.", __FUNCTION__);
         _hidl_cb(/*bufferClient=*/nullptr, /*status=*/BufferHubStatus::ALLOCATION_FAILED);
diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp
index 715d0a1f67..ec84849e22 100644
--- a/services/bufferhub/BufferNode.cpp
+++ b/services/bufferhub/BufferNode.cpp
@@ -66,8 +66,8 @@ BufferNode::~BufferNode() {
     }
 
     // Free the id, if valid
-    if (id() != UniqueIdGenerator::kInvalidId) {
-        if (nodeIdGenerator.freeId(id())) {
+    if (id() != BufferHubIdGenerator::kInvalidId) {
+        if (BufferHubIdGenerator::getInstance().freeId(id())) {
             ALOGI("%s: id #%u is freed.", __FUNCTION__, id());
         } else {
             ALOGE("%s: Cannot free nonexistent id #%u", __FUNCTION__, id());
diff --git a/services/bufferhub/include/bufferhub/UniqueIdGenerator.h b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h
index d2e702f7e0..c5b2cdef33 100644
--- a/services/bufferhub/include/bufferhub/UniqueIdGenerator.h
+++ b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h
@@ -29,11 +29,14 @@ namespace V1_0 {
 namespace implementation {
 
 // A thread-safe incremental uint32_t id generator.
-class UniqueIdGenerator {
+class BufferHubIdGenerator {
 public:
     // 0 is considered invalid
     static constexpr uint32_t kInvalidId = 0UL;
 
+    // Get the singleton instance of this class
+    static BufferHubIdGenerator& getInstance();
+
     // Gets next available id. If next id is greater than std::numeric_limits<uint32_t>::max() (2 ^
     // 32 - 1), it will try to get an id start from 1 again.
     uint32_t getId();
@@ -42,6 +45,9 @@ public:
     bool freeId(uint32_t id);
 
 private:
+    BufferHubIdGenerator() = default;
+    ~BufferHubIdGenerator() = default;
+
     std::mutex mIdsInUseMutex;
     // Start from kInvalidID to avoid generating it.
     uint32_t mLastId = kInvalidId;
diff --git a/services/bufferhub/include/bufferhub/BufferHubService.h b/services/bufferhub/include/bufferhub/BufferHubService.h
index 6535659ae7..63997aeff9 100644
--- a/services/bufferhub/include/bufferhub/BufferHubService.h
+++ b/services/bufferhub/include/bufferhub/BufferHubService.h
@@ -22,7 +22,7 @@
 
 #include <android/frameworks/bufferhub/1.0/IBufferHub.h>
 #include <bufferhub/BufferClient.h>
-#include <bufferhub/UniqueIdGenerator.h>
+#include <bufferhub/BufferHubIdGenerator.h>
 #include <utils/Mutex.h>
 
 namespace android {
@@ -35,8 +35,6 @@ using hardware::hidl_handle;
 using hardware::Return;
 using hardware::graphics::common::V1_2::HardwareBufferDescription;
 
-static UniqueIdGenerator nodeIdGenerator;
-
 class BufferHubService : public IBufferHub {
 public:
     Return<void> allocateBuffer(const HardwareBufferDescription& description,
diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h
index c490e7c709..94ef505d41 100644
--- a/services/bufferhub/include/bufferhub/BufferNode.h
+++ b/services/bufferhub/include/bufferhub/BufferNode.h
@@ -2,7 +2,7 @@
 #define ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_BUFFER_NODE_H_
 
 #include <android/hardware_buffer.h>
-#include <bufferhub/UniqueIdGenerator.h>
+#include <bufferhub/BufferHubIdGenerator.h>
 #include <ui/BufferHubMetadata.h>
 
 namespace android {
@@ -16,7 +16,7 @@ public:
     // Allocates a new BufferNode.
     BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format,
                uint64_t usage, size_t user_metadata_size,
-               uint32_t id = UniqueIdGenerator::kInvalidId);
+               uint32_t id = BufferHubIdGenerator::kInvalidId);
 
     ~BufferNode();
 
diff --git a/services/bufferhub/tests/Android.bp b/services/bufferhub/tests/Android.bp
index 8a30ef5820..39678865cb 100644
--- a/services/bufferhub/tests/Android.bp
+++ b/services/bufferhub/tests/Android.bp
@@ -24,10 +24,10 @@ cc_test {
 }
 
 cc_test {
-    name: "UniqueIdGenerator_test",
-    srcs: ["UniqueIdGenerator_test.cpp"],
+    name: "BufferHubIdGenerator_test",
+    srcs: ["BufferHubIdGenerator_test.cpp"],
     cflags: [
-        "-DLOG_TAG=\"UniqueIdGenerator_test\"",
+        "-DLOG_TAG=\"BufferHubIdGenerator_test\"",
         "-DTRACE=0",
         "-DATRACE_TAG=ATRACE_TAG_GRAPHICS",
     ],
diff --git a/services/bufferhub/tests/UniqueIdGenerator_test.cpp b/services/bufferhub/tests/BufferHubIdGenerator_test.cpp
index c4d83e0c1f..4eddfe0e9b 100644
--- a/services/bufferhub/tests/UniqueIdGenerator_test.cpp
+++ b/services/bufferhub/tests/BufferHubIdGenerator_test.cpp
@@ -1,4 +1,4 @@
-#include <bufferhub/UniqueIdGenerator.h>
+#include <bufferhub/BufferHubIdGenerator.h>
 #include <gtest/gtest.h>
 
 namespace android {
@@ -9,27 +9,27 @@ namespace implementation {
 
 namespace {
 
-class UniqueIdGeneratorTest : public testing::Test {
+class BufferHubIdGeneratorTest : public testing::Test {
 protected:
-    UniqueIdGenerator mIdGenerator;
+    BufferHubIdGenerator* mIdGenerator = &BufferHubIdGenerator::getInstance();
 };
 
-TEST_F(UniqueIdGeneratorTest, TestGenerateAndFreeID) {
-    uint32_t id = mIdGenerator.getId();
-    EXPECT_NE(id, UniqueIdGenerator::kInvalidId);
+TEST_F(BufferHubIdGeneratorTest, TestGenerateAndFreeID) {
+    uint32_t id = mIdGenerator->getId();
+    EXPECT_NE(id, BufferHubIdGenerator::kInvalidId);
 
-    EXPECT_TRUE(mIdGenerator.freeId(id));
-    EXPECT_FALSE(mIdGenerator.freeId(id));
+    EXPECT_TRUE(mIdGenerator->freeId(id));
+    EXPECT_FALSE(mIdGenerator->freeId(id));
 }
 
-TEST_F(UniqueIdGeneratorTest, TestGenerateUniqueIncrementalID) {
+TEST_F(BufferHubIdGeneratorTest, TestGenerateUniqueIncrementalID) {
     // 10 IDs should not overflow the UniqueIdGenerator to cause a roll back to start, so the
     // resulting IDs should still keep incresing.
     const size_t kTestSize = 10U;
     uint32_t ids[kTestSize];
     for (int i = 0; i < kTestSize; ++i) {
-        ids[i] = mIdGenerator.getId();
-        EXPECT_NE(ids[i], UniqueIdGenerator::kInvalidId);
+        ids[i] = mIdGenerator->getId();
+        EXPECT_NE(ids[i], BufferHubIdGenerator::kInvalidId);
         if (i >= 1) {
             EXPECT_GT(ids[i], ids[i - 1]);
         }