From e148bc29c27ae7a346fa686fdda6425ba202d97a Mon Sep 17 00:00:00 2001
From: Aravind Akella <aakella@google.com>
Date: Wed, 24 Sep 2014 22:12:58 -0700
Subject: Fix a possible SensorService deadlock.

 If the destructor of SensorEventConnection gets called when the main
 sendEvents loop of SensorService is executing it may result in a deadlock.
 The loop promotes each connection to a strong_pointer, calls sendEvents
 and cleans up the connection if necessary. It is possible that the sp's
 destructor may delete SensorEventConnection which will call the dtor
 ~SensorEventConnection(). This dtor again needs SensorService mLock to
 execute which may result in a deadlock.

Bug: 17617897
Change-Id: I76c244dbe85fadb591c0bd1a9a5eb01d93f56505
---
 services/sensorservice/SensorService.cpp | 36 ++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 11 deletions(-)

(limited to 'services/sensorservice')

diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp
index 77ada40..4b50f6f 100644
--- a/services/sensorservice/SensorService.cpp
+++ b/services/sensorservice/SensorService.cpp
@@ -398,6 +398,24 @@ bool SensorService::threadLoop()
         for (int i = 0; i < count; i++) {
              mSensorEventBuffer[i].flags = 0;
         }
+
+        // Make a copy of the connection vector as some connections may be removed during the
+        // course of this loop (especially when one-shot sensor events are present in the
+        // sensor_event buffer). Promote all connections to StrongPointers before the lock is
+        // acquired. If the destructor of the sp gets called when the lock is acquired, it may
+        // result in a deadlock as ~SensorEventConnection() needs to acquire mLock again for
+        // cleanup. So copy all the strongPointers to a vector before the lock is acquired.
+        SortedVector< sp<SensorEventConnection> > activeConnections;
+        {
+            Mutex::Autolock _l(mLock);
+            for (size_t i=0 ; i < mActiveConnections.size(); ++i) {
+                sp<SensorEventConnection> connection(mActiveConnections[i].promote());
+                if (connection != 0) {
+                    activeConnections.add(connection);
+                }
+            }
+        }
+
         Mutex::Autolock _l(mLock);
         // Poll has returned. Hold a wakelock if one of the events is from a wake up sensor. The
         // rest of this loop is under a critical section protected by mLock. Acquiring a wakeLock,
@@ -487,21 +505,17 @@ bool SensorService::threadLoop()
         // Send our events to clients. Check the state of wake lock for each client and release the
         // lock if none of the clients need it.
         bool needsWakeLock = false;
-        // Make a copy of the connection vector as some connections may be removed during the
-        // course of this loop (especially when one-shot sensor events are present in the
-        // sensor_event buffer).
-        const SortedVector< wp<SensorEventConnection> > activeConnections(mActiveConnections);
         size_t numConnections = activeConnections.size();
         for (size_t i=0 ; i < numConnections; ++i) {
-            sp<SensorEventConnection> connection(activeConnections[i].promote());
-            if (connection != 0) {
-                connection->sendEvents(mSensorEventBuffer, count, mSensorEventScratch,
+            if (activeConnections[i] != 0) {
+                activeConnections[i]->sendEvents(mSensorEventBuffer, count, mSensorEventScratch,
                         mMapFlushEventsToConnections);
-                needsWakeLock |= connection->needsWakeLock();
+                needsWakeLock |= activeConnections[i]->needsWakeLock();
                 // If the connection has one-shot sensors, it may be cleaned up after first trigger.
                 // Early check for one-shot sensors.
-                if (connection->hasOneShotSensors()) {
-                    cleanupAutoDisabledSensorLocked(connection, mSensorEventBuffer, count);
+                if (activeConnections[i]->hasOneShotSensors()) {
+                    cleanupAutoDisabledSensorLocked(activeConnections[i], mSensorEventBuffer,
+                            count);
                 }
             }
         }
@@ -987,10 +1001,10 @@ SensorService::SensorEventConnection::SensorEventConnection(
 
 SensorService::SensorEventConnection::~SensorEventConnection() {
     ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this);
+    mService->cleanupConnection(this);
     if (mEventCache != NULL) {
         delete mEventCache;
     }
-    mService->cleanupConnection(this);
 }
 
 void SensorService::SensorEventConnection::onFirstRef() {
-- 
cgit v1.1