Merge RQ1A.210105.003 to stage-aosp-master - DO NOT MERGE

Merged-In: I3c36d6f26c6a855a890329fc8c5229e5946b2d36
Change-Id: Id97e4f718debccece7fe73d87feb00c892ac1c6b

3 files changed, 75 insertions, 74 deletions

diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp
index 325e204203..f2c5139b56 100644
--- a/libs/binder/LazyServiceRegistrar.cpp
+++ b/libs/binder/LazyServiceRegistrar.cpp
@@ -29,53 +29,59 @@ namespace internal {
 
 using AidlServiceManager = android::os::IServiceManager;
 
-class ClientCounterCallback : public ::android::os::BnClientCallback {
+class ClientCounterCallbackImpl : public ::android::os::BnClientCallback {
 public:
-    ClientCounterCallback() : mNumConnectedServices(0), mForcePersist(false) {}
+    ClientCounterCallbackImpl() : mNumConnectedServices(0), mForcePersist(false) {}
 
     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);
 
 protected:
     Status onClients(const sp<IBinder>& service, bool clients) override;
 
 private:
-    struct Service {
-        sp<IBinder> service;
-        bool allowIsolated;
-        int dumpFlags;
-
-        // whether, based on onClients calls, we know we have a client for this
-        // service or not
-        bool clients = false;
-    };
-
-    /**
-     * Looks up a service guaranteed to be registered (service from onClients).
-     */
-    std::map<std::string, Service>::iterator assertRegisteredService(const sp<IBinder>& service);
-
     /**
      * Unregisters all services that we can. If we can't unregister all, re-register other
      * services.
      */
     void tryShutdown();
 
-    // count of services with clients
+    /**
+     * Counter of the number of services that currently have at least one client.
+     */
     size_t mNumConnectedServices;
 
-    // map of registered names and services
+    struct Service {
+        sp<IBinder> service;
+        bool allowIsolated;
+        int dumpFlags;
+    };
+    /**
+     * Map of registered names and services
+     */
     std::map<std::string, Service> mRegisteredServices;
 
     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) {
     auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));
 
@@ -95,29 +101,13 @@ bool ClientCounterCallback::registerService(const sp<IBinder>& service, const st
         }
 
         // Only add this when a service is added for the first time, as it is not removed
-        mRegisteredServices[name] = {
-              .service = service,
-              .allowIsolated = allowIsolated,
-              .dumpFlags = dumpFlags
-        };
+        mRegisteredServices[name] = {service, allowIsolated, dumpFlags};
     }
 
     return true;
 }
 
-std::map<std::string, ClientCounterCallback::Service>::iterator ClientCounterCallback::assertRegisteredService(const sp<IBinder>& service) {
-    LOG_ALWAYS_FATAL_IF(service == nullptr, "Got onClients callback for null service");
-    for (auto it = mRegisteredServices.begin(); it != mRegisteredServices.end(); ++it) {
-        auto const& [name, registered] = *it;
-        (void) name;
-        if (registered.service != service) continue;
-        return it;
-    }
-    LOG_ALWAYS_FATAL("Got callback on service which we did not register: %s", String8(service->getInterfaceDescriptor()).c_str());
-    __builtin_unreachable();
-}
-
-void ClientCounterCallback::forcePersist(bool persist) {
+void ClientCounterCallbackImpl::forcePersist(bool persist) {
     mForcePersist = persist;
     if(!mForcePersist) {
         // Attempt a shutdown in case the number of clients hit 0 while the flag was on
@@ -129,32 +119,22 @@ void ClientCounterCallback::forcePersist(bool persist) {
  * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple
  * invocations could occur on different threads however.
  */
-Status ClientCounterCallback::onClients(const sp<IBinder>& service, bool clients) {
-    auto & [name, registered] = *assertRegisteredService(service);
-    if (registered.clients == clients) {
-        LOG_ALWAYS_FATAL("Process already thought %s had clients: %d but servicemanager has "
-                         "notified has clients: %d", name.c_str(), registered.clients, clients);
-    }
-    registered.clients = clients;
-
-    // update cache count of clients
-    {
-         size_t numWithClients = 0;
-         for (const auto& [name, registered] : mRegisteredServices) {
-             (void) name;
-             if (registered.clients) numWithClients++;
-         }
-         mNumConnectedServices = numWithClients;
+Status ClientCounterCallbackImpl::onClients(const sp<IBinder>& service, bool clients) {
+    if (clients) {
+        mNumConnectedServices++;
+    } else {
+        mNumConnectedServices--;
     }
 
     ALOGI("Process has %zu (of %zu available) client(s) in use after notification %s has clients: %d",
-          mNumConnectedServices, mRegisteredServices.size(), name.c_str(), clients);
+          mNumConnectedServices, mRegisteredServices.size(),
+          String8(service->getInterfaceDescriptor()).string(), clients);
 
     tryShutdown();
     return Status::ok();
 }
 
-void ClientCounterCallback::tryShutdown() {
+void ClientCounterCallbackImpl::tryShutdown() {
     if(mNumConnectedServices > 0) {
         // Should only shut down if there are no clients
         return;
@@ -175,7 +155,6 @@ void ClientCounterCallback::tryShutdown() {
 
         bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk();
 
-
         if (!success) {
             ALOGI("Failed to unregister service %s", entry.first.c_str());
             break;
@@ -200,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
 
 LazyServiceRegistrar::LazyServiceRegistrar() {
@@ -224,4 +216,4 @@ void LazyServiceRegistrar::forcePersist(bool persist) {
 }
 
 }  // namespace hardware
-}  // namespace android
+}  // namespace android
\ No newline at end of file
diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp
index d14a3014c8..3cccaf9329 100644
--- a/services/sensorservice/SensorEventConnection.cpp
+++ b/services/sensorservice/SensorEventConnection.cpp
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <log/log.h>
 #include <sys/socket.h>
 #include <utils/threads.h>
 
@@ -53,20 +54,13 @@ SensorService::SensorEventConnection::SensorEventConnection(
 SensorService::SensorEventConnection::~SensorEventConnection() {
     ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this);
     destroy();
-}
-
-void SensorService::SensorEventConnection::destroy() {
-    Mutex::Autolock _l(mDestroyLock);
-
-    // destroy once only
-    if (mDestroyed) {
-        return;
-    }
-
     mService->cleanupConnection(this);
     if (mEventCache != nullptr) {
         delete[] mEventCache;
     }
+}
+
+void SensorService::SensorEventConnection::destroy() {
     mDestroyed = true;
 }
 
@@ -679,6 +673,11 @@ status_t SensorService::SensorEventConnection::enableDisable(
         int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs,
         int reservedFlags)
 {
+    if (mDestroyed) {
+        android_errorWriteLog(0x534e4554, "168211968");
+        return DEAD_OBJECT;
+    }
+
     status_t err;
     if (enabled) {
         err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs,
@@ -693,10 +692,19 @@ status_t SensorService::SensorEventConnection::enableDisable(
 status_t SensorService::SensorEventConnection::setEventRate(
         int handle, nsecs_t samplingPeriodNs)
 {
+    if (mDestroyed) {
+        android_errorWriteLog(0x534e4554, "168211968");
+        return DEAD_OBJECT;
+    }
+
     return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName);
 }
 
 status_t  SensorService::SensorEventConnection::flush() {
+    if (mDestroyed) {
+        return DEAD_OBJECT;
+    }
+
     return  mService->flushSensor(this, mOpPackageName);
 }
 
diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h
index 8f2d5db28f..9487a39a92 100644
--- a/services/sensorservice/SensorEventConnection.h
+++ b/services/sensorservice/SensorEventConnection.h
@@ -17,6 +17,7 @@
 #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H
 #define ANDROID_SENSOR_EVENT_CONNECTION_H
 
+#include <atomic>
 #include <stdint.h>
 #include <sys/types.h>
 #include <unordered_map>
@@ -182,8 +183,8 @@ private:
     int mTotalAcksNeeded, mTotalAcksReceived;
 #endif
 
-    mutable Mutex mDestroyLock;
-    bool mDestroyed;
+    // Used to track if this object was inappropriately used after destroy().
+    std::atomic_bool mDestroyed;
 
     // Store a mapping of sensor handles to required AppOp for a sensor. This map only contains a
     // valid mapping for sensors that require a permission in order to reduce the lookup time.