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

Commit a228cc5e authored by Mathias Agopian's avatar Mathias Agopian Committed by Android (Google) Code Review
Browse files

Merge "fix a race condition in undoDequeue(), where 'tail' could be computed... 

Merge "fix a race condition in undoDequeue(), where 'tail' could be computed incorrectly." into kraken
parents b219882a 0a8cd068

include/private/surfaceflinger/SharedBufferStack.h

+2 −2
Original line number Original line Diff line number Diff line
@@ -167,6 +167,7 @@ protected: 
    SharedBufferStack* const mSharedStack;

    SharedBufferStack* const mSharedStack;

    const int mNumBuffers;

    const int mNumBuffers;

    const int mIdentity;

    const int mIdentity;

    int32_t computeTail() const;





    friend struct Update;

    friend struct Update;

    friend struct QueueUpdate;

    friend struct QueueUpdate;
@@ -260,8 +261,6 @@ private: 
    friend struct DequeueCondition;

    friend struct DequeueCondition;

    friend struct LockCondition;

    friend struct LockCondition;





    int32_t computeTail() const;



    struct QueueUpdate : public UpdateBase {

    struct QueueUpdate : public UpdateBase {

        inline QueueUpdate(SharedBufferBase* sbb);

        inline QueueUpdate(SharedBufferBase* sbb);

        inline ssize_t operator()();

        inline ssize_t operator()();
@@ -288,6 +287,7 @@ private: 
    };

    };





    int32_t tail;

    int32_t tail;

    int32_t undoDequeueTail;

    // statistics...

    // statistics...

    nsecs_t mDequeueTime[NUM_BUFFER_MAX];

    nsecs_t mDequeueTime[NUM_BUFFER_MAX];

};

};

libs/surfaceflinger_client/SharedBufferStack.cpp

+11 −24
Original line number Original line Diff line number Diff line
@@ -179,7 +179,7 @@ String8 SharedBufferBase::dump(char const* prefix) const 
    char buffer[SIZE];

    char buffer[SIZE];

    String8 result;

    String8 result;

    SharedBufferStack& stack( *mSharedStack );

    SharedBufferStack& stack( *mSharedStack );

    int tail = (mNumBuffers + stack.head - stack.available + 1) % mNumBuffers;

    int tail = computeTail();

    snprintf(buffer, SIZE, 

    snprintf(buffer, SIZE, 

            "%s[ head=%2d, available=%2d, queued=%2d, tail=%2d ] "

            "%s[ head=%2d, available=%2d, queued=%2d, tail=%2d ] "

            "reallocMask=%08x, inUse=%2d, identity=%d, status=%d\n",

            "reallocMask=%08x, inUse=%2d, identity=%d, status=%d\n",
@@ -189,6 +189,12 @@ String8 SharedBufferBase::dump(char const* prefix) const 
    return result;

    return result;

}

}





int32_t SharedBufferBase::computeTail() const

{

    SharedBufferStack& stack( *mSharedStack );

    return (mNumBuffers + stack.head - stack.available + 1) % mNumBuffers;

}



// ============================================================================

// ============================================================================

// conditions and updates

// conditions and updates

// ============================================================================

// ============================================================================
@@ -297,32 +303,12 @@ ssize_t SharedBufferServer::StatusUpdate::operator()() { 




SharedBufferClient::SharedBufferClient(SharedClient* sharedClient,

SharedBufferClient::SharedBufferClient(SharedClient* sharedClient,

        int surface, int num, int32_t identity)

        int surface, int num, int32_t identity)

    : SharedBufferBase(sharedClient, surface, num, identity), tail(0)

    : SharedBufferBase(sharedClient, surface, num, identity),

      tail(0), undoDequeueTail(0)

{

{

    tail = computeTail();

    tail = computeTail();

}

}





int32_t SharedBufferClient::computeTail() const

{

    SharedBufferStack& stack( *mSharedStack );

    // we need to make sure we read available and head coherently,

    // w.r.t RetireUpdate.

    int32_t newTail;

    int32_t avail;

    int32_t head;

    do {

        avail = stack.available;

        head = stack.head;

    } while (stack.available != avail);

    newTail = head - avail + 1;

    if (newTail < 0) {

        newTail += mNumBuffers;

    } else if (newTail >= mNumBuffers) {

        newTail -= mNumBuffers;

    }

    return newTail;

}



ssize_t SharedBufferClient::dequeue()

ssize_t SharedBufferClient::dequeue()

{

{

    SharedBufferStack& stack( *mSharedStack );

    SharedBufferStack& stack( *mSharedStack );
@@ -350,6 +336,7 @@ ssize_t SharedBufferClient::dequeue() 




    int dequeued = tail;

    int dequeued = tail;

    tail = ((tail+1 >= mNumBuffers) ? 0 : tail+1);

    tail = ((tail+1 >= mNumBuffers) ? 0 : tail+1);

    undoDequeueTail = dequeued;

    LOGD_IF(DEBUG_ATOMICS, "dequeued=%d, tail=%d, %s",

    LOGD_IF(DEBUG_ATOMICS, "dequeued=%d, tail=%d, %s",

            dequeued, tail, dump("").string());

            dequeued, tail, dump("").string());
@@ -363,7 +350,7 @@ status_t SharedBufferClient::undoDequeue(int buf) 
    UndoDequeueUpdate update(this);

    UndoDequeueUpdate update(this);

    status_t err = updateCondition( update );

    status_t err = updateCondition( update );

    if (err == NO_ERROR) {

    if (err == NO_ERROR) {

        tail = computeTail();

        tail = undoDequeueTail;

    }

    }

    return err;

    return err;

}

}