Add duplicate to BufferClient
Calling BufferClient::duplicate() will now return you an uint64_t token
stands for a specific BufferNode. In later CLs users could pass this
IPC-friendly token everywhere and use it to create a IBufferClient
linked to that BufferNode.
The token is derived from the mt19936_64 random algorithm. No same
tokens will be generated at the same time by checking the token_map_.
The map holds a weak_ptr so that if the BufferNode goes away by some
reason the map will not keep it, preventing from memory leak.
Add test case to test on the function.
Test: "atest buffer_hub_binder_service-test" passed.
Bug: 116681016
Change-Id: I54c34d9fabe882326faa141a6f0691419b740694
diff --git a/services/vr/bufferhubd/Android.bp b/services/vr/bufferhubd/Android.bp
index c35ddd4..ea9bb1f 100644
--- a/services/vr/bufferhubd/Android.bp
+++ b/services/vr/bufferhubd/Android.bp
@@ -29,6 +29,7 @@
name: "libbufferhubd",
srcs: [
"buffer_channel.cpp",
+ "buffer_client.cpp",
"buffer_hub.cpp",
"buffer_hub_binder.cpp",
"buffer_node.cpp",
diff --git a/services/vr/bufferhubd/IBufferHub.cpp b/services/vr/bufferhubd/IBufferHub.cpp
index 2f39e41..a2c0a4a 100644
--- a/services/vr/bufferhubd/IBufferHub.cpp
+++ b/services/vr/bufferhubd/IBufferHub.cpp
@@ -64,7 +64,7 @@
sp<IBufferClient> ret = createBuffer(width, height, layer_count, format,
usage, user_metadata_size);
return reply->writeStrongBinder(IInterface::asBinder(ret));
- } break;
+ }
default:
// Should not reach except binder defined transactions such as dumpsys
return BBinder::onTransact(code, data, reply, flags);
diff --git a/services/vr/bufferhubd/buffer_client.cpp b/services/vr/bufferhubd/buffer_client.cpp
new file mode 100644
index 0000000..f14faf7
--- /dev/null
+++ b/services/vr/bufferhubd/buffer_client.cpp
@@ -0,0 +1,18 @@
+#include <private/dvr/buffer_client.h>
+#include <private/dvr/buffer_hub_binder.h>
+
+namespace android {
+namespace dvr {
+
+status_t BufferClient::duplicate(uint64_t* outToken) {
+ if (!buffer_node_) {
+ // Should never happen
+ ALOGE("BufferClient::duplicate: node is missing.");
+ return UNEXPECTED_NULL;
+ }
+ return service_->registerToken(std::weak_ptr<BufferNode>(buffer_node_),
+ outToken);
+}
+
+} // namespace dvr
+} // namespace android
\ No newline at end of file
diff --git a/services/vr/bufferhubd/buffer_hub_binder.cpp b/services/vr/bufferhubd/buffer_hub_binder.cpp
index f8a9758..7b0dc9a 100644
--- a/services/vr/bufferhubd/buffer_hub_binder.cpp
+++ b/services/vr/bufferhubd/buffer_hub_binder.cpp
@@ -65,13 +65,23 @@
return NO_ERROR;
}
+status_t BufferHubBinderService::registerToken(
+ const std::weak_ptr<BufferNode> node, uint64_t* outToken) {
+ do {
+ *outToken = token_engine_();
+ } while (token_map_.find(*outToken) != token_map_.end());
+
+ token_map_.emplace(*outToken, node);
+ return NO_ERROR;
+}
+
sp<IBufferClient> BufferHubBinderService::createBuffer(
uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format,
uint64_t usage, uint64_t user_metadata_size) {
std::shared_ptr<BufferNode> node = std::make_shared<BufferNode>(
width, height, layer_count, format, usage, user_metadata_size);
- sp<BufferClient> client = new BufferClient(node);
+ sp<BufferClient> client = new BufferClient(node, this);
// Add it to list for bookkeeping and dumpsys.
client_list_.push_back(client);
diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_client.h b/services/vr/bufferhubd/include/private/dvr/buffer_client.h
index 20d51ee..d79ec0a 100644
--- a/services/vr/bufferhubd/include/private/dvr/buffer_client.h
+++ b/services/vr/bufferhubd/include/private/dvr/buffer_client.h
@@ -7,19 +7,31 @@
namespace android {
namespace dvr {
+// Forward declaration to avoid circular dependency
+class BufferHubBinderService;
+
class BufferClient : public BnBufferClient {
public:
// Creates a server-side buffer client from an existing BufferNode. Note that
// this funciton takes ownership of the shared_ptr.
- explicit BufferClient(std::shared_ptr<BufferNode> node)
- : buffer_node_(std::move(node)){};
+ explicit BufferClient(std::shared_ptr<BufferNode> node,
+ BufferHubBinderService* service)
+ : service_(service), buffer_node_(std::move(node)){};
// Binder IPC functions
bool isValid() override {
return buffer_node_ ? buffer_node_->IsValid() : false;
};
+ status_t duplicate(uint64_t* outToken) override;
+
private:
+ // Hold a pointer to the service to bypass binder interface, as BufferClient
+ // and the service will be in the same process. Also, since service owns
+ // Client, if service dead the clients will be destroyed, so this pointer is
+ // guaranteed to be valid.
+ BufferHubBinderService* service_;
+
std::shared_ptr<BufferNode> buffer_node_;
};
diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_hub_binder.h b/services/vr/bufferhubd/include/private/dvr/buffer_hub_binder.h
index 9064d87..6c74cc4 100644
--- a/services/vr/bufferhubd/include/private/dvr/buffer_hub_binder.h
+++ b/services/vr/bufferhubd/include/private/dvr/buffer_hub_binder.h
@@ -1,12 +1,15 @@
#ifndef ANDROID_DVR_BUFFER_HUB_BINDER_H
#define ANDROID_DVR_BUFFER_HUB_BINDER_H
+#include <random>
+#include <unordered_map>
#include <vector>
#include <binder/BinderService.h>
#include <private/dvr/IBufferHub.h>
#include <private/dvr/buffer_client.h>
#include <private/dvr/buffer_hub.h>
+#include <private/dvr/buffer_node.h>
namespace android {
namespace dvr {
@@ -15,10 +18,15 @@
public BnBufferHub {
public:
static status_t start(const std::shared_ptr<BufferHubService>& pdx_service);
- // Dump bufferhub related information to given fd (usually stdout)
+ // Dumps bufferhub related information to given fd (usually stdout)
// usage: adb shell dumpsys bufferhubd
virtual status_t dump(int fd, const Vector<String16>& args) override;
+ // Marks a BufferNode to be duplicated.
+ // TODO(b/116681016): add importToken(int64_t)
+ status_t registerToken(const std::weak_ptr<BufferNode> node,
+ uint64_t* outToken);
+
// Binder IPC functions
sp<IBufferClient> createBuffer(uint32_t width, uint32_t height,
uint32_t layer_count, uint32_t format,
@@ -29,6 +37,12 @@
std::shared_ptr<BufferHubService> pdx_service_;
std::vector<sp<BufferClient>> client_list_;
+
+ // TODO(b/118180214): use a more secure implementation
+ std::mt19937_64 token_engine_;
+ // The mapping from token to a specific node. This is a many-to-one mapping.
+ // One node could be refered by 0 to multiple tokens.
+ std::unordered_map<uint64_t, std::weak_ptr<BufferNode>> token_map_;
};
} // namespace dvr
diff --git a/services/vr/bufferhubd/tests/buffer_hub_binder_service-test.cpp b/services/vr/bufferhubd/tests/buffer_hub_binder_service-test.cpp
index 7fa2226..393f13e 100644
--- a/services/vr/bufferhubd/tests/buffer_hub_binder_service-test.cpp
+++ b/services/vr/bufferhubd/tests/buffer_hub_binder_service-test.cpp
@@ -38,6 +38,23 @@
EXPECT_TRUE(bufferClient->isValid());
}
+TEST_F(BufferHubBinderServiceTest, TestDuplicateBuffer) {
+ sp<IBufferClient> bufferClient = service->createBuffer(
+ kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize);
+ EXPECT_THAT(bufferClient, NotNull());
+ EXPECT_TRUE(bufferClient->isValid());
+
+ uint64_t token1 = 0ULL;
+ status_t ret = bufferClient->duplicate(&token1);
+ EXPECT_EQ(ret, NO_ERROR);
+
+ // Should be different
+ uint64_t token2 = 0ULL;
+ ret = bufferClient->duplicate(&token2);
+ EXPECT_EQ(ret, NO_ERROR);
+ EXPECT_NE(token2, token1);
+}
+
} // namespace
} // namespace dvr