Use IBinder to identify callbacks in VHAL

Right now there's no way to distinguish multiple clients, thus we cannot
associate IVehicleCallback from subscribe and unsubscribe method. As a
temporary solution we will use pointer to IBinder from underlying proxy
object until HILD will have some API to do that.

Test: verified unit tests run, checked that subsequent 'subscribe' calls
do not create new client objects in VHAL

Bug: b/37254245
Change-Id: I029564b7b03e4821287c2110d0929a5c4cbce0ce

5 files changed, 59 insertions, 32 deletions

diff --git a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h
index fd59802923..8e9089d600 100644
--- a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h
+++ b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h
@@ -78,6 +78,8 @@ struct HalClientValues {
     std::list<VehiclePropValue *> values;
 };
 
+using ClientId = uint64_t;
+
 class SubscriptionManager {
 public:
     using OnPropertyUnsubscribed = std::function<void(int32_t)>;
@@ -100,7 +102,8 @@ public:
      * Updates subscription. Returns the vector of properties subscription that
      * needs to be updated in VehicleHAL.
      */
-    StatusCode addOrUpdateSubscription(const sp<IVehicleCallback>& callback,
+    StatusCode addOrUpdateSubscription(ClientId clientId,
+                                       const sp<IVehicleCallback>& callback,
                                        const hidl_vec<SubscribeOptions>& optionList,
                                        std::list<SubscribeOptions>* outUpdatedOptions);
 
@@ -119,7 +122,7 @@ public:
      * If there are no clients subscribed to given properties than callback function provided
      * in the constructor will be called.
      */
-    void unsubscribe(const sp<IVehicleCallback>& callback, int32_t propId);
+    void unsubscribe(ClientId clientId, int32_t propId);
 private:
     std::list<sp<HalClient>> getSubscribedClientsLocked(int32_t propId,
                                                         int32_t area,
@@ -131,7 +134,8 @@ private:
 
     sp<HalClientVector> getClientsForPropertyLocked(int32_t propId) const;
 
-    sp<HalClient> getOrCreateHalClientLocked(const sp<IVehicleCallback> &callback);
+    sp<HalClient> getOrCreateHalClientLocked(ClientId callingPid,
+                                             const sp<IVehicleCallback>& callback);
 
     void onCallbackDead(uint64_t cookie);
 
@@ -160,7 +164,7 @@ private:
 
     mutable std::mutex mLock;
 
-    std::map<sp<IVehicleCallback>, sp<HalClient>> mClients;
+    std::map<ClientId, sp<HalClient>> mClients;
     std::map<int32_t, sp<HalClientVector>> mPropToClients;
     std::map<int32_t, SubscribeOptions> mHalEventSubscribeOptions;
 
diff --git a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h
index 1d45f4b6e0..c1e9e88609 100644
--- a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h
+++ b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h
@@ -101,6 +101,7 @@ private:
     static bool isSampleRateFixed(VehiclePropertyChangeMode mode);
     static float checkSampleRate(const VehiclePropConfig& config,
                                  float sampleRate);
+    static ClientId getClientId(const sp<IVehicleCallback>& callback);
 private:
     VehicleHal* mHal;
     std::unique_ptr<VehiclePropConfigIndex> mConfigIndex;
diff --git a/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp b/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp
index e0f3f31a3c..74f0a5f55e 100644
--- a/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp
+++ b/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp
@@ -97,6 +97,7 @@ std::vector<int32_t> HalClient::getSubscribedProperties() const {
 }
 
 StatusCode SubscriptionManager::addOrUpdateSubscription(
+        ClientId clientId,
         const sp<IVehicleCallback> &callback,
         const hidl_vec<SubscribeOptions> &optionList,
         std::list<SubscribeOptions>* outUpdatedSubscriptions) {
@@ -106,7 +107,7 @@ StatusCode SubscriptionManager::addOrUpdateSubscription(
 
     ALOGI("SubscriptionManager::addOrUpdateSubscription, callback: %p", callback.get());
 
-    const sp<HalClient>& client = getOrCreateHalClientLocked(callback);
+    const sp<HalClient>& client = getOrCreateHalClientLocked(clientId, callback);
     if (client.get() == nullptr) {
         return StatusCode::INTERNAL_ERROR;
     }
@@ -221,10 +222,11 @@ sp<HalClientVector> SubscriptionManager::getClientsForPropertyLocked(
 }
 
 sp<HalClient> SubscriptionManager::getOrCreateHalClientLocked(
-        const sp<IVehicleCallback>& callback) {
-    auto it = mClients.find(callback);
+        ClientId clientId, const sp<IVehicleCallback>& callback) {
+    auto it = mClients.find(clientId);
+
     if (it == mClients.end()) {
-        uint64_t cookie = reinterpret_cast<uint64_t>(callback.get());
+        uint64_t cookie = reinterpret_cast<uint64_t>(clientId);
         ALOGI("Creating new client and linking to death recipient, cookie: 0x%" PRIx64, cookie);
         auto res = callback->linkToDeath(mCallbackDeathRecipient, cookie);
         if (!res.isOk()) {  // Client is already dead?
@@ -234,18 +236,18 @@ sp<HalClient> SubscriptionManager::getOrCreateHalClientLocked(
         }
 
         sp<HalClient> client = new HalClient(callback);
-        mClients.emplace(callback, client);
+        mClients.insert({clientId, client});
         return client;
     } else {
         return it->second;
     }
 }
 
-void SubscriptionManager::unsubscribe(const sp<IVehicleCallback>& callback,
+void SubscriptionManager::unsubscribe(ClientId clientId,
                                       int32_t propId) {
     MuxGuard g(mLock);
     auto propertyClients = getClientsForPropertyLocked(propId);
-    auto clientIter = mClients.find(callback);
+    auto clientIter = mClients.find(clientId);
     if (clientIter == mClients.end()) {
         ALOGW("Unable to unsubscribe: no callback found, propId: 0x%x", propId);
     } else {
@@ -285,12 +287,12 @@ void SubscriptionManager::unsubscribe(const sp<IVehicleCallback>& callback,
 
 void SubscriptionManager::onCallbackDead(uint64_t cookie) {
     ALOGI("%s, cookie: 0x%" PRIx64, __func__, cookie);
-    IVehicleCallback* callback = reinterpret_cast<IVehicleCallback*>(cookie);
+    ClientId clientId = cookie;
 
     std::vector<int32_t> props;
     {
         MuxGuard g(mLock);
-        const auto& it = mClients.find(callback);
+        const auto& it = mClients.find(clientId);
         if (it == mClients.end()) {
             return;  // Nothing to do here, client wasn't subscribed to any properties.
         }
@@ -299,7 +301,7 @@ void SubscriptionManager::onCallbackDead(uint64_t cookie) {
     }
 
     for (int32_t propId : props) {
-        unsubscribe(callback, propId);
+        unsubscribe(clientId, propId);
     }
 }
 
diff --git a/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp b/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp
index f452be8978..ae543bb3fc 100644
--- a/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp
+++ b/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp
@@ -22,6 +22,7 @@
 #include <fstream>
 
 #include <android/log.h>
+#include <android/hardware/automotive/vehicle/2.0/BpHwVehicleCallback.h>
 
 #include "VehicleUtils.h"
 
@@ -154,7 +155,8 @@ Return<StatusCode> VehicleHalManager::subscribe(const sp<IVehicleCallback> &call
     }
 
     std::list<SubscribeOptions> updatedOptions;
-    auto res = mSubscriptionManager.addOrUpdateSubscription(callback, verifiedOptions,
+    auto res = mSubscriptionManager.addOrUpdateSubscription(getClientId(callback),
+                                                            callback, verifiedOptions,
                                                             &updatedOptions);
     if (StatusCode::OK != res) {
         ALOGW("%s failed to subscribe, error code: %d", __func__, res);
@@ -170,7 +172,7 @@ Return<StatusCode> VehicleHalManager::subscribe(const sp<IVehicleCallback> &call
 
 Return<StatusCode> VehicleHalManager::unsubscribe(const sp<IVehicleCallback>& callback,
                                                   int32_t propId) {
-    mSubscriptionManager.unsubscribe(callback, propId);
+    mSubscriptionManager.unsubscribe(getClientId(callback), propId);
     return StatusCode::OK;
 }
 
@@ -341,6 +343,18 @@ void VehicleHalManager::onAllClientsUnsubscribed(int32_t propertyId) {
     mHal->unsubscribe(propertyId);
 }
 
+ClientId VehicleHalManager::getClientId(const sp<IVehicleCallback>& callback) {
+    //TODO(b/32172906): rework this to get some kind of unique id for callback interface when this
+    // feature is ready in HIDL.
+
+    if (callback->isRemote()) {
+        BpHwVehicleCallback* hwCallback = static_cast<BpHwVehicleCallback*>(callback.get());
+        return static_cast<ClientId>(reinterpret_cast<intptr_t>(hwCallback->onAsBinder()));
+    } else {
+        return static_cast<ClientId>(reinterpret_cast<intptr_t>(callback.get()));
+    }
+}
+
 }  // namespace V2_0
 }  // namespace vehicle
 }  // namespace automotive
diff --git a/automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp b/automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp
index 7ec9b798a3..5688dd6da1 100644
--- a/automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp
+++ b/automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp
@@ -119,8 +119,10 @@ private:
 
 TEST_F(SubscriptionManagerTest, multipleClients) {
     std::list<SubscribeOptions> updatedOptions;
-    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));
-    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb2, subscrToProp1, &updatedOptions));
+    ASSERT_EQ(StatusCode::OK,
+              manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));
+    ASSERT_EQ(StatusCode::OK,
+              manager.addOrUpdateSubscription(2, cb2, subscrToProp1, &updatedOptions));
 
     auto clients = manager.getSubscribedClients(
             PROP1,
@@ -132,7 +134,8 @@ TEST_F(SubscriptionManagerTest, multipleClients) {
 
 TEST_F(SubscriptionManagerTest, negativeCases) {
     std::list<SubscribeOptions> updatedOptions;
-    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));
+    ASSERT_EQ(StatusCode::OK,
+              manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));
 
     // Wrong zone
     auto clients = manager.getSubscribedClients(
@@ -158,7 +161,8 @@ TEST_F(SubscriptionManagerTest, negativeCases) {
 
 TEST_F(SubscriptionManagerTest, mulipleSubscriptions) {
     std::list<SubscribeOptions> updatedOptions;
-    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));
+    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(1, cb1, subscrToProp1,
+                                                              &updatedOptions));
 
     auto clients = manager.getSubscribedClients(
             PROP1,
@@ -169,7 +173,7 @@ TEST_F(SubscriptionManagerTest, mulipleSubscriptions) {
 
     // Same property, but different zone, to make sure we didn't unsubscribe
     // from previous zone.
-    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, {
+    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(1, cb1, {
         SubscribeOptions {
                 .propId = PROP1,
                 .vehicleAreas = toInt(VehicleAreaZone::ROW_2),
@@ -190,15 +194,17 @@ TEST_F(SubscriptionManagerTest, mulipleSubscriptions) {
 
 TEST_F(SubscriptionManagerTest, unsubscribe) {
     std::list<SubscribeOptions> updatedOptions;
-    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));
-    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb2, subscrToProp2, &updatedOptions));
-    ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb3, subscrToProp1and2,
-                                                              &updatedOptions));
-
-    ASSERT_ALL_EXISTS({cb1, cb3}, extractCallbacks(clientsToProp1()));
+    ASSERT_EQ(StatusCode::OK,
+              manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));
+    ASSERT_EQ(StatusCode::OK,
+              manager.addOrUpdateSubscription(2, cb2, subscrToProp2, &updatedOptions));
+    ASSERT_EQ(StatusCode::OK,
+              manager.addOrUpdateSubscription(3, cb3, subscrToProp1and2, &updatedOptions));
+
+    ASSERT_ALL_EXISTS({ cb1, cb3 }, extractCallbacks(clientsToProp1()));
     ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));
 
-    manager.unsubscribe(cb1, PROP1);
+    manager.unsubscribe(1, PROP1);
     assertOnPropertyUnsubscribedNotCalled();
     ASSERT_ALL_EXISTS({cb3}, extractCallbacks(clientsToProp1()));
 
@@ -206,20 +212,20 @@ TEST_F(SubscriptionManagerTest, unsubscribe) {
     ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));
 
     // No one subscribed to PROP1, subscription for PROP2 is not affected.
-    manager.unsubscribe(cb3, PROP1);
+    manager.unsubscribe(3, PROP1);
     assertLastUnsubscribedProperty(PROP1);
     ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));
 
-    manager.unsubscribe(cb3, PROP2);
+    manager.unsubscribe(3, PROP2);
     assertOnPropertyUnsubscribedNotCalled();
     ASSERT_ALL_EXISTS({cb2}, extractCallbacks(clientsToProp2()));
 
     // The last client unsubscribed from this property.
-    manager.unsubscribe(cb2, PROP2);
+    manager.unsubscribe(2, PROP2);
     assertLastUnsubscribedProperty(PROP2);
 
     // No one subscribed anymore
-    manager.unsubscribe(cb1, PROP1);
+    manager.unsubscribe(1, PROP1);
     assertLastUnsubscribedProperty(PROP1);
 }