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
diff --git a/services/bufferhub/Android.bp b/services/bufferhub/Android.bp
index b747dbc..f9aaa20 100644
--- a/services/bufferhub/Android.bp
+++ b/services/bufferhub/Android.bp
@@ -24,9 +24,9 @@
],
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
similarity index 81%
rename from services/bufferhub/UniqueIdGenerator.cpp
rename to services/bufferhub/BufferHubIdGenerator.cpp
index 362a026..6444a03 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 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 @@
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 6f97f0d..6eaf0ab 100644
--- a/services/bufferhub/BufferHubService.cpp
+++ b/services/bufferhub/BufferHubService.cpp
@@ -35,7 +35,8 @@
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 715d0a1..ec84849 100644
--- a/services/bufferhub/BufferNode.cpp
+++ b/services/bufferhub/BufferNode.cpp
@@ -66,8 +66,8 @@
}
// 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
similarity index 89%
rename from services/bufferhub/include/bufferhub/UniqueIdGenerator.h
rename to services/bufferhub/include/bufferhub/BufferHubIdGenerator.h
index d2e702f..c5b2cde 100644
--- a/services/bufferhub/include/bufferhub/UniqueIdGenerator.h
+++ b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h
@@ -29,11 +29,14 @@
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 @@
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 6535659..63997ae 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::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 c490e7c..94ef505 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 @@
// 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 8a30ef5..3967886 100644
--- a/services/bufferhub/tests/Android.bp
+++ b/services/bufferhub/tests/Android.bp
@@ -24,10 +24,10 @@
}
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/BufferHubIdGenerator_test.cpp b/services/bufferhub/tests/BufferHubIdGenerator_test.cpp
new file mode 100644
index 0000000..4eddfe0
--- /dev/null
+++ b/services/bufferhub/tests/BufferHubIdGenerator_test.cpp
@@ -0,0 +1,45 @@
+#include <bufferhub/BufferHubIdGenerator.h>
+#include <gtest/gtest.h>
+
+namespace android {
+namespace frameworks {
+namespace bufferhub {
+namespace V1_0 {
+namespace implementation {
+
+namespace {
+
+class BufferHubIdGeneratorTest : public testing::Test {
+protected:
+ BufferHubIdGenerator* mIdGenerator = &BufferHubIdGenerator::getInstance();
+};
+
+TEST_F(BufferHubIdGeneratorTest, TestGenerateAndFreeID) {
+ uint32_t id = mIdGenerator->getId();
+ EXPECT_NE(id, BufferHubIdGenerator::kInvalidId);
+
+ EXPECT_TRUE(mIdGenerator->freeId(id));
+ EXPECT_FALSE(mIdGenerator->freeId(id));
+}
+
+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], BufferHubIdGenerator::kInvalidId);
+ if (i >= 1) {
+ EXPECT_GT(ids[i], ids[i - 1]);
+ }
+ }
+}
+
+} // namespace
+
+} // namespace implementation
+} // namespace V1_0
+} // namespace bufferhub
+} // namespace frameworks
+} // namespace android
\ No newline at end of file
diff --git a/services/bufferhub/tests/UniqueIdGenerator_test.cpp b/services/bufferhub/tests/UniqueIdGenerator_test.cpp
deleted file mode 100644
index c4d83e0..0000000
--- a/services/bufferhub/tests/UniqueIdGenerator_test.cpp
+++ /dev/null
@@ -1,45 +0,0 @@
-#include <bufferhub/UniqueIdGenerator.h>
-#include <gtest/gtest.h>
-
-namespace android {
-namespace frameworks {
-namespace bufferhub {
-namespace V1_0 {
-namespace implementation {
-
-namespace {
-
-class UniqueIdGeneratorTest : public testing::Test {
-protected:
- UniqueIdGenerator mIdGenerator;
-};
-
-TEST_F(UniqueIdGeneratorTest, TestGenerateAndFreeID) {
- uint32_t id = mIdGenerator.getId();
- EXPECT_NE(id, UniqueIdGenerator::kInvalidId);
-
- EXPECT_TRUE(mIdGenerator.freeId(id));
- EXPECT_FALSE(mIdGenerator.freeId(id));
-}
-
-TEST_F(UniqueIdGeneratorTest, 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);
- if (i >= 1) {
- EXPECT_GT(ids[i], ids[i - 1]);
- }
- }
-}
-
-} // namespace
-
-} // namespace implementation
-} // namespace V1_0
-} // namespace bufferhub
-} // namespace frameworks
-} // namespace android
\ No newline at end of file