Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 410d567e authored by Arthur Ishiguro's avatar Arthur Ishiguro
Browse files

Fix deadlock on SensorService 

If mLock is held by SensorService::threadLoop(), and a new call
from SensorEventConnection comes in to e.g. enable/disable a sensor,
this thread must wait for the mLock to be released. However, if this
thread holds mConnectionLock in SensorEventConnection, and SensorService
invokes SensorEventConnection.sendEvent, the system will come into a
deadlock because both threads are waiting for each other.

To avoid this situation unlock SensotEventConnection's mConnectionLock
while going out of scope.

Also create separate mutexes for modifying mSensorInfo and
mSensorInfoBackup for better separation.

Bug: 153188258
Test: 1) Run sensor logger and enable sensor, put app in background.
      2) Open a_sns_test and start streaming sensor samples
      3) Open app again and verify no system ANR, sensor samples
      continue to be received on app.
Change-Id: If137f39918a8b1148dcc00f1b1d82b4454bb5c7b
parent 584ab0d9

services/sensorservice/SensorEventConnection.cpp

+33 −23
Original line number Diff line number Diff line
@@ -70,17 +70,16 @@ void SensorService::SensorEventConnection::onFirstRef() { 
}



bool SensorService::SensorEventConnection::needsWakeLock() {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    return !mDead && mWakeLockRefCount > 0;

}



void SensorService::SensorEventConnection::resetWakeLockRefCount() {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    mWakeLockRefCount = 0;

}



void SensorService::SensorEventConnection::dump(String8& result) {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    result.appendFormat("\tOperating Mode: ");

    if (!mService->isWhiteListedPackage(getPackageName())) {

        result.append("RESTRICTED\n");
@@ -92,6 +91,8 @@ void SensorService::SensorEventConnection::dump(String8& result) { 
    result.appendFormat("\t %s | WakeLockRefCount %d | uid %d | cache size %d | "

            "max cache size %d\n", mPackageName.string(), mWakeLockRefCount, mUid, mCacheSize,

            mMaxCacheSize);



    std::lock_guard<std::mutex> _l(mConnectionLock);

    for (auto& it : mSensorInfo) {

        const FlushInfo& flushInfo = it.second.flushInfo;

        result.appendFormat("\t %s 0x%08x | status: %s | pending flush events %d \n",
@@ -122,7 +123,7 @@ void SensorService::SensorEventConnection::dump(String8& result) { 
 */

void SensorService::SensorEventConnection::dump(util::ProtoOutputStream* proto) const {

    using namespace service::SensorEventConnectionProto;

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);



    if (!mService->isWhiteListedPackage(getPackageName())) {

        proto->write(OPERATING_MODE, OP_MODE_RESTRICTED);
@@ -160,7 +161,7 @@ void SensorService::SensorEventConnection::dump(util::ProtoOutputStream* proto) 


bool SensorService::SensorEventConnection::addSensor(

    int32_t handle, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    sp<SensorInterface> si = mService->getSensorInterfaceFromHandle(handle);

    if (si == nullptr ||

        !canAccessSensor(si->getSensor(), "Tried adding", mOpPackageName) ||
@@ -179,12 +180,12 @@ bool SensorService::SensorEventConnection::addSensor( 
}



bool SensorService::SensorEventConnection::removeSensor(int32_t handle) {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    return mSensorInfo.erase(handle) > 0;

}



std::vector<int32_t> SensorService::SensorEventConnection::getActiveSensorHandles() const {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    std::vector<int32_t> list;

    for (auto& it : mSensorInfo) {

        list.push_back(it.first);
@@ -193,17 +194,19 @@ std::vector<int32_t> SensorService::SensorEventConnection::getActiveSensorHandle 
}



bool SensorService::SensorEventConnection::hasSensor(int32_t handle) const {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::recursive_mutex> _backlock(mBackupLock);

    std::lock_guard<std::mutex> _lock(mConnectionLock);

    return mSensorInfo.count(handle) + mSensorInfoBackup.count(handle) > 0;

}



bool SensorService::SensorEventConnection::hasAnySensor() const {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::recursive_mutex> _backlock(mBackupLock);

    std::lock_guard<std::mutex> _lock(mConnectionLock);

    return mSensorInfo.size() + mSensorInfoBackup.size() ? true : false;

}



bool SensorService::SensorEventConnection::hasOneShotSensors() const {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    for (auto &it : mSensorInfo) {

        const int handle = it.first;

        sp<SensorInterface> si = mService->getSensorInterfaceFromHandle(handle);
@@ -220,7 +223,7 @@ String8 SensorService::SensorEventConnection::getPackageName() const { 


void SensorService::SensorEventConnection::setFirstFlushPending(int32_t handle,

                                bool value) {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    if (mSensorInfo.count(handle) > 0) {

        FlushInfo& flushInfo = mSensorInfo[handle].flushInfo;

        flushInfo.mFirstFlushPending = value;
@@ -228,7 +231,7 @@ void SensorService::SensorEventConnection::setFirstFlushPending(int32_t handle, 
}



void SensorService::SensorEventConnection::updateLooperRegistration(const sp<Looper>& looper) {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    updateLooperRegistrationLocked(looper);

}
@@ -279,7 +282,7 @@ void SensorService::SensorEventConnection::updateLooperRegistrationLocked( 
}



void SensorService::SensorEventConnection::incrementPendingFlushCount(int32_t handle) {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    if (mSensorInfo.count(handle) > 0) {

        FlushInfo& flushInfo = mSensorInfo[handle].flushInfo;

        flushInfo.mPendingFlushEventsToSend++;
@@ -295,7 +298,7 @@ status_t SensorService::SensorEventConnection::sendEvents( 
    std::unique_ptr<sensors_event_t[]> sanitizedBuffer;



    int count = 0;

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    if (scratch) {

        size_t i=0;

        while (i<numEvents) {
@@ -453,11 +456,18 @@ void SensorService::SensorEventConnection::setSensorAccess(const bool hasAccess) 
}



void SensorService::SensorEventConnection::stopAll() {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    bool backupPerformed = false;

    std::lock_guard<std::recursive_mutex> _backlock(mBackupLock);

    {

        std::lock_guard<std::mutex> _lock(mConnectionLock);

        if (!mSensorInfo.empty()) {

            mSensorInfoBackup = mSensorInfo;

            mSensorInfo.clear();

            backupPerformed = true;

        }

    }



    if (backupPerformed) {

        for (auto& it : mSensorInfoBackup) {

            int32_t handle = it.first;
@@ -471,7 +481,7 @@ void SensorService::SensorEventConnection::stopAll() { 
}



void SensorService::SensorEventConnection::recoverAll() {

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::recursive_mutex> _l(mBackupLock);

    for (auto& it : mSensorInfoBackup) {

        int32_t handle = it.first;

        SensorRequest &request = it.second;
@@ -612,7 +622,7 @@ void SensorService::SensorEventConnection::writeToSocketFromCache() { 
    // half the size of the socket buffer allocated in BitTube whichever is smaller.

    const int maxWriteSize = helpers::min(SensorEventQueue::MAX_RECEIVE_BUFFER_EVENT_COUNT/2,

            int(mService->mSocketBufferSize/(sizeof(sensors_event_t)*2)));

    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    // Send pending flush complete events (if any)

    sendPendingFlushEventsLocked();

    for (int numEventsSent = 0; numEventsSent < mCacheSize;) {
@@ -724,7 +734,7 @@ status_t SensorService::SensorEventConnection::setEventRate( 
{

    status_t err = mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName);



    std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

    std::lock_guard<std::mutex> _l(mConnectionLock);

    if (err == NO_ERROR && mSensorInfo.count(handle) > 0) {

        mSensorInfo[handle].samplingPeriodNs = samplingPeriodNs;

    }
@@ -750,7 +760,7 @@ int SensorService::SensorEventConnection::handleEvent(int fd, int events, void* 
            // and remove the fd from Looper. Call checkWakeLockState to know if SensorService

            // can release the wake-lock.

            ALOGD_IF(DEBUG_CONNECTIONS, "%p Looper error %d", this, fd);

            std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

            std::lock_guard<std::mutex> _l(mConnectionLock);

            mDead = true;

            mWakeLockRefCount = 0;

            updateLooperRegistrationLocked(mService->getLooper());
@@ -769,7 +779,7 @@ int SensorService::SensorEventConnection::handleEvent(int fd, int events, void* 
        unsigned char buf[sizeof(sensors_event_t)];

        ssize_t numBytesRead = ::recv(fd, buf, sizeof(buf), MSG_DONTWAIT);

        {

            std::lock_guard<std::recursive_mutex> _l(mConnectionLock);

            std::lock_guard<std::mutex> _l(mConnectionLock);

            if (numBytesRead == sizeof(sensors_event_t)) {

                if (!mDataInjectionMode) {

                    ALOGE("Data injected in normal mode, dropping event"

services/sensorservice/SensorEventConnection.h

+7 −1
Original line number Diff line number Diff line
@@ -146,7 +146,13 @@ private: 
    sp<SensorService> const mService;

    sp<BitTube> mChannel;

    uid_t mUid;

    mutable std::recursive_mutex mConnectionLock;



    // A lock that should be used when modifying mSensorInfo

    mutable std::mutex mConnectionLock;



    // A lock that should be used when modifying mSensorInfoBackup

    mutable std::recursive_mutex mBackupLock;



    // Number of events from wake up sensors which are still pending and haven't been delivered to

    // the corresponding application. It is incremented by one unit for each write to the socket.

    uint32_t mWakeLockRefCount;

services/sensorservice/SensorService.cpp

+3 −0
Original line number Diff line number Diff line
@@ -316,6 +316,9 @@ void SensorService::setSensorAccess(uid_t uid, bool hasAccess) { 
            conn->setSensorAccess(hasAccess);

        }

    }



    // Lock the mutex again for clean shutdown

    mLock.lock();

}



const Sensor& SensorService::registerSensor(SensorInterface* s, bool isDebug, bool isVirtual) {