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

Commit 3688d0aa authored by Ray Essick's avatar Ray Essick
Browse files

Remove pid-caching from BufferPoolAccessor 

BufferPoolAccessor cached the pid via a static constructor. If the
process forks after this, then multiple processes generating unique ids
using the same pid value. This resulted in connection ID collisions.
use getpid(), which  already caches and resets appropriately across fork().

Bug: 142423602
Bug: 133186424
Test: boot, watch log connectionIds, collision-induced failures are gone
parent 5871ab51

media/bufferpool/1.0/AccessorImpl.cpp

+9 −4
Original line number Diff line number Diff line
@@ -14,10 +14,11 @@ 
 * limitations under the License.

 */



#define LOG_TAG "BufferPoolAccessor"

#define LOG_TAG "BufferPoolAccessor1.0"

//#define LOG_NDEBUG 0



#include <sys/types.h>

#include <stdint.h>

#include <time.h>

#include <unistd.h>

#include <utils/Log.h>
@@ -127,7 +128,6 @@ bool contains(std::map<T, std::set<U>> *mapOfSet, T key, U value) { 
    return false;

}



int32_t Accessor::Impl::sPid = getpid();

uint32_t Accessor::Impl::sSeqId = time(nullptr);



Accessor::Impl::Impl(
@@ -145,16 +145,21 @@ ResultStatus Accessor::Impl::connect( 
    {

        std::lock_guard<std::mutex> lock(mBufferPool.mMutex);

        if (newConnection) {

            ConnectionId id = (int64_t)sPid << 32 | sSeqId;

            int32_t pid = getpid();

            ConnectionId id = (int64_t)pid << 32 | sSeqId;

            status = mBufferPool.mObserver.open(id, fmqDescPtr);

            if (status == ResultStatus::OK) {

                newConnection->initialize(accessor, id);

                *connection = newConnection;

                *pConnectionId = id;

                mBufferPool.mConnectionIds.insert(id);

                if (sSeqId == UINT32_MAX) {

                   sSeqId = 0;

                } else {

                    ++sSeqId;

                }

            }

        }

        mBufferPool.processStatusMessages();

        mBufferPool.cleanUp();

    }

media/bufferpool/1.0/AccessorImpl.h

+0 −1
Original line number Diff line number Diff line
@@ -61,7 +61,6 @@ private: 
    // ConnectionId = pid : (timestamp_created + seqId)

    // in order to guarantee uniqueness for each connection

    static uint32_t sSeqId;

    static int32_t sPid;



    const std::shared_ptr<BufferPoolAllocator> mAllocator;

media/bufferpool/2.0/AccessorImpl.cpp

+9 −4
Original line number Diff line number Diff line
@@ -14,10 +14,11 @@ 
 * limitations under the License.

 */



#define LOG_TAG "BufferPoolAccessor"

#define LOG_TAG "BufferPoolAccessor2.0"

//#define LOG_NDEBUG 0



#include <sys/types.h>

#include <stdint.h>

#include <time.h>

#include <unistd.h>

#include <utils/Log.h>
@@ -134,7 +135,6 @@ bool contains(std::map<T, std::set<U>> *mapOfSet, T key, U value) { 
    return false;

}



int32_t Accessor::Impl::sPid = getpid();

uint32_t Accessor::Impl::sSeqId = time(nullptr);



Accessor::Impl::Impl(
@@ -156,7 +156,8 @@ ResultStatus Accessor::Impl::connect( 
    {

        std::lock_guard<std::mutex> lock(mBufferPool.mMutex);

        if (newConnection) {

            ConnectionId id = (int64_t)sPid << 32 | sSeqId;

            int32_t pid = getpid();

            ConnectionId id = (int64_t)pid << 32 | sSeqId;

            status = mBufferPool.mObserver.open(id, statusDescPtr);

            if (status == ResultStatus::OK) {

                newConnection->initialize(accessor, id);
@@ -166,8 +167,12 @@ ResultStatus Accessor::Impl::connect( 
                mBufferPool.mConnectionIds.insert(id);

                mBufferPool.mInvalidationChannel.getDesc(invDescPtr);

                mBufferPool.mInvalidation.onConnect(id, observer);

                if (sSeqId == UINT32_MAX) {

                   sSeqId = 0;

                } else {

                    ++sSeqId;

                }

            }



        }

        mBufferPool.processStatusMessages();

media/bufferpool/2.0/AccessorImpl.h

+0 −1
Original line number Diff line number Diff line
@@ -75,7 +75,6 @@ private: 
    // ConnectionId = pid : (timestamp_created + seqId)

    // in order to guarantee uniqueness for each connection

    static uint32_t sSeqId;

    static int32_t sPid;



    const std::shared_ptr<BufferPoolAllocator> mAllocator;