Implement linkToDeath, use binder as client ID.
Implement linkToDeath for binders. Delete allocated resources for a
binder when it died or unlinked.
This CL also uses 'const AIBinder*' as client id type instead of
the callback because the Binder object corresponds to the remote
proxy and is guaranteed to be unique per client.
Bug: 204943359
Test: atest DefaultVehicleHalTest
Change-Id: If2e0c58e86a041a78b8ca69597aef4733ce1826c
diff --git a/automotive/vehicle/aidl/impl/vhal/include/ConnectedClient.h b/automotive/vehicle/aidl/impl/vhal/include/ConnectedClient.h
index 4f0b74a..833707a 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/ConnectedClient.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/ConnectedClient.h
@@ -42,10 +42,10 @@
// This class is thread-safe.
class ConnectedClient {
public:
- ConnectedClient(
- std::shared_ptr<PendingRequestPool> requestPool,
- std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
- callback);
+ using CallbackType =
+ std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>;
+
+ ConnectedClient(std::shared_ptr<PendingRequestPool> requestPool, CallbackType callback);
virtual ~ConnectedClient() = default;
@@ -68,8 +68,7 @@
virtual std::shared_ptr<const PendingRequestPool::TimeoutCallbackFunc> getTimeoutCallback() = 0;
const std::shared_ptr<PendingRequestPool> mRequestPool;
- const std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
- mCallback;
+ const CallbackType mCallback;
};
// A class to represent a client that calls {@code IVehicle.setValues} or {@code
@@ -77,10 +76,7 @@
template <class ResultType, class ResultsType>
class GetSetValuesClient final : public ConnectedClient {
public:
- GetSetValuesClient(
- std::shared_ptr<PendingRequestPool> requestPool,
- std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
- callback);
+ GetSetValuesClient(std::shared_ptr<PendingRequestPool> requestPool, CallbackType callback);
// Sends the results to this client.
void sendResults(const std::vector<ResultType>& results);
@@ -105,10 +101,7 @@
// A class to represent a client that calls {@code IVehicle.subscribe}.
class SubscriptionClient final : public ConnectedClient {
public:
- SubscriptionClient(
- std::shared_ptr<PendingRequestPool> requestPool,
- std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
- callback);
+ SubscriptionClient(std::shared_ptr<PendingRequestPool> requestPool, CallbackType callback);
// Gets the callback to be called when the request for this client has finished.
std::shared_ptr<const IVehicleHardware::GetValuesCallback> getResultCallback();
@@ -116,8 +109,7 @@
// Marshals the updated values into largeParcelable and sents it through {@code onPropertyEvent}
// callback.
static void sendUpdatedValues(
- std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
- callback,
+ CallbackType callback,
std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>&&
updatedValues);
@@ -132,9 +124,7 @@
std::shared_ptr<const IVehicleHardware::PropertyChangeCallback> mPropertyChangeCallback;
static void onGetValueResults(
- const void* clientId,
- std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
- callback,
+ const void* clientId, CallbackType callback,
std::shared_ptr<PendingRequestPool> requestPool,
std::vector<::aidl::android::hardware::automotive::vehicle::GetValueResult> results);
};
diff --git a/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h b/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
index b9975bc..62b2627 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
@@ -53,7 +53,7 @@
explicit DefaultVehicleHal(std::unique_ptr<IVehicleHardware> hardware);
- ~DefaultVehicleHal() = default;
+ ~DefaultVehicleHal();
::ndk::ScopedAStatus getAllPropConfigs(
::aidl::android::hardware::automotive::vehicle::VehiclePropConfigs* returnConfigs)
@@ -101,7 +101,7 @@
private:
std::mutex mLock;
- std::unordered_map<CallbackType, int64_t> mIds GUARDED_BY(mLock);
+ std::unordered_map<const AIBinder*, int64_t> mIds GUARDED_BY(mLock);
};
// A thread safe class to store all subscribe clients. This class is safe to pass to async
@@ -112,16 +112,42 @@
std::shared_ptr<SubscriptionClient> getClient(const CallbackType& callback);
+ void removeClient(const AIBinder* clientId);
+
size_t countClients();
private:
std::mutex mLock;
- std::unordered_map<CallbackType, std::shared_ptr<SubscriptionClient>> mClients
+ std::unordered_map<const AIBinder*, std::shared_ptr<SubscriptionClient>> mClients
GUARDED_BY(mLock);
// PendingRequestPool is thread-safe.
std::shared_ptr<PendingRequestPool> mPendingRequestPool;
};
+ // A wrapper for linkToDeath to enable stubbing for test.
+ class ILinkToDeath {
+ public:
+ virtual ~ILinkToDeath() = default;
+
+ virtual binder_status_t linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+ void* cookie) = 0;
+ };
+
+ // A real implementation for ILinkToDeath.
+ class AIBinderLinkToDeathImpl final : public ILinkToDeath {
+ public:
+ binder_status_t linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+ void* cookie) override;
+ };
+
+ // OnBinderDiedContext is a type used as a cookie passed deathRecipient. The deathRecipient's
+ // onBinderDied function takes only a cookie as input and we have to store all the contexts
+ // as the cookie.
+ struct OnBinderDiedContext {
+ DefaultVehicleHal* vhal;
+ const AIBinder* clientId;
+ };
+
// The default timeout of get or set value requests is 30s.
// TODO(b/214605968): define TIMEOUT_IN_NANO in IVehicle and allow getValues/setValues/subscribe
// to specify custom timeouts.
@@ -142,15 +168,22 @@
std::shared_ptr<SubscriptionManager> mSubscriptionManager;
std::mutex mLock;
- std::unordered_map<CallbackType, std::shared_ptr<GetValuesClient>> mGetValuesClients
+ std::unordered_map<const AIBinder*, std::unique_ptr<OnBinderDiedContext>> mOnBinderDiedContexts
GUARDED_BY(mLock);
- std::unordered_map<CallbackType, std::shared_ptr<SetValuesClient>> mSetValuesClients
+ std::unordered_map<const AIBinder*, std::shared_ptr<GetValuesClient>> mGetValuesClients
+ GUARDED_BY(mLock);
+ std::unordered_map<const AIBinder*, std::shared_ptr<SetValuesClient>> mSetValuesClients
GUARDED_BY(mLock);
// SubscriptionClients is thread-safe.
std::shared_ptr<SubscriptionClients> mSubscriptionClients;
+ // mLinkToDeathImpl is only going to be changed in test.
+ std::unique_ptr<ILinkToDeath> mLinkToDeathImpl;
+
// RecurrentTimer is thread-safe.
RecurrentTimer mRecurrentTimer;
+ ::ndk::ScopedAIBinder_DeathRecipient mDeathRecipient;
+
::android::base::Result<void> checkProperty(
const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue);
@@ -176,9 +209,15 @@
const ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig*>
getConfig(int32_t propId) const;
+ void onBinderDiedWithContext(const AIBinder* clientId);
+
+ void onBinderUnlinkedWithContext(const AIBinder* clientId);
+
+ void monitorBinderLifeCycle(const CallbackType& callback);
+
template <class T>
static std::shared_ptr<T> getOrCreateClient(
- std::unordered_map<CallbackType, std::shared_ptr<T>>* clients,
+ std::unordered_map<const AIBinder*, std::shared_ptr<T>>* clients,
const CallbackType& callback, std::shared_ptr<PendingRequestPool> pendingRequestPool);
static void getValueFromHardwareCallCallback(
@@ -195,9 +234,16 @@
static void checkHealth(std::weak_ptr<IVehicleHardware> hardware,
std::weak_ptr<SubscriptionManager> subscriptionManager);
+ static void onBinderDied(void* cookie);
+
+ static void onBinderUnlinked(void* cookie);
+
// Test-only
// Set the default timeout for pending requests.
void setTimeout(int64_t timeoutInNano);
+
+ // Test-only
+ void setLinkToDeathImpl(std::unique_ptr<ILinkToDeath> impl);
};
} // namespace vehicle
diff --git a/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h b/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
index 28809c6..e739c8c 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
@@ -38,6 +38,7 @@
// A thread-safe subscription manager that manages all VHAL subscriptions.
class SubscriptionManager final {
public:
+ using ClientIdType = const AIBinder*;
using CallbackType =
std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>;
using GetValueFunc = std::function<void(
@@ -59,24 +60,24 @@
options,
bool isContinuousProperty);
- // Unsubscribes from the properties for the callback.
- // Returns error if the callback was not subscribed before or one of the given property was not
+ // Unsubscribes from the properties for the client.
+ // Returns error if the client was not subscribed before or one of the given property was not
// subscribed. If error is returned, no property would be unsubscribed.
- // Returns ok if all the requested properties for the callback are unsubscribed.
- ::android::base::Result<void> unsubscribe(const CallbackType& callback,
+ // Returns ok if all the requested properties for the client are unsubscribed.
+ ::android::base::Result<void> unsubscribe(ClientIdType client,
const std::vector<int32_t>& propIds);
- // Unsubscribes to all the properties for the callback.
- // Returns error if the callback was not subscribed before. If error is returned, no property
+ // Unsubscribes from all the properties for the client.
+ // Returns error if the client was not subscribed before. If error is returned, no property
// would be unsubscribed.
- // Returns ok if all the properties for the callback are unsubscribed.
- ::android::base::Result<void> unsubscribe(const CallbackType& callback);
+ // Returns ok if all the properties for the client are unsubscribed.
+ ::android::base::Result<void> unsubscribe(ClientIdType client);
// For a list of updated properties, returns a map that maps clients subscribing to
// the updated properties to a list of updated values. This would only return on-change property
// clients that should be informed for the given updated values.
std::unordered_map<
- std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>,
+ CallbackType,
std::vector<const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue*>>
getSubscribedClients(
const std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>&
@@ -86,6 +87,9 @@
static bool checkSampleRate(float sampleRate);
private:
+ // Friend class for testing.
+ friend class DefaultVehicleHalTest;
+
struct PropIdAreaId {
int32_t propId;
int32_t areaId;
@@ -131,9 +135,10 @@
};
mutable std::mutex mLock;
- std::unordered_map<PropIdAreaId, std::unordered_set<CallbackType>, PropIdAreaIdHash>
+ std::unordered_map<PropIdAreaId, std::unordered_map<ClientIdType, CallbackType>,
+ PropIdAreaIdHash>
mClientsByPropIdArea GUARDED_BY(mLock);
- std::unordered_map<CallbackType, std::unordered_map<PropIdAreaId, std::unique_ptr<Subscription>,
+ std::unordered_map<ClientIdType, std::unordered_map<PropIdAreaId, std::unique_ptr<Subscription>,
PropIdAreaIdHash>>
mSubscriptionsByClient GUARDED_BY(mLock);
// RecurrentTimer is thread-safe.
@@ -141,6 +146,9 @@
const GetValueFunc mGetValue;
static ::android::base::Result<int64_t> getInterval(float sampleRate);
+
+ // Checks whether the manager is empty. For testing purpose.
+ bool isEmpty();
};
} // namespace vehicle
diff --git a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
index c4cbc68..3e088c5 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
@@ -42,7 +42,6 @@
using ::aidl::android::hardware::automotive::vehicle::GetValueRequests;
using ::aidl::android::hardware::automotive::vehicle::GetValueResult;
using ::aidl::android::hardware::automotive::vehicle::GetValueResults;
-using ::aidl::android::hardware::automotive::vehicle::IVehicleCallback;
using ::aidl::android::hardware::automotive::vehicle::SetValueRequest;
using ::aidl::android::hardware::automotive::vehicle::SetValueRequests;
using ::aidl::android::hardware::automotive::vehicle::SetValueResult;
@@ -62,6 +61,8 @@
using ::android::base::expected;
using ::android::base::Result;
using ::android::base::StringPrintf;
+
+using ::ndk::ScopedAIBinder_DeathRecipient;
using ::ndk::ScopedAStatus;
std::string toString(const std::unordered_set<int64_t>& values) {
@@ -86,10 +87,15 @@
int64_t DefaultVehicleHal::SubscribeIdByClient::getId(const CallbackType& callback) {
std::scoped_lock<std::mutex> lockGuard(mLock);
// This would be initialized to 0 if callback does not exist in the map.
- int64_t subscribeId = (mIds[callback])++;
+ int64_t subscribeId = (mIds[callback->asBinder().get()])++;
return subscribeId;
}
+void DefaultVehicleHal::SubscriptionClients::removeClient(const AIBinder* clientId) {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ mClients.erase(clientId);
+}
+
size_t DefaultVehicleHal::SubscriptionClients::countClients() {
std::scoped_lock<std::mutex> lockGuard(mLock);
return mClients.size();
@@ -139,6 +145,17 @@
std::make_shared<std::function<void()>>([hardwareCopy, subscriptionManagerCopy]() {
checkHealth(hardwareCopy, subscriptionManagerCopy);
}));
+
+ mLinkToDeathImpl = std::make_unique<AIBinderLinkToDeathImpl>();
+ mDeathRecipient = ScopedAIBinder_DeathRecipient(
+ AIBinder_DeathRecipient_new(&DefaultVehicleHal::onBinderDied));
+ AIBinder_DeathRecipient_setOnUnlinked(mDeathRecipient.get(),
+ &DefaultVehicleHal::onBinderUnlinked);
+}
+
+DefaultVehicleHal::~DefaultVehicleHal() {
+ // Delete the deathRecipient so that onBinderDied would not be called to reference 'this'.
+ mDeathRecipient = ScopedAIBinder_DeathRecipient();
}
void DefaultVehicleHal::onPropertyChangeEvent(
@@ -161,26 +178,73 @@
template <class T>
std::shared_ptr<T> DefaultVehicleHal::getOrCreateClient(
- std::unordered_map<CallbackType, std::shared_ptr<T>>* clients, const CallbackType& callback,
- std::shared_ptr<PendingRequestPool> pendingRequestPool) {
- if (clients->find(callback) == clients->end()) {
- // TODO(b/204943359): Remove client from clients when linkToDeath is implemented.
- (*clients)[callback] = std::make_shared<T>(pendingRequestPool, callback);
+ std::unordered_map<const AIBinder*, std::shared_ptr<T>>* clients,
+ const CallbackType& callback, std::shared_ptr<PendingRequestPool> pendingRequestPool) {
+ const AIBinder* clientId = callback->asBinder().get();
+ if (clients->find(clientId) == clients->end()) {
+ (*clients)[clientId] = std::make_shared<T>(pendingRequestPool, callback);
}
- return (*clients)[callback];
+ return (*clients)[clientId];
+}
+
+void DefaultVehicleHal::monitorBinderLifeCycle(const CallbackType& callback) {
+ AIBinder* clientId = callback->asBinder().get();
+ {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ if (mOnBinderDiedContexts.find(clientId) != mOnBinderDiedContexts.end()) {
+ // Already registered.
+ return;
+ }
+ }
+
+ std::unique_ptr<OnBinderDiedContext> context = std::make_unique<OnBinderDiedContext>(
+ OnBinderDiedContext{.vhal = this, .clientId = clientId});
+ binder_status_t status = mLinkToDeathImpl->linkToDeath(clientId, mDeathRecipient.get(),
+ static_cast<void*>(context.get()));
+ if (status == STATUS_OK) {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ // Insert into a map to keep the context object alive.
+ mOnBinderDiedContexts[clientId] = std::move(context);
+ } else {
+ ALOGE("failed to call linkToDeath on client binder, status: %d", static_cast<int>(status));
+ }
+}
+
+void DefaultVehicleHal::onBinderDied(void* cookie) {
+ OnBinderDiedContext* context = reinterpret_cast<OnBinderDiedContext*>(cookie);
+ context->vhal->onBinderDiedWithContext(context->clientId);
+}
+
+void DefaultVehicleHal::onBinderDiedWithContext(const AIBinder* clientId) {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ mSetValuesClients.erase(clientId);
+ mGetValuesClients.erase(clientId);
+ mSubscriptionClients->removeClient(clientId);
+ mSubscriptionManager->unsubscribe(clientId);
+}
+
+void DefaultVehicleHal::onBinderUnlinked(void* cookie) {
+ // Delete the context associated with this cookie.
+ OnBinderDiedContext* context = reinterpret_cast<OnBinderDiedContext*>(cookie);
+ context->vhal->onBinderUnlinkedWithContext(context->clientId);
+}
+
+void DefaultVehicleHal::onBinderUnlinkedWithContext(const AIBinder* clientId) {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ mOnBinderDiedContexts.erase(clientId);
}
template std::shared_ptr<DefaultVehicleHal::GetValuesClient>
DefaultVehicleHal::getOrCreateClient<DefaultVehicleHal::GetValuesClient>(
- std::unordered_map<CallbackType, std::shared_ptr<GetValuesClient>>* clients,
+ std::unordered_map<const AIBinder*, std::shared_ptr<GetValuesClient>>* clients,
const CallbackType& callback, std::shared_ptr<PendingRequestPool> pendingRequestPool);
template std::shared_ptr<DefaultVehicleHal::SetValuesClient>
DefaultVehicleHal::getOrCreateClient<DefaultVehicleHal::SetValuesClient>(
- std::unordered_map<CallbackType, std::shared_ptr<SetValuesClient>>* clients,
+ std::unordered_map<const AIBinder*, std::shared_ptr<SetValuesClient>>* clients,
const CallbackType& callback, std::shared_ptr<PendingRequestPool> pendingRequestPool);
template std::shared_ptr<SubscriptionClient>
DefaultVehicleHal::getOrCreateClient<SubscriptionClient>(
- std::unordered_map<CallbackType, std::shared_ptr<SubscriptionClient>>* clients,
+ std::unordered_map<const AIBinder*, std::shared_ptr<SubscriptionClient>>* clients,
const CallbackType& callback, std::shared_ptr<PendingRequestPool> pendingRequestPool);
void DefaultVehicleHal::getValueFromHardwareCallCallback(
@@ -268,6 +332,8 @@
ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,
const GetValueRequests& requests) {
+ monitorBinderLifeCycle(callback);
+
expected<LargeParcelableBase::BorrowedOwnedObject<GetValueRequests>, ScopedAStatus>
deserializedResults = fromStableLargeParcelable(requests);
if (!deserializedResults.ok()) {
@@ -344,6 +410,8 @@
ScopedAStatus DefaultVehicleHal::setValues(const CallbackType& callback,
const SetValueRequests& requests) {
+ monitorBinderLifeCycle(callback);
+
expected<LargeParcelableBase::BorrowedOwnedObject<SetValueRequests>, ScopedAStatus>
deserializedResults = fromStableLargeParcelable(requests);
if (!deserializedResults.ok()) {
@@ -526,6 +594,8 @@
ScopedAStatus DefaultVehicleHal::subscribe(const CallbackType& callback,
const std::vector<SubscribeOptions>& options,
[[maybe_unused]] int32_t maxSharedMemoryFileCount) {
+ monitorBinderLifeCycle(callback);
+
// TODO(b/205189110): Use shared memory file count.
if (auto result = checkSubscribeOptions(options); !result.ok()) {
ALOGE("subscribe: invalid subscribe options: %s", getErrorMsg(result).c_str());
@@ -571,7 +641,7 @@
ScopedAStatus DefaultVehicleHal::unsubscribe(const CallbackType& callback,
const std::vector<int32_t>& propIds) {
- return toScopedAStatus(mSubscriptionManager->unsubscribe(callback, propIds),
+ return toScopedAStatus(mSubscriptionManager->unsubscribe(callback->asBinder().get(), propIds),
StatusCode::INVALID_ARG);
}
@@ -639,6 +709,15 @@
return;
}
+binder_status_t DefaultVehicleHal::AIBinderLinkToDeathImpl::linkToDeath(
+ AIBinder* binder, AIBinder_DeathRecipient* recipient, void* cookie) {
+ return AIBinder_linkToDeath(binder, recipient, cookie);
+}
+
+void DefaultVehicleHal::setLinkToDeathImpl(std::unique_ptr<ILinkToDeath> impl) {
+ mLinkToDeathImpl = std::move(impl);
+}
+
} // namespace vehicle
} // namespace automotive
} // namespace hardware
diff --git a/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp b/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
index ff996fe..21bfba6 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
@@ -26,7 +26,7 @@
namespace {
-constexpr float ONE_SECOND_IN_NANO = 1000000000.;
+constexpr float ONE_SECOND_IN_NANO = 1'000'000'000.;
} // namespace
@@ -100,6 +100,7 @@
}
size_t intervalIndex = 0;
+ ClientIdType clientId = callback->asBinder().get();
for (const auto& option : options) {
int32_t propId = option.propId;
const std::vector<int32_t>& areaIds = option.areaIds;
@@ -118,7 +119,7 @@
.prop = propId,
.areaId = areaId,
};
- mSubscriptionsByClient[callback][propIdAreaId] =
+ mSubscriptionsByClient[clientId][propIdAreaId] =
std::make_unique<RecurrentSubscription>(
mTimer,
[this, callback, propValueRequest] {
@@ -126,24 +127,24 @@
},
interval);
} else {
- mSubscriptionsByClient[callback][propIdAreaId] =
+ mSubscriptionsByClient[clientId][propIdAreaId] =
std::make_unique<OnChangeSubscription>();
}
- mClientsByPropIdArea[propIdAreaId].insert(callback);
+ mClientsByPropIdArea[propIdAreaId][clientId] = callback;
}
}
return {};
}
-Result<void> SubscriptionManager::unsubscribe(const std::shared_ptr<IVehicleCallback>& callback,
+Result<void> SubscriptionManager::unsubscribe(SubscriptionManager::ClientIdType clientId,
const std::vector<int32_t>& propIds) {
std::scoped_lock<std::mutex> lockGuard(mLock);
- if (mSubscriptionsByClient.find(callback) == mSubscriptionsByClient.end()) {
+ if (mSubscriptionsByClient.find(clientId) == mSubscriptionsByClient.end()) {
return Error() << "No property was subscribed for the callback";
}
std::unordered_set<int32_t> subscribedPropIds;
- for (auto const& [propIdAreaId, _] : mSubscriptionsByClient[callback]) {
+ for (auto const& [propIdAreaId, _] : mSubscriptionsByClient[clientId]) {
subscribedPropIds.insert(propIdAreaId.propId);
}
@@ -153,13 +154,13 @@
}
}
- auto& subscriptions = mSubscriptionsByClient[callback];
+ auto& subscriptions = mSubscriptionsByClient[clientId];
auto it = subscriptions.begin();
while (it != subscriptions.end()) {
int32_t propId = it->first.propId;
if (std::find(propIds.begin(), propIds.end(), propId) != propIds.end()) {
auto& clients = mClientsByPropIdArea[it->first];
- clients.erase(callback);
+ clients.erase(clientId);
if (clients.empty()) {
mClientsByPropIdArea.erase(it->first);
}
@@ -169,27 +170,27 @@
}
}
if (subscriptions.empty()) {
- mSubscriptionsByClient.erase(callback);
+ mSubscriptionsByClient.erase(clientId);
}
return {};
}
-Result<void> SubscriptionManager::unsubscribe(const std::shared_ptr<IVehicleCallback>& callback) {
+Result<void> SubscriptionManager::unsubscribe(SubscriptionManager::ClientIdType clientId) {
std::scoped_lock<std::mutex> lockGuard(mLock);
- if (mSubscriptionsByClient.find(callback) == mSubscriptionsByClient.end()) {
- return Error() << "No property was subscribed for the callback";
+ if (mSubscriptionsByClient.find(clientId) == mSubscriptionsByClient.end()) {
+ return Error() << "No property was subscribed for this client";
}
- auto& subscriptions = mSubscriptionsByClient[callback];
+ auto& subscriptions = mSubscriptionsByClient[clientId];
for (auto const& [propIdAreaId, _] : subscriptions) {
auto& clients = mClientsByPropIdArea[propIdAreaId];
- clients.erase(callback);
+ clients.erase(clientId);
if (clients.empty()) {
mClientsByPropIdArea.erase(propIdAreaId);
}
}
- mSubscriptionsByClient.erase(callback);
+ mSubscriptionsByClient.erase(clientId);
return {};
}
@@ -207,8 +208,8 @@
if (mClientsByPropIdArea.find(propIdAreaId) == mClientsByPropIdArea.end()) {
continue;
}
- for (const auto& client : mClientsByPropIdArea[propIdAreaId]) {
- if (!mSubscriptionsByClient[client][propIdAreaId]->isOnChange()) {
+ for (const auto& [clientId, client] : mClientsByPropIdArea[propIdAreaId]) {
+ if (!mSubscriptionsByClient[clientId][propIdAreaId]->isOnChange()) {
continue;
}
clients[client].push_back(&value);
@@ -217,6 +218,11 @@
return clients;
}
+bool SubscriptionManager::isEmpty() {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ return mSubscriptionsByClient.empty() && mClientsByPropIdArea.empty();
+}
+
SubscriptionManager::RecurrentSubscription::RecurrentSubscription(
std::shared_ptr<RecurrentTimer> timer, std::function<void()>&& action, int64_t interval)
: mAction(std::make_shared<std::function<void()>>(action)), mTimer(timer) {
diff --git a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
index 54fc17d..ff355c3 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
@@ -73,6 +73,7 @@
using ::ndk::ScopedAStatus;
using ::ndk::ScopedFileDescriptor;
+using ::ndk::SpAIBinder;
using ::testing::Eq;
using ::testing::UnorderedElementsAre;
@@ -324,7 +325,12 @@
mVhal = ndk::SharedRefBase::make<DefaultVehicleHal>(std::move(hardware));
mVhalClient = IVehicle::fromBinder(mVhal->asBinder());
mCallback = ndk::SharedRefBase::make<MockVehicleCallback>();
- mCallbackClient = IVehicleCallback::fromBinder(mCallback->asBinder());
+ // Keep the local binder alive.
+ mBinder = mCallback->asBinder();
+ mCallbackClient = IVehicleCallback::fromBinder(mBinder);
+
+ // Set the linkToDeath to a fake implementation that always returns OK.
+ setTestLinkToDeathImpl();
}
void TearDown() override {
@@ -342,10 +348,36 @@
void setTimeout(int64_t timeoutInNano) { mVhal->setTimeout(timeoutInNano); }
+ void setTestLinkToDeathImpl() {
+ mVhal->setLinkToDeathImpl(std::make_unique<TestLinkToDeathImpl>());
+ }
+
size_t countPendingRequests() { return mVhal->mPendingRequestPool->countPendingRequests(); }
+ size_t countClients() {
+ std::scoped_lock<std::mutex> lockGuard(mVhal->mLock);
+ return mVhal->mGetValuesClients.size() + mVhal->mSetValuesClients.size() +
+ mVhal->mSubscriptionClients->countClients();
+ }
+
std::shared_ptr<PendingRequestPool> getPool() { return mVhal->mPendingRequestPool; }
+ void onBinderDied(void* cookie) { return mVhal->onBinderDied(cookie); }
+
+ void onBinderUnlinked(void* cookie) { return mVhal->onBinderUnlinked(cookie); }
+
+ void* getOnBinderDiedContexts(AIBinder* clientId) {
+ std::scoped_lock<std::mutex> lockGuard(mVhal->mLock);
+ return mVhal->mOnBinderDiedContexts[clientId].get();
+ }
+
+ bool countOnBinderDiedContexts() {
+ std::scoped_lock<std::mutex> lockGuard(mVhal->mLock);
+ return mVhal->mOnBinderDiedContexts.size();
+ }
+
+ bool hasNoSubscriptions() { return mVhal->mSubscriptionManager->isEmpty(); }
+
static Result<void> getValuesTestCases(size_t size, GetValueRequests& requests,
std::vector<GetValueResult>& expectedResults,
std::vector<GetValueRequest>& expectedHardwareRequests) {
@@ -416,18 +448,20 @@
return {};
}
- size_t countClients() {
- std::scoped_lock<std::mutex> lockGuard(mVhal->mLock);
- return mVhal->mGetValuesClients.size() + mVhal->mSetValuesClients.size() +
- mVhal->mSubscriptionClients->countClients();
- }
-
private:
std::shared_ptr<DefaultVehicleHal> mVhal;
std::shared_ptr<IVehicle> mVhalClient;
MockVehicleHardware* mHardwarePtr;
std::shared_ptr<MockVehicleCallback> mCallback;
std::shared_ptr<IVehicleCallback> mCallbackClient;
+ SpAIBinder mBinder;
+
+ class TestLinkToDeathImpl final : public DefaultVehicleHal::ILinkToDeath {
+ public:
+ binder_status_t linkToDeath(AIBinder*, AIBinder_DeathRecipient*, void*) override {
+ return STATUS_OK;
+ }
+ };
};
TEST_F(DefaultVehicleHalTest, testGetAllPropConfigsSmall) {
@@ -1445,6 +1479,71 @@
<< "expect to get the latest timestamp with the heartbeat event";
}
+TEST_F(DefaultVehicleHalTest, testOnBinderDiedUnlinked) {
+ // First subscribe to a continuous property so that we register a death recipient for our
+ // client.
+ VehiclePropValue testValue{
+ .prop = GLOBAL_CONTINUOUS_PROP,
+ .value.int32Values = {0},
+ };
+ // Set responses for all the hardware getValues requests.
+ getHardware()->setGetValueResponder(
+ [](std::shared_ptr<const IVehicleHardware::GetValuesCallback> callback,
+ const std::vector<GetValueRequest>& requests) {
+ std::vector<GetValueResult> results;
+ for (auto& request : requests) {
+ VehiclePropValue prop = request.prop;
+ prop.value.int32Values = {0};
+ results.push_back({
+ .requestId = request.requestId,
+ .status = StatusCode::OK,
+ .prop = prop,
+ });
+ }
+ (*callback)(results);
+ return StatusCode::OK;
+ });
+ std::vector<SubscribeOptions> options = {
+ {
+ .propId = GLOBAL_CONTINUOUS_PROP,
+ .sampleRate = 20.0,
+ },
+ };
+ auto status = getClient()->subscribe(getCallbackClient(), options, 0);
+ ASSERT_TRUE(status.isOk()) << "subscribe failed: " << status.getMessage();
+ // Sleep for 100ms so that the subscriptionClient gets created because we would at least try to
+ // get value once.
+ std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+ // Issue another getValue request on the same client.
+ GetValueRequests requests;
+ std::vector<GetValueResult> expectedResults;
+ std::vector<GetValueRequest> expectedHardwareRequests;
+ ASSERT_TRUE(getValuesTestCases(1, requests, expectedResults, expectedHardwareRequests).ok());
+ getHardware()->addGetValueResponses(expectedResults);
+ status = getClient()->getValues(getCallbackClient(), requests);
+ ASSERT_TRUE(status.isOk()) << "getValues failed: " << status.getMessage();
+
+ ASSERT_EQ(countOnBinderDiedContexts(), static_cast<size_t>(1))
+ << "expect one OnBinderDied context when one client is registered";
+
+ // Get the death recipient cookie for our callback that would be used in onBinderDied and
+ // onBinderUnlinked.
+ AIBinder* clientId = getCallbackClient()->asBinder().get();
+ void* context = getOnBinderDiedContexts(clientId);
+
+ onBinderDied(context);
+
+ ASSERT_EQ(countClients(), static_cast<size_t>(0))
+ << "expect all clients to be removed when binder died";
+ ASSERT_TRUE(hasNoSubscriptions()) << "expect no subscriptions when binder died";
+
+ onBinderUnlinked(context);
+
+ ASSERT_EQ(countOnBinderDiedContexts(), static_cast<size_t>(0))
+ << "expect OnBinderDied context to be deleted when binder is unlinked";
+}
+
} // namespace vehicle
} // namespace automotive
} // namespace hardware
diff --git a/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
index c14f4fd..f81b1a2 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
@@ -45,6 +45,7 @@
using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropValues;
using ::ndk::ScopedAStatus;
+using ::ndk::SpAIBinder;
using ::testing::ElementsAre;
using ::testing::WhenSorted;
@@ -95,7 +96,9 @@
0);
});
mCallback = ::ndk::SharedRefBase::make<PropertyCallback>();
- mCallbackClient = IVehicleCallback::fromBinder(mCallback->asBinder());
+ // Keep the local binder alive.
+ mBinder = mCallback->asBinder();
+ mCallbackClient = IVehicleCallback::fromBinder(mBinder);
}
SubscriptionManager* getManager() { return mManager.get(); }
@@ -112,6 +115,7 @@
std::unique_ptr<SubscriptionManager> mManager;
std::shared_ptr<PropertyCallback> mCallback;
std::shared_ptr<IVehicleCallback> mCallbackClient;
+ SpAIBinder mBinder;
};
TEST_F(SubscriptionManagerTest, testSubscribeGlobalContinuous) {
@@ -229,7 +233,7 @@
auto result = getManager()->subscribe(getCallbackClient(), options, true);
ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
- result = getManager()->unsubscribe(getCallbackClient());
+ result = getManager()->unsubscribe(getCallbackClient()->asBinder().get());
ASSERT_TRUE(result.ok()) << "failed to unsubscribe: " << result.error().message();
clearEvents();
@@ -257,7 +261,8 @@
auto result = getManager()->subscribe(getCallbackClient(), options, true);
ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
- result = getManager()->unsubscribe(getCallbackClient(), std::vector<int32_t>({0}));
+ result = getManager()->unsubscribe(getCallbackClient()->asBinder().get(),
+ std::vector<int32_t>({0}));
ASSERT_TRUE(result.ok()) << "failed to unsubscribe: " << result.error().message();
clearEvents();
@@ -289,7 +294,7 @@
auto result = getManager()->subscribe(getCallbackClient(), options, true);
ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
- result = getManager()->unsubscribe(getCallbackClient());
+ result = getManager()->unsubscribe(getCallbackClient()->asBinder().get());
ASSERT_TRUE(result.ok()) << "failed to unsubscribe: " << result.error().message();
clearEvents();
@@ -315,12 +320,14 @@
ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
// Property ID: 2 was not subscribed.
- result = getManager()->unsubscribe(getCallbackClient(), std::vector<int32_t>({0, 1, 2}));
+ result = getManager()->unsubscribe(getCallbackClient()->asBinder().get(),
+ std::vector<int32_t>({0, 1, 2}));
ASSERT_FALSE(result.ok()) << "unsubscribe an unsubscribed property must fail";
// Since property 0 and property 1 was not unsubscribed successfully, we should be able to
// unsubscribe them again.
- result = getManager()->unsubscribe(getCallbackClient(), std::vector<int32_t>({0, 1}));
+ result = getManager()->unsubscribe(getCallbackClient()->asBinder().get(),
+ std::vector<int32_t>({0, 1}));
ASSERT_TRUE(result.ok()) << "a failed unsubscription must not unsubscribe any properties"
<< result.error().message();
}
@@ -343,10 +350,10 @@
},
};
- std::shared_ptr<IVehicleCallback> client1 = IVehicleCallback::fromBinder(
- ::ndk::SharedRefBase::make<PropertyCallback>()->asBinder());
- std::shared_ptr<IVehicleCallback> client2 = IVehicleCallback::fromBinder(
- ::ndk::SharedRefBase::make<PropertyCallback>()->asBinder());
+ SpAIBinder binder1 = ::ndk::SharedRefBase::make<PropertyCallback>()->asBinder();
+ std::shared_ptr<IVehicleCallback> client1 = IVehicleCallback::fromBinder(binder1);
+ SpAIBinder binder2 = ::ndk::SharedRefBase::make<PropertyCallback>()->asBinder();
+ std::shared_ptr<IVehicleCallback> client2 = IVehicleCallback::fromBinder(binder2);
auto result = getManager()->subscribe(client1, options1, false);
ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
result = getManager()->subscribe(client2, options2, false);
@@ -447,7 +454,8 @@
auto result = getManager()->subscribe(getCallbackClient(), options, false);
ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
- result = getManager()->unsubscribe(getCallbackClient(), std::vector<int32_t>({0}));
+ result = getManager()->unsubscribe(getCallbackClient()->asBinder().get(),
+ std::vector<int32_t>({0}));
ASSERT_TRUE(result.ok()) << "failed to unsubscribe: " << result.error().message();
std::vector<VehiclePropValue> updatedValues = {
