Clean up resource correctly when client died. (f46e1df1) · Commits · e / os / android_hardware_interfaces

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

+2 −1

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

	std::vector<T> flush() {		std::vector<T> flush() {

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

+37 −9

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

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

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

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

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

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

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

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

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

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

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

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

	// OnBinderDiedContext is a type used as a cookie passed deathRecipient. The deathRecipient's		// 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;		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.		// 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		// TODO(b/214605968): define TIMEOUT_IN_NANO in IVehicle and allow getValues/setValues/subscribe
	// to specify custom timeouts.		// to specify custom timeouts.
	@@ -171,14 +186,20 @@ class DefaultVehicleHal final : public aidl::android::hardware::automotive::vehi
	GUARDED_BY(mLock);		GUARDED_BY(mLock);
	// SubscriptionClients is thread-safe.		// SubscriptionClients is thread-safe.
	std::shared_ptr<SubscriptionClients> mSubscriptionClients;		std::shared_ptr<SubscriptionClients> mSubscriptionClients;
	// mLinkToDeathImpl is only going to be changed in test.		// mBinderImpl is only going to be changed in test.
	std::unique_ptr<ILinkToDeath> mLinkToDeathImpl;		std::unique_ptr<IBinder> mBinderImpl;

	// RecurrentTimer is thread-safe.		// RecurrentTimer is thread-safe.
	RecurrentTimer mRecurrentTimer;		RecurrentTimer mRecurrentTimer;

	ndk::ScopedAIBinder_DeathRecipient mDeathRecipient;		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(		android::base::Result<void> checkProperty(
	const aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue);		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 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();		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>		template <class T>
	static std::shared_ptr<T> getOrCreateClient(		static std::shared_ptr<T> getOrCreateClient(
	std::unordered_map<const AIBinder, std::shared_ptr<T>> clients,		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);		void setTimeout(int64_t timeoutInNano);

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

	} // namespace vehicle		} // namespace vehicle

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

+12 −10

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

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

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

+104 −43

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

	} // namespace		} // namespace

	std::shared_ptr<SubscriptionClient> DefaultVehicleHal::SubscriptionClients::getClient(		std::shared_ptr<SubscriptionClient> DefaultVehicleHal::SubscriptionClients::maybeAddClient(
	const CallbackType& callback) {		const CallbackType& callback) {
	std::scoped_lock<std::mutex> lockGuard(mLock);		std::scoped_lock<std::mutex> lockGuard(mLock);
	return getOrCreateClient(&mClients, callback, mPendingRequestPool);		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) {		int64_t DefaultVehicleHal::SubscribeIdByClient::getId(const CallbackType& callback) {
	std::scoped_lock<std::mutex> lockGuard(mLock);		std::scoped_lock<std::mutex> lockGuard(mLock);
	// This would be initialized to 0 if callback does not exist in the map.		// 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);		checkHealth(hardwareCopy, subscriptionManagerCopy);
	}));		}));

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

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

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

	std::unique_ptr<OnBinderDiedContext> context = std::make_unique<OnBinderDiedContext>(		std::unique_ptr<OnBinderDiedContext> context = std::make_unique<OnBinderDiedContext>(
	OnBinderDiedContext{.vhal = this, .clientId = clientId});		OnBinderDiedContext{.vhal = this, .clientId = clientId});
	binder_status_t status = mLinkToDeathImpl->linkToDeath(clientId, mDeathRecipient.get(),		// We know context must be alive when we use contextPtr because context would only
	static_cast<void*>(context.get()));		// be removed in OnBinderUnlinked, which must be called after OnBinderDied.
	if (status == STATUS_OK) {		contextPtr = context.get();
	std::scoped_lock<std::mutex> lockGuard(mLock);
	// Insert into a map to keep the context object alive.		// Insert into a map to keep the context object alive.
	mOnBinderDiedContexts[clientId] = std::move(context);		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) {		void DefaultVehicleHal::onBinderDied(void* cookie) {
	OnBinderDiedContext* context = reinterpret_cast<OnBinderDiedContext*>(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) {		void DefaultVehicleHal::onBinderDiedWithContext(const AIBinder* clientId) {
	ALOGD("binder died");
	std::scoped_lock<std::mutex> lockGuard(mLock);		std::scoped_lock<std::mutex> lockGuard(mLock);
			ALOGD("binder died, client ID: %p", clientId);
	mSetValuesClients.erase(clientId);		mSetValuesClients.erase(clientId);
	mGetValuesClients.erase(clientId);		mGetValuesClients.erase(clientId);
	mSubscriptionClients->removeClient(clientId);		mSubscriptionClients->removeClient(clientId);
	@@ -227,17 +246,31 @@ void DefaultVehicleHal::onBinderDiedWithContext(const AIBinder* clientId) {
	}		}

	void DefaultVehicleHal::onBinderUnlinked(void* cookie) {		void DefaultVehicleHal::onBinderUnlinked(void* cookie) {
	// Delete the context associated with this cookie.
	OnBinderDiedContext* context = reinterpret_cast<OnBinderDiedContext*>(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) {		void DefaultVehicleHal::onBinderUnlinkedWithContext(const AIBinder* clientId) {
	ALOGD("binder unlinked");		ALOGD("binder unlinked");
	std::scoped_lock<std::mutex> lockGuard(mLock);		std::scoped_lock<std::mutex> lockGuard(mLock);
			// Delete the context associated with this cookie.
	mOnBinderDiedContexts.erase(clientId);		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>		template std::shared_ptr<DefaultVehicleHal::GetValuesClient>
	DefaultVehicleHal::getOrCreateClient<DefaultVehicleHal::GetValuesClient>(		DefaultVehicleHal::getOrCreateClient<DefaultVehicleHal::GetValuesClient>(
	std::unordered_map<const AIBinder, std::shared_ptr<GetValuesClient>> clients,		std::unordered_map<const AIBinder, std::shared_ptr<GetValuesClient>> clients,
	@@ -258,6 +291,10 @@ void DefaultVehicleHal::getValueFromHardwareCallCallback(
	const VehiclePropValue& value) {		const VehiclePropValue& value) {
	int64_t subscribeId = subscribeIdByClient->getId(callback);		int64_t subscribeId = subscribeIdByClient->getId(callback);
	auto client = subscriptionClients->getClient(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()) {		if (auto addRequestResult = client->addRequests({subscribeId}); !addRequestResult.ok()) {
	ALOGE("subscribe[%" PRId64 "]: too many pending requests, ignore the getValue request",		ALOGE("subscribe[%" PRId64 "]: too many pending requests, ignore the getValue request",
	subscribeId);		subscribeId);
	@@ -336,8 +373,6 @@ Result<void> DefaultVehicleHal::checkProperty(const VehiclePropValue& propValue)

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

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

	std::shared_ptr<GetValuesClient> client;		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);		client = getOrCreateClient(&mGetValuesClients, callback, mPendingRequestPool);
	}		}

	// Register the pending hardware requests and also check for duplicate request Ids.		// Register the pending hardware requests and also check for duplicate request Ids.
	if (auto addRequestResult = client->addRequests(hardwareRequestIds); !addRequestResult.ok()) {		if (auto addRequestResult = client->addRequests(hardwareRequestIds); !addRequestResult.ok()) {
	ALOGE("getValues[%s]: failed to add pending requests, error: %s",		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,		ScopedAStatus DefaultVehicleHal::setValues(const CallbackType& callback,
	const SetValueRequests& requests) {		const SetValueRequests& requests) {
	monitorBinderLifeCycle(callback);

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

	std::shared_ptr<SetValuesClient> client;		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);		client = getOrCreateClient(&mSetValuesClients, callback, mPendingRequestPool);
	}		}

	@@ -602,8 +647,6 @@ Result<void, VhalError> DefaultVehicleHal::checkSubscribeOptions(
	ScopedAStatus DefaultVehicleHal::subscribe(const CallbackType& callback,		ScopedAStatus DefaultVehicleHal::subscribe(const CallbackType& callback,
	const std::vector<SubscribeOptions>& options,		const std::vector<SubscribeOptions>& options,
	[[maybe_unused]] int32_t maxSharedMemoryFileCount) {		[[maybe_unused]] int32_t maxSharedMemoryFileCount) {
	monitorBinderLifeCycle(callback);

	// TODO(b/205189110): Use shared memory file count.		// TODO(b/205189110): Use shared memory file count.
	if (auto result = checkSubscribeOptions(options); !result.ok()) {		if (auto result = checkSubscribeOptions(options); !result.ok()) {
	ALOGE("subscribe: invalid subscribe options: %s", getErrorMsg(result).c_str());		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));		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.		// Since we have already check the sample rates, the following functions must succeed.
	if (!onChangeSubscriptions.empty()) {		if (!onChangeSubscriptions.empty()) {
	mSubscriptionManager->subscribe(callback, onChangeSubscriptions,		mSubscriptionManager->subscribe(callback, onChangeSubscriptions,
	@@ -644,6 +699,7 @@ ScopedAStatus DefaultVehicleHal::subscribe(const CallbackType& callback,
	mSubscriptionManager->subscribe(callback, continuousSubscriptions,		mSubscriptionManager->subscribe(callback, continuousSubscriptions,
	/isContinuousProperty=/true);		/isContinuousProperty=/true);
	}		}
			}
	return ScopedAStatus::ok();		return ScopedAStatus::ok();
	}		}

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

	binder_status_t DefaultVehicleHal::AIBinderLinkToDeathImpl::linkToDeath(		binder_status_t DefaultVehicleHal::AIBinderImpl::linkToDeath(AIBinder* binder,
	AIBinder* binder, AIBinder_DeathRecipient* recipient, void* cookie) {		AIBinder_DeathRecipient* recipient,
			void* cookie) {
	return AIBinder_linkToDeath(binder, recipient, cookie);		return AIBinder_linkToDeath(binder, recipient, cookie);
	}		}

	void DefaultVehicleHal::setLinkToDeathImpl(std::unique_ptr<ILinkToDeath> impl) {		bool DefaultVehicleHal::AIBinderImpl::isAlive(const AIBinder* binder) {
	mLinkToDeathImpl = std::move(impl);		return AIBinder_isAlive(binder);
			}

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

	bool DefaultVehicleHal::checkDumpPermission() {		bool DefaultVehicleHal::checkDumpPermission() {

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

+87 −14

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

	// Set the linkToDeath to a fake implementation that always returns OK.		// 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 {		void TearDown() override {
	@@ -350,10 +352,6 @@ class DefaultVehicleHalTest : public testing::Test {

	void setTimeout(int64_t timeoutInNano) { mVhal->setTimeout(timeoutInNano); }		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 countPendingRequests() { return mVhal->mPendingRequestPool->countPendingRequests(); }

	size_t countClients() {		size_t countClients() {
	@@ -380,6 +378,8 @@ class DefaultVehicleHalTest : public testing::Test {

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

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

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

	private:		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<DefaultVehicleHal> mVhal;
	std::shared_ptr<IVehicle> mVhalClient;		std::shared_ptr<IVehicle> mVhalClient;
	MockVehicleHardware* mHardwarePtr;		MockVehicleHardware* mHardwarePtr;
	std::shared_ptr<MockVehicleCallback> mCallback;		std::shared_ptr<MockVehicleCallback> mCallback;
	std::shared_ptr<IVehicleCallback> mCallbackClient;		std::shared_ptr<IVehicleCallback> mCallbackClient;
	SpAIBinder mBinder;		SpAIBinder mBinder;
			TestBinderImpl* mBinderImpl;
	class TestLinkToDeathImpl final : public DefaultVehicleHal::ILinkToDeath {
	public:
	binder_status_t linkToDeath(AIBinder, AIBinder_DeathRecipient, void*) override {
	return STATUS_OK;
	}
	};
	};		};

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

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

	getHardware()->addGetValueResponses(expectedResults);		getHardware()->addGetValueResponses(expectedResults);

	@@ -806,6 +817,51 @@ TEST_F(DefaultVehicleHalTest, testGetValuesDuplicateRequestProps) {
	ASSERT_FALSE(status.isOk()) << "duplicate request properties in one request must fail";		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) {		TEST_F(DefaultVehicleHalTest, testSetValuesSmall) {
	SetValueRequests requests;		SetValueRequests requests;
	std::vector<SetValueResult> expectedResults;		std::vector<SetValueResult> expectedResults;
	@@ -1111,7 +1167,8 @@ TEST_F(DefaultVehicleHalTest, testSubscribeGlobalOnChangeNormal) {
	<< "results mismatch, expect on change event for the updated value";		<< "results mismatch, expect on change event for the updated value";
	ASSERT_FALSE(getCallback()->nextOnPropertyEventResults().has_value())		ASSERT_FALSE(getCallback()->nextOnPropertyEventResults().has_value())
	<< "more results than expected";		<< "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) {		TEST_F(DefaultVehicleHalTest, testSubscribeGlobalOnchangeUnrelatedEventIgnored) {
	@@ -1581,12 +1638,28 @@ TEST_F(DefaultVehicleHalTest, testOnBinderDiedUnlinked) {

	onBinderDied(context);		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))		ASSERT_EQ(countClients(), static_cast<size_t>(0))
	<< "expect all clients to be removed when binder died";		<< "expect all clients to be removed when binder died";
	ASSERT_TRUE(hasNoSubscriptions()) << "expect no subscriptions when binder died";		ASSERT_TRUE(hasNoSubscriptions()) << "expect no subscriptions when binder died";

	onBinderUnlinked(context);		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))		ASSERT_EQ(countOnBinderDiedContexts(), static_cast<size_t>(0))
	<< "expect OnBinderDied context to be deleted when binder is unlinked";		<< "expect OnBinderDied context to be deleted when binder is unlinked";
	}		}