Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dc6cb05e authored by Jon Spivack's avatar Jon Spivack Committed by android-build-team Robot
Browse files

libbinder: Add ClientCounterCallbackImpl to LazyServiceRegistrar 

This extra layer of indirection below ClientCounterCallback fixes a shared pointer ownership issue between LazyServiceRegistrar and ServiceManager. It also allows for implementation changes (like this one) without changing headers and breaking VNDK.

Bug: 170212632
Test: Manual (Went through reproduction steps in bug on cf_x86_phone-userdebug)
Test: atest aidl_lazy_test
Change-Id: I4164a6d44e567c752726953e85aee0e91c6b525e
Merged-In: I4164a6d44e567c752726953e85aee0e91c6b525e
(cherry picked from commit 7c227cc3)
parent adb416ac

libs/binder/LazyServiceRegistrar.cpp

+35 −11
Original line number Original line Diff line number Diff line
@@ -29,16 +29,12 @@ namespace internal { 




using AidlServiceManager = android::os::IServiceManager;

using AidlServiceManager = android::os::IServiceManager;





class ClientCounterCallback : public ::android::os::BnClientCallback {

class ClientCounterCallbackImpl : public ::android::os::BnClientCallback {

public:

public:

    ClientCounterCallback() : mNumConnectedServices(0), mForcePersist(false) {}

    ClientCounterCallbackImpl() : mNumConnectedServices(0), mForcePersist(false) {}





    bool registerService(const sp<IBinder>& service, const std::string& name,

    bool registerService(const sp<IBinder>& service, const std::string& name,

                         bool allowIsolated, int dumpFlags);

                         bool allowIsolated, int dumpFlags);



    /**

     * Set a flag to prevent services from automatically shutting down

     */

    void forcePersist(bool persist);

    void forcePersist(bool persist);





protected:

protected:
@@ -69,7 +65,23 @@ private: 
    bool mForcePersist;

    bool mForcePersist;

};

};





bool ClientCounterCallback::registerService(const sp<IBinder>& service, const std::string& name,

class ClientCounterCallback {

public:

    ClientCounterCallback();



    bool registerService(const sp<IBinder>& service, const std::string& name,

                                            bool allowIsolated, int dumpFlags);



    /**

     * Set a flag to prevent services from automatically shutting down

     */

    void forcePersist(bool persist);



private:

    sp<ClientCounterCallbackImpl> mImpl;

};



bool ClientCounterCallbackImpl::registerService(const sp<IBinder>& service, const std::string& name,

                                            bool allowIsolated, int dumpFlags) {

                                            bool allowIsolated, int dumpFlags) {

    auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));

    auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));
@@ -95,7 +107,7 @@ bool ClientCounterCallback::registerService(const sp<IBinder>& service, const st 
    return true;

    return true;

}

}





void ClientCounterCallback::forcePersist(bool persist) {

void ClientCounterCallbackImpl::forcePersist(bool persist) {

    mForcePersist = persist;

    mForcePersist = persist;

    if(!mForcePersist) {

    if(!mForcePersist) {

        // Attempt a shutdown in case the number of clients hit 0 while the flag was on

        // Attempt a shutdown in case the number of clients hit 0 while the flag was on
@@ -107,7 +119,7 @@ void ClientCounterCallback::forcePersist(bool persist) { 
 * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple

 * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple

 * invocations could occur on different threads however.

 * invocations could occur on different threads however.

 */

 */

Status ClientCounterCallback::onClients(const sp<IBinder>& service, bool clients) {

Status ClientCounterCallbackImpl::onClients(const sp<IBinder>& service, bool clients) {

    if (clients) {

    if (clients) {

        mNumConnectedServices++;

        mNumConnectedServices++;

    } else {

    } else {
@@ -122,7 +134,7 @@ Status ClientCounterCallback::onClients(const sp<IBinder>& service, bool clients 
    return Status::ok();

    return Status::ok();

}

}





void ClientCounterCallback::tryShutdown() {

void ClientCounterCallbackImpl::tryShutdown() {

    if(mNumConnectedServices > 0) {

    if(mNumConnectedServices > 0) {

        // Should only shut down if there are no clients

        // Should only shut down if there are no clients

        return;

        return;
@@ -143,7 +155,6 @@ void ClientCounterCallback::tryShutdown() { 




        bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk();

        bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk();







        if (!success) {

        if (!success) {

            ALOGI("Failed to unregister service %s", entry.first.c_str());

            ALOGI("Failed to unregister service %s", entry.first.c_str());

            break;

            break;
@@ -168,6 +179,19 @@ void ClientCounterCallback::tryShutdown() { 
    }

    }

}

}





ClientCounterCallback::ClientCounterCallback() {

      mImpl = new ClientCounterCallbackImpl();

}



bool ClientCounterCallback::registerService(const sp<IBinder>& service, const std::string& name,

                                            bool allowIsolated, int dumpFlags) {

    return mImpl->registerService(service, name, allowIsolated, dumpFlags);

}



void ClientCounterCallback::forcePersist(bool persist) {

    mImpl->forcePersist(persist);

}



}  // namespace internal

}  // namespace internal





LazyServiceRegistrar::LazyServiceRegistrar() {

LazyServiceRegistrar::LazyServiceRegistrar() {