Ensure looper isn't used after invalidateSensorQueue
Previous attempts resulted in improper locking that created a deadlock
siutation. This attempt creates a new lock for mValid that should avoid
any deadlock scenario that could arise from normal use.
Bug: 175074139
Test: Load on Pixel device and run various applications
Merged-In: I4e5a68b70353161f21e0d93cac1ceb11cc9ad035
Change-Id: I4e5a68b70353161f21e0d93cac1ceb11cc9ad035
(cherry picked from commit 99d5d9ac9e30c43e7d8c58cd562e3fffb26f4771)
diff --git a/sensorservice/libsensorndkbridge/ASensorEventQueue.cpp b/sensorservice/libsensorndkbridge/ASensorEventQueue.cpp
index d109426..bf24f68 100644
--- a/sensorservice/libsensorndkbridge/ASensorEventQueue.cpp
+++ b/sensorservice/libsensorndkbridge/ASensorEventQueue.cpp
@@ -30,7 +30,11 @@
using android::hardware::Return;
ASensorEventQueue::ASensorEventQueue(ALooper* looper, ALooper_callbackFunc callback, void* data)
- : mLooper(looper), mCallback(callback), mData(data), mRequestAdditionalInfo(false) {}
+ : mLooper(looper),
+ mCallback(callback),
+ mData(data),
+ mRequestAdditionalInfo(false),
+ mValid(true) {}
void ASensorEventQueue::setImpl(const sp<IEventQueue> &queueImpl) {
mQueueImpl = queueImpl;
@@ -106,17 +110,31 @@
LOG(VERBOSE) << "ASensorEventQueue::onEvent";
if (static_cast<int32_t>(event.sensorType) != ASENSOR_TYPE_ADDITIONAL_INFO ||
- mRequestAdditionalInfo.load()) {
- {
- Mutex::Autolock autoLock(mLock);
+ mRequestAdditionalInfo.load()) {
+ {
+ // Only lock the mutex in this block to avoid the following deadlock scenario:
+ //
+ // ASensorEventQueue::onEvent is called which grabs ASensorEventQueue::mLock followed
+ // by ALooper::mLock via ALooper::signalSensorEvents.
+ //
+ // Meanwhile
+ //
+ // ASensorEventQueue::dispatchCallback is invoked from ALooper::pollOnce which has
+ // has ALooper::mLock locked and the dispatched callback invokes
+ // ASensorEventQueue::getEvents which would try to grab ASensorEventQueue::mLock
+ // resulting in a deadlock.
+ Mutex::Autolock autoLock(mLock);
mQueue.emplace_back();
sensors_event_t* sensorEvent = &mQueue[mQueue.size() - 1];
android::hardware::sensors::V1_0::implementation::convertToSensorEvent(event,
sensorEvent);
}
- mLooper->signalSensorEvents(this);
+ Mutex::Autolock autoLock(mValidLock);
+ if (mValid) {
+ mLooper->signalSensorEvents(this);
+ }
}
return android::hardware::Void();
@@ -134,6 +152,13 @@
}
void ASensorEventQueue::invalidate() {
+ {
+ // Only lock within this context to avoid locking while calling invalidateSensorQueue which
+ // also holds a lock. This is safe to do because mValid can't be made true after it's false
+ // so onEvent will never signal new sensor events after mValid is false.
+ Mutex::Autolock autoLock(mValidLock);
+ mValid = false;
+ }
mLooper->invalidateSensorQueue(this);
setImpl(nullptr);
}
diff --git a/sensorservice/libsensorndkbridge/ASensorEventQueue.h b/sensorservice/libsensorndkbridge/ASensorEventQueue.h
index 4937276..7139d34 100644
--- a/sensorservice/libsensorndkbridge/ASensorEventQueue.h
+++ b/sensorservice/libsensorndkbridge/ASensorEventQueue.h
@@ -73,6 +73,8 @@
std::vector<sensors_event_t> mQueue;
std::atomic_bool mRequestAdditionalInfo;
+ android::Mutex mValidLock;
+ bool mValid;
DISALLOW_COPY_AND_ASSIGN(ASensorEventQueue);
};
diff --git a/sensorservice/libsensorndkbridge/ASensorManager.h b/sensorservice/libsensorndkbridge/ASensorManager.h
index 818429d..4e91122 100644
--- a/sensorservice/libsensorndkbridge/ASensorManager.h
+++ b/sensorservice/libsensorndkbridge/ASensorManager.h
@@ -44,6 +44,8 @@
ALooper_callbackFunc callback,
void *data);
+ // This must not be called from inside ALooper_callbackFunc to avoid deadlocking inside of the
+ // ALooper.
void destroyEventQueue(ASensorEventQueue *queue);
private:
