Merge changes from topic "b123318663"
* changes:
Update LazyServiceRegistrar for tryUnregister.
Fix race conditions for lazy services.
diff --git a/transport/HidlLazyUtils.cpp b/transport/HidlLazyUtils.cpp
index 9223af6..ed40416 100644
--- a/transport/HidlLazyUtils.cpp
+++ b/transport/HidlLazyUtils.cpp
@@ -26,26 +26,42 @@
namespace hardware {
namespace details {
+using ::android::hidl::base::V1_0::IBase;
+
class ClientCounterCallback : public ::android::hidl::manager::V1_2::IClientCallback {
public:
- ClientCounterCallback() : mNumConnectedServices(0), mNumRegisteredServices(0) {}
+ ClientCounterCallback() : mNumConnectedServices(0) {}
- void incServiceCounter();
+ bool addRegisteredService(const sp<IBase>& service, const std::string& name);
protected:
- Return<void> onClients(const sp<::android::hidl::base::V1_0::IBase>& service,
- bool clients) override;
+ Return<void> onClients(const sp<IBase>& service, bool clients) override;
private:
/**
+ * Registers or re-registers services. Returns whether successful.
+ */
+ bool registerService(const sp<IBase>& service, const std::string& name);
+
+ /**
+ * Unregisters all services that we can. If we can't unregister all, re-register other
+ * services.
+ */
+ void tryShutdown();
+
+ /**
* Counter of the number of services that currently have at least one client.
*/
size_t mNumConnectedServices;
+ struct Service {
+ sp<IBase> service;
+ std::string name;
+ };
/**
* Number of services that have been registered.
*/
- size_t mNumRegisteredServices;
+ std::vector<Service> mRegisteredServices;
};
class LazyServiceRegistrarImpl {
@@ -59,8 +75,37 @@
sp<ClientCounterCallback> mClientCallback;
};
-void ClientCounterCallback::incServiceCounter() {
- mNumRegisteredServices++;
+bool ClientCounterCallback::addRegisteredService(const sp<IBase>& service,
+ const std::string& name) {
+ bool success = registerService(service, name);
+
+ if (success) {
+ mRegisteredServices.push_back({service, name});
+ }
+
+ return success;
+}
+
+bool ClientCounterCallback::registerService(const sp<IBase>& service, const std::string& name) {
+ auto manager = hardware::defaultServiceManager1_2();
+
+ const std::string descriptor = getDescriptor(service.get());
+
+ LOG(INFO) << "Registering HAL: " << descriptor << " with name: " << name;
+
+ status_t res = android::hardware::details::registerAsServiceInternal(service, name);
+ if (res != android::OK) {
+ LOG(ERROR) << "Failed to register as service.";
+ return false;
+ }
+
+ bool ret = manager->registerClientCallback(getDescriptor(service.get()), name, service, this);
+ if (!ret) {
+ LOG(ERROR) << "Failed to add client callback.";
+ return false;
+ }
+
+ return true;
}
/**
@@ -70,38 +115,65 @@
Return<void> ClientCounterCallback::onClients(const sp<::android::hidl::base::V1_0::IBase>& service,
bool clients) {
if (clients) {
- LOG(INFO) << "HAL " << service->descriptor << " connected.";
mNumConnectedServices++;
} else {
- LOG(INFO) << "HAL " << service->descriptor << " disconnected.";
mNumConnectedServices--;
}
- LOG(INFO) << "HAL has " << mNumConnectedServices << " (of " << mNumRegisteredServices
- << " available) clients in use.";
+
+ LOG(INFO) << "Process has " << mNumConnectedServices << " (of " << mRegisteredServices.size()
+ << " available) clients in use after notification " << getDescriptor(service.get())
+ << " has clients: " << clients;
if (mNumConnectedServices == 0) {
- LOG(INFO) << "Exiting HAL. No clients in use for any service in process.";
+ tryShutdown();
+ }
+
+ return Status::ok();
+}
+
+void ClientCounterCallback::tryShutdown() {
+ LOG(INFO) << "Trying to exit HAL. No clients in use for any service in process.";
+
+ auto manager = hardware::defaultServiceManager1_2();
+
+ auto unRegisterIt = mRegisteredServices.begin();
+ for (; unRegisterIt != mRegisteredServices.end(); ++unRegisterIt) {
+ auto& entry = (*unRegisterIt);
+
+ const std::string descriptor = getDescriptor(entry.service.get());
+ bool success = manager->tryUnregister(descriptor, entry.name, entry.service);
+
+ if (!success) {
+ LOG(INFO) << "Failed to unregister HAL " << descriptor << "/" << entry.name
+ << ". Going to re-register remaining instances.";
+ break;
+ }
+ }
+
+ if (unRegisterIt == mRegisteredServices.end()) {
+ LOG(INFO) << "Unregistered all clients and exiting";
exit(EXIT_SUCCESS);
}
- return Status::ok();
+
+ for (auto reRegisterIt = mRegisteredServices.begin(); reRegisterIt != unRegisterIt;
+ reRegisterIt++) {
+ auto& entry = (*reRegisterIt);
+
+ // re-register entry
+ if (!registerService(entry.service, entry.name)) {
+ // Must restart. Otherwise, clients will never be able to get ahold of this service.
+ LOG(FATAL) << "Bad state: could not re-register " << getDescriptor(entry.service.get());
+ }
+ }
}
status_t LazyServiceRegistrarImpl::registerService(
const sp<::android::hidl::base::V1_0::IBase>& service, const std::string& name) {
- static auto manager = hardware::defaultServiceManager1_2();
- LOG(INFO) << "Registering HAL: " << service->descriptor << " with name: " << name;
- status_t res = android::hardware::details::registerAsServiceInternal(service, name);
- if (res == android::OK) {
- mClientCallback->incServiceCounter();
- bool ret = manager->registerClientCallback(service, mClientCallback);
- if (!ret) {
- res = android::INVALID_OPERATION;
- LOG(ERROR) << "Failed to add client callback";
- }
- } else {
- LOG(ERROR) << "Failed to register as service";
+ if (!mClientCallback->addRegisteredService(service, name)) {
+ return ::android::UNKNOWN_ERROR;
}
- return res;
+
+ return ::android::OK;
}
} // namespace details
diff --git a/transport/HidlTransportUtils.cpp b/transport/HidlTransportUtils.cpp
index 4e952eb..8c61281 100644
--- a/transport/HidlTransportUtils.cpp
+++ b/transport/HidlTransportUtils.cpp
@@ -56,6 +56,10 @@
}
std::string getDescriptor(IBase* interface) {
+ if (interface == nullptr) {
+ return "";
+ }
+
std::string myDescriptor{};
auto ret = interface->interfaceDescriptor([&](const hidl_string &types) {
myDescriptor = types.c_str();
diff --git a/transport/manager/1.2/IClientCallback.hal b/transport/manager/1.2/IClientCallback.hal
index 1189e44..8ebb044 100644
--- a/transport/manager/1.2/IClientCallback.hal
+++ b/transport/manager/1.2/IClientCallback.hal
@@ -21,6 +21,10 @@
* This is called when there is a transition between having >= 1 clients and having 0 clients
* (or vice versa).
*
+ * Upon receiving hasClients false, if the process decides to exit, it is recommended to try to
+ * unregister using @1.2::IServiceManager's tryUnregister before quiting in case another client
+ * associates.
+ *
* @param registered binder 'server' registered with IServiceManager's registerClientCallback
* @param hasClients whether there are currently clients
* true - when there are >= 1 clients. This must be called as soon as IServiceManager::get
diff --git a/transport/manager/1.2/IServiceManager.hal b/transport/manager/1.2/IServiceManager.hal
index e1629d6..d79403d 100644
--- a/transport/manager/1.2/IServiceManager.hal
+++ b/transport/manager/1.2/IServiceManager.hal
@@ -27,7 +27,9 @@
* If the service has clients at the time of registration, the callback is called with
* hasClients true. After that, it is called based on the changes in clientele.
*
- * @param server non-null service waiting to have no clients
+ * @param fqName Fully-qualified interface name (used to register)
+ * @param name Instance name (of the registered service)
+ * @param server non-null service waiting to have no clients (previously registered)
* @param cb non-null callback to call when there are no clients
* @return success
* true on success
@@ -35,7 +37,11 @@
* - the server or cb parameters are null
* - this is called by a process other than the server process
*/
- registerClientCallback(interface server, IClientCallback cb) generates (bool success);
+ registerClientCallback(string fqName,
+ string name,
+ interface server,
+ IClientCallback cb)
+ generates (bool success);
/**
* Removes a callback previously registered with registerClientCallback.
@@ -68,9 +74,32 @@
*
* See also @1.0::IServiceManager's listByInterface function.
*
- * @param fqName Fully-qualified interface name.
+ * @param fqName Fully-qualified interface name.
*
* @return instanceNames List of instance names running the particular service.
*/
listManifestByInterface(string fqName) generates (vec<string> instanceNames);
+
+ /**
+ * Unregisters a service if there are no clients for it. This must only unregister the
+ * service if it is called from the same process that registered the service.
+ *
+ * This unregisters all instances of the service, even if they are under a different instance
+ * name.
+ *
+ * Recommended usage is when creating a lazy process, try unregistering when IClientCallback's
+ * onClients(*, false) is called. If this unregister is successful, then it is safe to exit. If
+ * it is unsuccessful, then you can assume a client re-associated with the server. If a process
+ * has multiple servers, and only some succeed in unregistration, then the unregistered services
+ * can be re-registered.
+ *
+ * See also addWithChain and @1.0::IServiceManager's add.
+ *
+ * @param fqName Fully-qualified interface name.
+ * @param name Instance name.
+ * @param service Handle to registering service.
+ *
+ * @return success Whether the service was successfully unregistered.
+ */
+ tryUnregister(string fqName, string name, interface service) generates (bool success);
};