fix a dead-lock in sensorservice

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
This commit is contained in:
Mathias Agopian 2013-07-12 02:01:16 -07:00
parent c07b52060a
commit ac9a96da65
6 changed files with 38 additions and 31 deletions

View File

@ -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

View File

@ -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);
};

View File

@ -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 {

View File

@ -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);
};

View File

@ -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

View File

@ -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);