fix a dead-lock in sensorserviceandroid-4.3_r3.1android-4.3_r3android-4.3.1_r1jb-mr2.0.0-releasejb-mr2-dev

sensorservice would deadlock if for some reason
a sensor failed to enable.

simplifed the code a bit, and made it behave a little
closer to mr1.1 -- I couldn't convince myself that
some changes in how locks were used were correct.

Bug: 9794362
Change-Id: I6110f5dbb67e543f1c71d127de2299232badb36a

6 files changed, 38 insertions, 31 deletions

diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp
index 16dabe8fe5..a12529e17a 100644
--- a/services/sensorservice/SensorDevice.cpp
+++ b/services/sensorservice/SensorDevice.cpp
@@ -111,13 +111,10 @@ ssize_t SensorDevice::poll(sensors_event_t* buffer, size_t count) {
     return c;
 }
 
-status_t SensorDevice::resetStateWithoutActuatingHardware(void *ident, int handle)
-{
-    if (!mSensorDevice) return NO_INIT;
-    Info& info( mActivationCount.editValueFor(handle));
+void SensorDevice::autoDisable(void *ident, int handle) {
+    Info& info( mActivationCount.editValueFor(handle) );
     Mutex::Autolock _l(mLock);
     info.rates.removeItem(ident);
-    return NO_ERROR;
 }
 
 status_t SensorDevice::activate(void* ident, int handle, int enabled)
@@ -168,6 +165,15 @@ status_t SensorDevice::activate(void* ident, int handle, int enabled)
         ALOGE_IF(err, "Error %s sensor %d (%s)",
                 enabled ? "activating" : "disabling",
                 handle, strerror(-err));
+
+        if (err != NO_ERROR) {
+            // clean-up on failure
+            if (enabled) {
+                // failure when enabling the sensor
+                Mutex::Autolock _l(mLock);
+                info.rates.removeItem(ident);
+            }
+        }
     }
 
     { // scope for the lock
diff --git a/services/sensorservice/SensorDevice.h b/services/sensorservice/SensorDevice.h
index c0b357d5c1..227dab6686 100644
--- a/services/sensorservice/SensorDevice.h
+++ b/services/sensorservice/SensorDevice.h
@@ -56,7 +56,7 @@ public:
     ssize_t poll(sensors_event_t* buffer, size_t count);
     status_t activate(void* ident, int handle, int enabled);
     status_t setDelay(void* ident, int handle, int64_t ns);
-    status_t resetStateWithoutActuatingHardware(void *ident, int handle);
+    void autoDisable(void *ident, int handle);
     void dump(String8& result, char* buffer, size_t SIZE);
 };
 
diff --git a/services/sensorservice/SensorInterface.cpp b/services/sensorservice/SensorInterface.cpp
index cf0a11df6b..b483b75149 100644
--- a/services/sensorservice/SensorInterface.cpp
+++ b/services/sensorservice/SensorInterface.cpp
@@ -54,8 +54,8 @@ status_t HardwareSensor::setDelay(void* ident, int handle, int64_t ns) {
     return mSensorDevice.setDelay(ident, handle, ns);
 }
 
-status_t HardwareSensor::resetStateWithoutActuatingHardware(void *ident, int handle) {
-    return mSensorDevice.resetStateWithoutActuatingHardware(ident, handle);
+void HardwareSensor::autoDisable(void *ident, int handle) {
+    mSensorDevice.autoDisable(ident, handle);
 }
 
 Sensor HardwareSensor::getSensor() const {
diff --git a/services/sensorservice/SensorInterface.h b/services/sensorservice/SensorInterface.h
index 2e709ae4d2..2e14e57821 100644
--- a/services/sensorservice/SensorInterface.h
+++ b/services/sensorservice/SensorInterface.h
@@ -40,11 +40,7 @@ public:
     virtual status_t setDelay(void* ident, int handle, int64_t ns) = 0;
     virtual Sensor getSensor() const = 0;
     virtual bool isVirtual() const = 0;
-    virtual status_t resetStateWithoutActuatingHardware(void *ident, int handle) {
-        // Override when you want to clean up for sensors which auto disable
-        // after trigger, or when enabling sensors fail.
-        return INVALID_OPERATION;
-    }
+    virtual void autoDisable(void *ident, int handle) { }
 };
 
 // ---------------------------------------------------------------------------
@@ -66,7 +62,7 @@ public:
     virtual status_t setDelay(void* ident, int handle, int64_t ns);
     virtual Sensor getSensor() const;
     virtual bool isVirtual() const { return false; }
-    virtual status_t resetStateWithoutActuatingHardware(void *ident, int handle);
+    virtual void autoDisable(void *ident, int handle);
 };
 
 
diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp
index ebf5cf0c7d..6481584a62 100644
--- a/services/sensorservice/SensorService.cpp
+++ b/services/sensorservice/SensorService.cpp
@@ -249,10 +249,8 @@ void SensorService::cleanupAutoDisabledSensor(const sp<SensorEventConnection>& c
         if (getSensorType(handle) == SENSOR_TYPE_SIGNIFICANT_MOTION) {
             if (connection->hasSensor(handle)) {
                 sensor = mSensorMap.valueFor(handle);
-                err = sensor ?sensor->resetStateWithoutActuatingHardware(connection.get(), handle)
-                        : status_t(BAD_VALUE);
-                if (err != NO_ERROR) {
-                    ALOGE("Sensor Inteface: Resetting state failed with err: %d", err);
+                if (sensor != NULL) {
+                    sensor->autoDisable(connection.get(), handle);
                 }
                 cleanupWithoutDisable(connection, handle);
             }
@@ -496,8 +494,12 @@ status_t SensorService::enable(const sp<SensorEventConnection>& connection,
     if (mInitCheck != NO_ERROR)
         return mInitCheck;
 
-    Mutex::Autolock _l(mLock);
     SensorInterface* sensor = mSensorMap.valueFor(handle);
+    if (sensor == NULL) {
+        return BAD_VALUE;
+    }
+
+    Mutex::Autolock _l(mLock);
     SensorRecord* rec = mActiveSensors.valueFor(handle);
     if (rec == 0) {
         rec = new SensorRecord(connection);
@@ -533,16 +535,11 @@ status_t SensorService::enable(const sp<SensorEventConnection>& connection,
             handle, connection.get());
     }
 
-
     // we are setup, now enable the sensor.
-    status_t err = sensor ? sensor->activate(connection.get(), true) : status_t(BAD_VALUE);
-
+    status_t err = sensor->activate(connection.get(), true);
     if (err != NO_ERROR) {
-        // enable has failed, reset state in SensorDevice.
-        status_t resetErr = sensor ? sensor->resetStateWithoutActuatingHardware(connection.get(),
-                handle) : status_t(BAD_VALUE);
         // enable has failed, reset our state.
-        cleanupWithoutDisable(connection, handle);
+        cleanupWithoutDisableLocked(connection, handle);
     }
     return err;
 }
@@ -553,7 +550,8 @@ status_t SensorService::disable(const sp<SensorEventConnection>& connection,
     if (mInitCheck != NO_ERROR)
         return mInitCheck;
 
-    status_t err = cleanupWithoutDisable(connection, handle);
+    Mutex::Autolock _l(mLock);
+    status_t err = cleanupWithoutDisableLocked(connection, handle);
     if (err == NO_ERROR) {
         SensorInterface* sensor = mSensorMap.valueFor(handle);
         err = sensor ? sensor->activate(connection.get(), false) : status_t(BAD_VALUE);
@@ -561,9 +559,14 @@ status_t SensorService::disable(const sp<SensorEventConnection>& connection,
     return err;
 }
 
-status_t SensorService::cleanupWithoutDisable(const sp<SensorEventConnection>& connection,
-        int handle) {
+status_t SensorService::cleanupWithoutDisable(
+        const sp<SensorEventConnection>& connection, int handle) {
     Mutex::Autolock _l(mLock);
+    return cleanupWithoutDisableLocked(connection, handle);
+}
+
+status_t SensorService::cleanupWithoutDisableLocked(
+        const sp<SensorEventConnection>& connection, int handle) {
     SensorRecord* rec = mActiveSensors.valueFor(handle);
     if (rec) {
         // see if this connection becomes inactive
diff --git a/services/sensorservice/SensorService.h b/services/sensorservice/SensorService.h
index 25e5f764b0..fcdbc7d625 100644
--- a/services/sensorservice/SensorService.h
+++ b/services/sensorservice/SensorService.h
@@ -115,8 +115,10 @@ class SensorService :
     static void sortEventBuffer(sensors_event_t* buffer, size_t count);
     void registerSensor(SensorInterface* sensor);
     void registerVirtualSensor(SensorInterface* sensor);
-    status_t cleanupWithoutDisable(const sp<SensorEventConnection>& connection,
-        int handle);
+    status_t cleanupWithoutDisable(
+            const sp<SensorEventConnection>& connection, int handle);
+    status_t cleanupWithoutDisableLocked(
+            const sp<SensorEventConnection>& connection, int handle);
     void cleanupAutoDisabledSensor(const sp<SensorEventConnection>& connection,
         sensors_event_t const* buffer, const int count);