Merge "Clean up resource correctly when client died." into tm-dev (cdd99897) · Commits · e / os / android_hardware_interfaces

automotive/vehicle/aidl/impl/utils/common/include/ConcurrentQueue.h

+2 −1

Original line number	Diff line number	Diff line
		@@ -33,12 +33,13 @@ namespace vehicle {
		template <typename T>
		class ConcurrentQueue {
		public:
		void waitForItems() {
		bool waitForItems() {
		std::unique_lock<std::mutex> lockGuard(mLock);
		android::base::ScopedLockAssertion lockAssertion(mLock);
		while (mQueue.empty() && mIsActive) {
		mCond.wait(lockGuard);
		}
		return mIsActive;
		}

		std::vector<T> flush() {

automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h

+37 −9

Original line number	Diff line number	Diff line
		@@ -22,6 +22,7 @@
		#include "PendingRequestPool.h"
		#include "SubscriptionManager.h"

		#include <ConcurrentQueue.h>
		#include <IVehicleHardware.h>
		#include <VehicleUtils.h>
		#include <aidl/android/hardware/automotive/vehicle/BnVehicle.h>
		@@ -31,6 +32,7 @@

		#include <memory>
		#include <mutex>
		#include <shared_mutex>
		#include <unordered_map>
		#include <vector>

		@@ -105,6 +107,8 @@ class DefaultVehicleHal final : public aidl::android::hardware::automotive::vehi
		public:
		SubscriptionClients(std::shared_ptr<PendingRequestPool> pool) : mPendingRequestPool(pool) {}

		std::shared_ptr<SubscriptionClient> maybeAddClient(const CallbackType& callback);

		std::shared_ptr<SubscriptionClient> getClient(const CallbackType& callback);

		void removeClient(const AIBinder* clientId);
		@@ -119,20 +123,24 @@ class DefaultVehicleHal final : public aidl::android::hardware::automotive::vehi
		std::shared_ptr<PendingRequestPool> mPendingRequestPool;
		};

		// A wrapper for linkToDeath to enable stubbing for test.
		class ILinkToDeath {
		// A wrapper for binder operations to enable stubbing for test.
		class IBinder {
		public:
		virtual ~ILinkToDeath() = default;
		virtual ~IBinder() = default;

		virtual binder_status_t linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
		void* cookie) = 0;

		virtual bool isAlive(const AIBinder* binder) = 0;
		};

		// A real implementation for ILinkToDeath.
		class AIBinderLinkToDeathImpl final : public ILinkToDeath {
		// A real implementation for IBinder.
		class AIBinderImpl final : public IBinder {
		public:
		binder_status_t linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
		void* cookie) override;

		bool isAlive(const AIBinder* binder) override;
		};

		// OnBinderDiedContext is a type used as a cookie passed deathRecipient. The deathRecipient's
		@@ -143,6 +151,13 @@ class DefaultVehicleHal final : public aidl::android::hardware::automotive::vehi
		const AIBinder* clientId;
		};

		// BinderDiedUnlinkedEvent represents either an onBinderDied or an onBinderUnlinked event.
		struct BinderDiedUnlinkedEvent {
		// true for onBinderDied, false for onBinderUnlinked.
		bool onBinderDied;
		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.
		@@ -171,14 +186,20 @@ class DefaultVehicleHal final : public aidl::android::hardware::automotive::vehi
		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;
		// mBinderImpl is only going to be changed in test.
		std::unique_ptr<IBinder> mBinderImpl;

		// RecurrentTimer is thread-safe.
		RecurrentTimer mRecurrentTimer;

		ndk::ScopedAIBinder_DeathRecipient mDeathRecipient;

		// ConcurrentQueue is thread-safe.
		ConcurrentQueue<BinderDiedUnlinkedEvent> mBinderEvents;

		// A thread to handle onBinderDied or onBinderUnlinked event.
		std::thread mOnBinderDiedUnlinkedHandlerThread;

		android::base::Result<void> checkProperty(
		const aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue);

		@@ -207,10 +228,17 @@ class DefaultVehicleHal final : public aidl::android::hardware::automotive::vehi

		void onBinderUnlinkedWithContext(const AIBinder* clientId);

		void monitorBinderLifeCycle(const CallbackType& callback);
		// Registers a onBinderDied callback for the client if not already registered.
		// Returns true if the client Binder is alive, false otherwise.
		bool monitorBinderLifeCycleLocked(const AIBinder* clientId) REQUIRES(mLock);

		bool checkDumpPermission();

		// The looping handler function to process all onBinderDied or onBinderUnlinked events in
		// mBinderEvents.
		void onBinderDiedUnlinkedHandler();

		// Gets or creates a {@code T} object for the client to or from {@code clients}.
		template <class T>
		static std::shared_ptr<T> getOrCreateClient(
		std::unordered_map<const AIBinder, std::shared_ptr<T>> clients,
		@@ -239,7 +267,7 @@ class DefaultVehicleHal final : public aidl::android::hardware::automotive::vehi
		void setTimeout(int64_t timeoutInNano);

		// Test-only
		void setLinkToDeathImpl(std::unique_ptr<ILinkToDeath> impl);
		void setBinderImpl(std::unique_ptr<IBinder> impl);
		};

		} // namespace vehicle

automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp

+12 −10

Original line number	Diff line number	Diff line
		@@ -67,9 +67,10 @@ void sendGetOrSetValueResult(std::shared_ptr<IVehicleCallback> callback, const R
		parcelableResults.payloads[0] = result;
		if (ScopedAStatus callbackStatus = callCallback(callback, parcelableResults);
		!callbackStatus.isOk()) {
		ALOGE("failed to call callback, error: %s, exception: %d, service specific error: %d",
		callbackStatus.getMessage(), callbackStatus.getExceptionCode(),
		callbackStatus.getServiceSpecificError());
		ALOGE("failed to call GetOrSetValueResult callback, client ID: %p, error: %s, "
		"exception: %d, service specific error: %d",
		callback->asBinder().get(), callbackStatus.getMessage(),
		callbackStatus.getExceptionCode(), callbackStatus.getServiceSpecificError());
		}
		}

		@@ -91,9 +92,10 @@ void sendGetOrSetValueResults(std::shared_ptr<IVehicleCallback> callback,
		if (status.isOk()) {
		if (ScopedAStatus callbackStatus = callCallback(callback, parcelableResults);
		!callbackStatus.isOk()) {
		ALOGE("failed to call callback, error: %s, exception: %d, service specific error: %d",
		callbackStatus.getMessage(), callbackStatus.getExceptionCode(),
		callbackStatus.getServiceSpecificError());
		ALOGE("failed to call GetOrSetValueResults callback, client ID: %p, error: %s, "
		"exception: %d, service specific error: %d",
		callback->asBinder().get(), callbackStatus.getMessage(),
		callbackStatus.getExceptionCode(), callbackStatus.getServiceSpecificError());
		}
		return;
		}
		@@ -299,10 +301,10 @@ void SubscriptionClient::sendUpdatedValues(std::shared_ptr<IVehicleCallback> cal
		if (ScopedAStatus callbackStatus =
		callback->onPropertyEvent(vehiclePropValues, sharedMemoryFileCount);
		!callbackStatus.isOk()) {
		ALOGE("subscribe: failed to call callback, error: %s, exception: %d, "
		"service specific error: %d",
		callbackStatus.getMessage(), callbackStatus.getExceptionCode(),
		callbackStatus.getServiceSpecificError());
		ALOGE("subscribe: failed to call UpdateValues callback, client ID: %p, error: %s, "
		"exception: %d, service specific error: %d",
		callback->asBinder().get(), callbackStatus.getMessage(),
		callbackStatus.getExceptionCode(), callbackStatus.getServiceSpecificError());
		}
		}

automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp

+104 −43

Original line number	Diff line number	Diff line
		@@ -80,12 +80,22 @@ std::string toString(const std::unordered_set<int64_t>& values) {

		} // namespace

		std::shared_ptr<SubscriptionClient> DefaultVehicleHal::SubscriptionClients::getClient(
		std::shared_ptr<SubscriptionClient> DefaultVehicleHal::SubscriptionClients::maybeAddClient(
		const CallbackType& callback) {
		std::scoped_lock<std::mutex> lockGuard(mLock);
		return getOrCreateClient(&mClients, callback, mPendingRequestPool);
		}

		std::shared_ptr<SubscriptionClient> DefaultVehicleHal::SubscriptionClients::getClient(
		const CallbackType& callback) {
		std::scoped_lock<std::mutex> lockGuard(mLock);
		const AIBinder* clientId = callback->asBinder().get();
		if (mClients.find(clientId) == mClients.end()) {
		return nullptr;
		}
		return mClients[clientId];
		}

		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.
		@@ -148,7 +158,8 @@ DefaultVehicleHal::DefaultVehicleHal(std::unique_ptr<IVehicleHardware> hardware)
		checkHealth(hardwareCopy, subscriptionManagerCopy);
		}));

		mLinkToDeathImpl = std::make_unique<AIBinderLinkToDeathImpl>();
		mBinderImpl = std::make_unique<AIBinderImpl>();
		mOnBinderDiedUnlinkedHandlerThread = std::thread([this] { onBinderDiedUnlinkedHandler(); });
		mDeathRecipient = ScopedAIBinder_DeathRecipient(
		AIBinder_DeathRecipient_new(&DefaultVehicleHal::onBinderDied));
		AIBinder_DeathRecipient_setOnUnlinked(mDeathRecipient.get(),
		@@ -158,6 +169,10 @@ DefaultVehicleHal::DefaultVehicleHal(std::unique_ptr<IVehicleHardware> hardware)
		DefaultVehicleHal::~DefaultVehicleHal() {
		// Delete the deathRecipient so that onBinderDied would not be called to reference 'this'.
		mDeathRecipient = ScopedAIBinder_DeathRecipient();
		mBinderEvents.deactivate();
		if (mOnBinderDiedUnlinkedHandlerThread.joinable()) {
		mOnBinderDiedUnlinkedHandlerThread.join();
		}
		}

		void DefaultVehicleHal::onPropertyChangeEvent(
		@@ -189,37 +204,41 @@ std::shared_ptr<T> DefaultVehicleHal::getOrCreateClient(
		return (*clients)[clientId];
		}

		void DefaultVehicleHal::monitorBinderLifeCycle(const CallbackType& callback) {
		AIBinder* clientId = callback->asBinder().get();
		{
		std::scoped_lock<std::mutex> lockGuard(mLock);
		bool DefaultVehicleHal::monitorBinderLifeCycleLocked(const AIBinder* clientId) {
		OnBinderDiedContext* contextPtr = nullptr;
		if (mOnBinderDiedContexts.find(clientId) != mOnBinderDiedContexts.end()) {
		// Already registered.
		return;
		}
		}

		return mBinderImpl->isAlive(clientId);
		} else {
		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);
		// We know context must be alive when we use contextPtr because context would only
		// be removed in OnBinderUnlinked, which must be called after OnBinderDied.
		contextPtr = context.get();
		// 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));
		}

		// If this function fails, onBinderUnlinked would be called to remove the added context.
		binder_status_t status = mBinderImpl->linkToDeath(
		const_cast<AIBinder>(clientId), mDeathRecipient.get(), static_cast<void>(contextPtr));
		if (status == STATUS_OK) {
		return true;
		}
		ALOGE("failed to call linkToDeath on client binder, client may already died, status: %d",
		static_cast<int>(status));
		return false;
		}

		void DefaultVehicleHal::onBinderDied(void* cookie) {
		OnBinderDiedContext* context = reinterpret_cast<OnBinderDiedContext*>(cookie);
		context->vhal->onBinderDiedWithContext(context->clientId);
		// To be handled in mOnBinderDiedUnlinkedHandlerThread. We cannot handle the event in the same
		// thread because we might be holding the mLock the handler requires.
		context->vhal->mBinderEvents.push(BinderDiedUnlinkedEvent{true, context->clientId});
		}

		void DefaultVehicleHal::onBinderDiedWithContext(const AIBinder* clientId) {
		ALOGD("binder died");
		std::scoped_lock<std::mutex> lockGuard(mLock);
		ALOGD("binder died, client ID: %p", clientId);
		mSetValuesClients.erase(clientId);
		mGetValuesClients.erase(clientId);
		mSubscriptionClients->removeClient(clientId);
		@@ -227,17 +246,31 @@ void DefaultVehicleHal::onBinderDiedWithContext(const AIBinder* clientId) {
		}

		void DefaultVehicleHal::onBinderUnlinked(void* cookie) {
		// Delete the context associated with this cookie.
		OnBinderDiedContext* context = reinterpret_cast<OnBinderDiedContext*>(cookie);
		context->vhal->onBinderUnlinkedWithContext(context->clientId);
		// To be handled in mOnBinderDiedUnlinkedHandlerThread. We cannot handle the event in the same
		// thread because we might be holding the mLock the handler requires.
		context->vhal->mBinderEvents.push(BinderDiedUnlinkedEvent{false, context->clientId});
		}

		void DefaultVehicleHal::onBinderUnlinkedWithContext(const AIBinder* clientId) {
		ALOGD("binder unlinked");
		std::scoped_lock<std::mutex> lockGuard(mLock);
		// Delete the context associated with this cookie.
		mOnBinderDiedContexts.erase(clientId);
		}

		void DefaultVehicleHal::onBinderDiedUnlinkedHandler() {
		while (mBinderEvents.waitForItems()) {
		for (BinderDiedUnlinkedEvent& event : mBinderEvents.flush()) {
		if (event.onBinderDied) {
		onBinderDiedWithContext(event.clientId);
		} else {
		onBinderUnlinkedWithContext(event.clientId);
		}
		}
		}
		}

		template std::shared_ptr<DefaultVehicleHal::GetValuesClient>
		DefaultVehicleHal::getOrCreateClient<DefaultVehicleHal::GetValuesClient>(
		std::unordered_map<const AIBinder, std::shared_ptr<GetValuesClient>> clients,
		@@ -258,6 +291,10 @@ void DefaultVehicleHal::getValueFromHardwareCallCallback(
		const VehiclePropValue& value) {
		int64_t subscribeId = subscribeIdByClient->getId(callback);
		auto client = subscriptionClients->getClient(callback);
		if (client == nullptr) {
		ALOGW("subscribe[%" PRId64 "]: the client has died", subscribeId);
		return;
		}
		if (auto addRequestResult = client->addRequests({subscribeId}); !addRequestResult.ok()) {
		ALOGE("subscribe[%" PRId64 "]: too many pending requests, ignore the getValue request",
		subscribeId);
		@@ -336,8 +373,6 @@ Result<void> DefaultVehicleHal::checkProperty(const VehiclePropValue& propValue)

		ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,
		const GetValueRequests& requests) {
		monitorBinderLifeCycle(callback);

		expected<LargeParcelableBase::BorrowedOwnedObject<GetValueRequests>, ScopedAStatus>
		deserializedResults = fromStableLargeParcelable(requests);
		if (!deserializedResults.ok()) {
		@@ -379,9 +414,16 @@ ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,

		std::shared_ptr<GetValuesClient> client;
		{
		std::scoped_lock<std::mutex> lockGuard(mLock);
		// Lock to make sure onBinderDied would not be called concurrently.
		std::scoped_lock lockGuard(mLock);
		if (!monitorBinderLifeCycleLocked(callback->asBinder().get())) {
		return ScopedAStatus::fromExceptionCodeWithMessage(EX_TRANSACTION_FAILED,
		"client died");
		}

		client = getOrCreateClient(&mGetValuesClients, callback, mPendingRequestPool);
		}

		// Register the pending hardware requests and also check for duplicate request Ids.
		if (auto addRequestResult = client->addRequests(hardwareRequestIds); !addRequestResult.ok()) {
		ALOGE("getValues[%s]: failed to add pending requests, error: %s",
		@@ -414,8 +456,6 @@ ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,

		ScopedAStatus DefaultVehicleHal::setValues(const CallbackType& callback,
		const SetValueRequests& requests) {
		monitorBinderLifeCycle(callback);

		expected<LargeParcelableBase::BorrowedOwnedObject<SetValueRequests>, ScopedAStatus>
		deserializedResults = fromStableLargeParcelable(requests);
		if (!deserializedResults.ok()) {
		@@ -467,7 +507,12 @@ ScopedAStatus DefaultVehicleHal::setValues(const CallbackType& callback,

		std::shared_ptr<SetValuesClient> client;
		{
		std::scoped_lock<std::mutex> lockGuard(mLock);
		// Lock to make sure onBinderDied would not be called concurrently.
		std::scoped_lock lockGuard(mLock);
		if (!monitorBinderLifeCycleLocked(callback->asBinder().get())) {
		return ScopedAStatus::fromExceptionCodeWithMessage(EX_TRANSACTION_FAILED,
		"client died");
		}
		client = getOrCreateClient(&mSetValuesClients, callback, mPendingRequestPool);
		}

		@@ -602,8 +647,6 @@ Result<void, VhalError> DefaultVehicleHal::checkSubscribeOptions(
		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());
		@@ -635,6 +678,18 @@ ScopedAStatus DefaultVehicleHal::subscribe(const CallbackType& callback,
		onChangeSubscriptions.push_back(std::move(optionCopy));
		}
		}

		{
		// Lock to make sure onBinderDied would not be called concurrently.
		std::scoped_lock lockGuard(mLock);
		if (!monitorBinderLifeCycleLocked(callback->asBinder().get())) {
		return ScopedAStatus::fromExceptionCodeWithMessage(EX_TRANSACTION_FAILED,
		"client died");
		}

		// Create a new SubscriptionClient if there isn't an existing one.
		mSubscriptionClients->maybeAddClient(callback);

		// Since we have already check the sample rates, the following functions must succeed.
		if (!onChangeSubscriptions.empty()) {
		mSubscriptionManager->subscribe(callback, onChangeSubscriptions,
		@@ -644,6 +699,7 @@ ScopedAStatus DefaultVehicleHal::subscribe(const CallbackType& callback,
		mSubscriptionManager->subscribe(callback, continuousSubscriptions,
		/isContinuousProperty=/true);
		}
		}
		return ScopedAStatus::ok();
		}

		@@ -719,13 +775,18 @@ void DefaultVehicleHal::checkHealth(std::weak_ptr<IVehicleHardware> hardware,
		return;
		}

		binder_status_t DefaultVehicleHal::AIBinderLinkToDeathImpl::linkToDeath(
		AIBinder* binder, AIBinder_DeathRecipient* recipient, void* cookie) {
		binder_status_t DefaultVehicleHal::AIBinderImpl::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);
		bool DefaultVehicleHal::AIBinderImpl::isAlive(const AIBinder* binder) {
		return AIBinder_isAlive(binder);
		}

		void DefaultVehicleHal::setBinderImpl(std::unique_ptr<IBinder> impl) {
		mBinderImpl = std::move(impl);
		}

		bool DefaultVehicleHal::checkDumpPermission() {

automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp

+87 −14

Original line number	Diff line number	Diff line
		@@ -332,7 +332,9 @@ class DefaultVehicleHalTest : public testing::Test {
		mCallbackClient = IVehicleCallback::fromBinder(mBinder);

		// Set the linkToDeath to a fake implementation that always returns OK.
		setTestLinkToDeathImpl();
		auto binderImpl = std::make_unique<TestBinderImpl>();
		mBinderImpl = binderImpl.get();
		mVhal->setBinderImpl(std::move(binderImpl));
		}

		void TearDown() override {
		@@ -350,10 +352,6 @@ class DefaultVehicleHalTest : public testing::Test {

		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() {
		@@ -380,6 +378,8 @@ class DefaultVehicleHalTest : public testing::Test {

		bool hasNoSubscriptions() { return mVhal->mSubscriptionManager->isEmpty(); }

		void setBinderAlive(bool isAlive) { mBinderImpl->setAlive(isAlive); };

		static Result<void> getValuesTestCases(size_t size, GetValueRequests& requests,
		std::vector<GetValueResult>& expectedResults,
		std::vector<GetValueRequest>& expectedHardwareRequests) {
		@@ -452,19 +452,31 @@ class DefaultVehicleHalTest : public testing::Test {
		}

		private:
		class TestBinderImpl final : public DefaultVehicleHal::IBinder {
		public:
		binder_status_t linkToDeath(AIBinder, AIBinder_DeathRecipient, void*) override {
		if (mIsAlive) {
		return STATUS_OK;
		} else {
		return STATUS_FAILED_TRANSACTION;
		}
		}

		bool isAlive(const AIBinder*) override { return mIsAlive; }

		void setAlive(bool isAlive) { mIsAlive = isAlive; }

		private:
		bool mIsAlive = true;
		};

		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;
		}
		};
		TestBinderImpl* mBinderImpl;
		};

		TEST_F(DefaultVehicleHalTest, testGetAllPropConfigsSmall) {
		@@ -587,7 +599,6 @@ TEST_F(DefaultVehicleHalTest, testGetValuesLarge) {

		ASSERT_TRUE(getValuesTestCases(5000, requests, expectedResults, expectedHardwareRequests).ok())
		<< "requests to hardware mismatch";
		;

		getHardware()->addGetValueResponses(expectedResults);

		@@ -806,6 +817,51 @@ TEST_F(DefaultVehicleHalTest, testGetValuesDuplicateRequestProps) {
		ASSERT_FALSE(status.isOk()) << "duplicate request properties in one request must fail";
		}

		TEST_F(DefaultVehicleHalTest, testGetValuesNewClientDied) {
		GetValueRequests requests;
		std::vector<GetValueResult> expectedResults;
		std::vector<GetValueRequest> expectedHardwareRequests;

		ASSERT_TRUE(getValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());

		getHardware()->addGetValueResponses(expectedResults);

		setBinderAlive(false);

		auto status = getClient()->getValues(getCallbackClient(), requests);

		ASSERT_FALSE(status.isOk()) << "getValues must fail if client died";
		ASSERT_EQ(status.getExceptionCode(), EX_TRANSACTION_FAILED);
		EXPECT_EQ(countClients(), static_cast<size_t>(0))
		<< "No client should be created if the client binder died";
		}

		TEST_F(DefaultVehicleHalTest, testGetValuesExistingClientDied) {
		GetValueRequests requests;
		std::vector<GetValueResult> expectedResults;
		std::vector<GetValueRequest> expectedHardwareRequests;

		ASSERT_TRUE(getValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());

		getHardware()->addGetValueResponses(expectedResults);

		// Try a normal getValue request to cache a GetValueClient first.
		auto status = getClient()->getValues(getCallbackClient(), requests);

		ASSERT_TRUE(status.isOk()) << "getValues failed: " << status.getMessage();
		EXPECT_EQ(countClients(), static_cast<size_t>(1));

		// The client binder died before onBinderUnlinked clean up the GetValueClient.
		setBinderAlive(false);

		status = getClient()->getValues(getCallbackClient(), requests);

		ASSERT_FALSE(status.isOk()) << "getValues must fail if client died";
		ASSERT_EQ(status.getExceptionCode(), EX_TRANSACTION_FAILED);
		// The client count should still be 1 but onBinderUnlinked will remove this later.
		EXPECT_EQ(countClients(), static_cast<size_t>(1));
		}

		TEST_F(DefaultVehicleHalTest, testSetValuesSmall) {
		SetValueRequests requests;
		std::vector<SetValueResult> expectedResults;
		@@ -1111,7 +1167,8 @@ TEST_F(DefaultVehicleHalTest, testSubscribeGlobalOnChangeNormal) {
		<< "results mismatch, expect on change event for the updated value";
		ASSERT_FALSE(getCallback()->nextOnPropertyEventResults().has_value())
		<< "more results than expected";
		EXPECT_EQ(countClients(), static_cast<size_t>(1));
		EXPECT_EQ(countClients(), static_cast<size_t>(2))
		<< "expect 2 clients, 1 subscribe client and 1 setvalue client";
		}

		TEST_F(DefaultVehicleHalTest, testSubscribeGlobalOnchangeUnrelatedEventIgnored) {
		@@ -1581,12 +1638,28 @@ TEST_F(DefaultVehicleHalTest, testOnBinderDiedUnlinked) {

		onBinderDied(context);

		// Sleep for 100ms between checks.
		int64_t sleep = 100;
		// Timeout: 10s.
		int64_t timeout = 10'000'000'000;
		int64_t stopTime = elapsedRealtimeNano() + timeout;
		// Wait until the onBinderDied event is handled.
		while (countClients() != 0u && elapsedRealtimeNano() <= stopTime) {
		std::this_thread::sleep_for(std::chrono::milliseconds(sleep));
		}

		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);

		stopTime = elapsedRealtimeNano() + timeout;
		// Wait until the onBinderUnlinked event is handled.
		while (countOnBinderDiedContexts() != 0u && elapsedRealtimeNano() <= stopTime) {
		std::this_thread::sleep_for(std::chrono::milliseconds(sleep));
		}

		ASSERT_EQ(countOnBinderDiedContexts(), static_cast<size_t>(0))
		<< "expect OnBinderDied context to be deleted when binder is unlinked";
		}