Merge "LazyServiceRegistrar: race w/ register & onClients" (d6afe2eb) · Commits · e / os / android_frameworks_native

libs/binder/LazyServiceRegistrar.cpp

+40 −25

Original line number	Diff line number	Diff line
		@@ -40,9 +40,9 @@ public:

		void setActiveServicesCallback(const std::function<bool(bool)>& activeServicesCallback);

		bool tryUnregister();
		bool tryUnregisterLocked();

		void reRegister();
		void reRegisterLocked();

		protected:
		Status onClients(const sp<IBinder>& service, bool clients) override;
		@@ -59,6 +59,9 @@ private:
		bool registered = true;
		};

		bool registerServiceLocked(const sp<IBinder>& service, const std::string& name,
		bool allowIsolated, int dumpFlags);

		/**
		* Looks up a service guaranteed to be registered (service from onClients).
		*/
		@@ -68,7 +71,7 @@ private:
		* Unregisters all services that we can. If we can't unregister all, re-register other
		* services.
		*/
		void tryShutdown();
		void tryShutdownLocked();

		/**
		* Try to shutdown the process, unless:
		@@ -76,7 +79,10 @@ private:
		* - The active services count callback returns 'true', or
		* - Some services have clients.
		*/
		void maybeTryShutdown();
		void maybeTryShutdownLocked();

		// for below
		std::mutex mMutex;

		// count of services with clients
		size_t mNumConnectedServices;
		@@ -117,6 +123,13 @@ private:

		bool ClientCounterCallbackImpl::registerService(const sp<IBinder>& service, const std::string& name,
		bool allowIsolated, int dumpFlags) {
		std::lock_guard<std::mutex> lock(mMutex);
		return registerServiceLocked(service, name, allowIsolated, dumpFlags);
		}

		bool ClientCounterCallbackImpl::registerServiceLocked(const sp<IBinder>& service,
		const std::string& name, bool allowIsolated,
		int dumpFlags) {
		auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));

		bool reRegister = mRegisteredServices.count(name) > 0;
		@@ -164,14 +177,15 @@ std::map<std::string, ClientCounterCallbackImpl::Service>::iterator ClientCounte
		}

		void ClientCounterCallbackImpl::forcePersist(bool persist) {
		std::lock_guard<std::mutex> lock(mMutex);
		mForcePersist = persist;
		if (!mForcePersist) {
		// Attempt a shutdown in case the number of clients hit 0 while the flag was on
		maybeTryShutdown();
		maybeTryShutdownLocked();
		}
		}

		bool ClientCounterCallbackImpl::tryUnregister() {
		bool ClientCounterCallbackImpl::tryUnregisterLocked() {
		auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));

		for (auto& [name, entry] : mRegisteredServices) {
		@@ -187,15 +201,14 @@ bool ClientCounterCallbackImpl::tryUnregister() {
		return true;
		}

		void ClientCounterCallbackImpl::reRegister() {
		void ClientCounterCallbackImpl::reRegisterLocked() {
		for (auto& [name, entry] : mRegisteredServices) {
		// re-register entry if not already registered
		if (entry.registered) {
		continue;
		}

		if (!registerService(entry.service, name, entry.allowIsolated,
		entry.dumpFlags)) {
		if (!registerServiceLocked(entry.service, name, entry.allowIsolated, entry.dumpFlags)) {
		// Must restart. Otherwise, clients will never be able to get a hold of this service.
		LOG_ALWAYS_FATAL("Bad state: could not re-register services");
		}
		@@ -204,7 +217,7 @@ void ClientCounterCallbackImpl::reRegister() {
		}
		}

		void ClientCounterCallbackImpl::maybeTryShutdown() {
		void ClientCounterCallbackImpl::maybeTryShutdownLocked() {
		if (mForcePersist) {
		ALOGI("Shutdown prevented by forcePersist override flag.");
		return;
		@@ -223,15 +236,12 @@ void ClientCounterCallbackImpl::maybeTryShutdown() {
		// client count change event, try to shutdown the process if its services
		// have no clients.
		if (!handledInCallback && mNumConnectedServices == 0) {
		tryShutdown();
		tryShutdownLocked();
		}
		}

		/**
		* onClients is oneway, so no need to worry about multi-threading. Note that this means multiple
		* invocations could occur on different threads however.
		*/
		Status ClientCounterCallbackImpl::onClients(const sp<IBinder>& service, bool clients) {
		std::lock_guard<std::mutex> lock(mMutex);
		auto & [name, registered] = *assertRegisteredService(service);
		if (registered.clients == clients) {
		LOG_ALWAYS_FATAL("Process already thought %s had clients: %d but servicemanager has "
		@@ -252,23 +262,24 @@ Status ClientCounterCallbackImpl::onClients(const sp<IBinder>& service, bool cli
		ALOGI("Process has %zu (of %zu available) client(s) in use after notification %s has clients: %d",
		mNumConnectedServices, mRegisteredServices.size(), name.c_str(), clients);

		maybeTryShutdown();
		maybeTryShutdownLocked();
		return Status::ok();
		}

		void ClientCounterCallbackImpl::tryShutdown() {
		void ClientCounterCallbackImpl::tryShutdownLocked() {
		ALOGI("Trying to shut down the service. No clients in use for any service in process.");

		if (tryUnregister()) {
		if (tryUnregisterLocked()) {
		ALOGI("Unregistered all clients and exiting");
		exit(EXIT_SUCCESS);
		}

		reRegister();
		reRegisterLocked();
		}

		void ClientCounterCallbackImpl::setActiveServicesCallback(const std::function<bool(bool)>&
		activeServicesCallback) {
		std::lock_guard<std::mutex> lock(mMutex);
		mActiveServicesCallback = activeServicesCallback;
		}

		@@ -291,11 +302,15 @@ void ClientCounterCallback::setActiveServicesCallback(const std::function<bool(b
		}

		bool ClientCounterCallback::tryUnregister() {
		return mImpl->tryUnregister();
		// see comments in header, this should only be called from the active
		// services callback, see also b/191781736
		return mImpl->tryUnregisterLocked();
		}

		void ClientCounterCallback::reRegister() {
		mImpl->reRegister();
		// see comments in header, this should only be called from the active
		// services callback, see also b/191781736
		mImpl->reRegisterLocked();
		}

		} // namespace internal

libs/binder/include/binder/LazyServiceRegistrar.h

+3 −2

Original line number	Diff line number	Diff line
		@@ -81,7 +81,8 @@ class LazyServiceRegistrar {

		/**
		* Try to unregister all services previously registered with 'registerService'.
		* Returns 'true' if successful.
		* Returns 'true' if successful. This should only be called within the callback registered by
		* setActiveServicesCallback.
		*/
		bool tryUnregister();