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

Commit 0b858c1f authored by Brian Stack's avatar Brian Stack
Browse files

Do not send stale event to on-change sensor 

When a client requests to activate an on-change sensor, the
SensorService sends the previous sensor event to the new client if
the sensor is already active. This is necessary to ensure that the new
client receives a sensor event since it is unknown when the sensor may
generate a new sensor event.

If two clients simultaneously activate the same on-change sensor that
is currently not active, then the second client would receive a sensor
event from the previous activation which may be from a significant
time ago, leading to an inconsistent state between the clients.

This patch checks that the last sensor event it would send to the
second client is from the current sensor activation in order to
prevent sending any stale sensor events.

Bug: 116283108
Test: 1) Artificially added delay to generate on-change sensor event.
      2) Requested on-change sensor from client 1
      3) Requested on-change sensor from client 2
      4) Verified that client 2 did not receive a stale sensor event
      5) Verified that client 1 and client 2 both received the same
         up-to-date sensor event
Test: Verified that above test case fails at step 4 without this patch

Change-Id: I49587e3da7177882a1e8e67347bbb64acfe38200
parent c284ebf1

services/sensorservice/RecentEventLogger.cpp

+10 −3
Original line number Original line Diff line number Diff line
@@ -32,19 +32,26 @@ namespace { 




RecentEventLogger::RecentEventLogger(int sensorType) :

RecentEventLogger::RecentEventLogger(int sensorType) :

        mSensorType(sensorType), mEventSize(eventSizeBySensorType(mSensorType)),

        mSensorType(sensorType), mEventSize(eventSizeBySensorType(mSensorType)),

        mRecentEvents(logSizeBySensorType(sensorType)), mMaskData(false) {

        mRecentEvents(logSizeBySensorType(sensorType)), mMaskData(false),

        mIsLastEventCurrent(false) {

    // blank

    // blank

}

}





void RecentEventLogger::addEvent(const sensors_event_t& event) {

void RecentEventLogger::addEvent(const sensors_event_t& event) {

    std::lock_guard<std::mutex> lk(mLock);

    std::lock_guard<std::mutex> lk(mLock);

    mRecentEvents.emplace(event);

    mRecentEvents.emplace(event);

    mIsLastEventCurrent = true;

}

}





bool RecentEventLogger::isEmpty() const {

bool RecentEventLogger::isEmpty() const {

    return mRecentEvents.size() == 0;

    return mRecentEvents.size() == 0;

}

}





void RecentEventLogger::setLastEventStale() {

    std::lock_guard<std::mutex> lk(mLock);

    mIsLastEventCurrent = false;

}



std::string RecentEventLogger::dump() const {

std::string RecentEventLogger::dump() const {

    std::lock_guard<std::mutex> lk(mLock);

    std::lock_guard<std::mutex> lk(mLock);
@@ -85,10 +92,10 @@ void RecentEventLogger::setFormat(std::string format) { 
    }

    }

}

}





bool RecentEventLogger::populateLastEvent(sensors_event_t *event) const {

bool RecentEventLogger::populateLastEventIfCurrent(sensors_event_t *event) const {

    std::lock_guard<std::mutex> lk(mLock);

    std::lock_guard<std::mutex> lk(mLock);





    if (mRecentEvents.size()) {

    if (mIsLastEventCurrent && mRecentEvents.size()) {

        // Index 0 contains the latest event emplace()'ed

        // Index 0 contains the latest event emplace()'ed

        *event = mRecentEvents[0].mEvent;

        *event = mRecentEvents[0].mEvent;

        return true;

        return true;

services/sensorservice/RecentEventLogger.h

+7 −1
Original line number Original line Diff line number Diff line
@@ -37,8 +37,13 @@ class RecentEventLogger : public Dumpable { 
public:

public:

    explicit RecentEventLogger(int sensorType);

    explicit RecentEventLogger(int sensorType);

    void addEvent(const sensors_event_t& event);

    void addEvent(const sensors_event_t& event);

    bool populateLastEvent(sensors_event_t *event) const;



    // Populate event with the last recorded sensor event if it is not stale. An event is

    // considered stale if the sensor has become deactivated since the event was recorded.

    // returns true on success, false if no recent event is available or the last event is stale

    bool populateLastEventIfCurrent(sensors_event_t *event) const;

    bool isEmpty() const;

    bool isEmpty() const;

    void setLastEventStale();

    virtual ~RecentEventLogger() {}

    virtual ~RecentEventLogger() {}





    // Dumpable interface

    // Dumpable interface
@@ -59,6 +64,7 @@ protected: 
    RingBuffer<SensorEventLog> mRecentEvents;

    RingBuffer<SensorEventLog> mRecentEvents;





    bool mMaskData;

    bool mMaskData;

    bool mIsLastEventCurrent;





private:

private:

    static size_t logSizeBySensorType(int sensorType);

    static size_t logSizeBySensorType(int sensorType);

services/sensorservice/SensorService.cpp

+14 −4
Original line number Original line Diff line number Diff line
@@ -1290,6 +1290,15 @@ void SensorService::cleanupConnection(SensorEventConnection* c) { 
            ALOGD_IF(DEBUG_CONNECTIONS, "... and it was the last connection");

            ALOGD_IF(DEBUG_CONNECTIONS, "... and it was the last connection");

            mActiveSensors.removeItemsAt(i, 1);

            mActiveSensors.removeItemsAt(i, 1);

            mActiveVirtualSensors.erase(handle);

            mActiveVirtualSensors.erase(handle);



            // If this is the last connection, then mark the RecentEventLogger as stale. This is

            // critical for on-change events since the previous event is sent to a client if the

            // sensor is already active. If two clients request the sensor at the same time, one

            // of the clients would receive a stale event.

            auto logger = mRecentEvent.find(handle);

            if (logger != mRecentEvent.end()) {

                logger->second->setLastEventStale();

            }

            delete rec;

            delete rec;

            size--;

            size--;

        } else {

        } else {
@@ -1356,10 +1365,11 @@ status_t SensorService::enable(const sp<SensorEventConnection>& connection, 
                auto logger = mRecentEvent.find(handle);

                auto logger = mRecentEvent.find(handle);

                if (logger != mRecentEvent.end()) {

                if (logger != mRecentEvent.end()) {

                    sensors_event_t event;

                    sensors_event_t event;

                    // It is unlikely that this buffer is empty as the sensor is already active.

                    // Verify that the last sensor event was generated from the current activation

                    // One possible corner case may be two applications activating an on-change

                    // of the sensor. If not, it is possible for an on-change sensor to receive a

                    // sensor at the same time.

                    // sensor event that is stale if two clients re-activate the sensor

                    if(logger->second->populateLastEvent(&event)) {

                    // simultaneously.

                    if(logger->second->populateLastEventIfCurrent(&event)) {

                        event.sensor = handle;

                        event.sensor = handle;

                        if (event.version == sizeof(sensors_event_t)) {

                        if (event.version == sizeof(sensors_event_t)) {

                            if (isWakeUpSensorEvent(event) && !mWakeLockAcquired) {

                            if (isWakeUpSensorEvent(event) && !mWakeLockAcquired) {