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

Commit 2d08ae57 authored by Josh Gao's avatar Josh Gao
Browse files

libutils: switch Looper's fds to unique_fd. 

Switch Looper to using unique_fd for its owned file descriptors, to
benefit from fdsan.

Bug: http://b/111560345
Test: treehugger
Change-Id: I8efff7741ed19fd71f82f7e604b4f1c66fc5ea2b
parent 1b49634f

libutils/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -155,6 +155,7 @@ cc_library { 
            ],

        },

        linux: {

            shared_libs: ["libbase"],

            srcs: [

                "Looper.cpp",

            ],

libutils/Looper.cpp

+25 −27
Original line number Diff line number Diff line
@@ -60,24 +60,22 @@ static const int EPOLL_MAX_EVENTS = 16; 
static pthread_once_t gTLSOnce = PTHREAD_ONCE_INIT;

static pthread_key_t gTLSKey = 0;



Looper::Looper(bool allowNonCallbacks) :

        mAllowNonCallbacks(allowNonCallbacks), mSendingMessage(false),

        mPolling(false), mEpollFd(-1), mEpollRebuildRequired(false),

        mNextRequestSeq(0), mResponseIndex(0), mNextMessageUptime(LLONG_MAX) {

    mWakeEventFd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);

    LOG_ALWAYS_FATAL_IF(mWakeEventFd < 0, "Could not make wake event fd: %s",

                        strerror(errno));

Looper::Looper(bool allowNonCallbacks)

    : mAllowNonCallbacks(allowNonCallbacks),

      mSendingMessage(false),

      mPolling(false),

      mEpollRebuildRequired(false),

      mNextRequestSeq(0),

      mResponseIndex(0),

      mNextMessageUptime(LLONG_MAX) {

    mWakeEventFd.reset(eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC));

    LOG_ALWAYS_FATAL_IF(mWakeEventFd.get() < 0, "Could not make wake event fd: %s", strerror(errno));



    AutoMutex _l(mLock);

    rebuildEpollLocked();

}



Looper::~Looper() {

    close(mWakeEventFd);

    mWakeEventFd = -1;

    if (mEpollFd >= 0) {

        close(mEpollFd);

    }

}



void Looper::initTLSKey() {
@@ -137,18 +135,18 @@ void Looper::rebuildEpollLocked() { 
#if DEBUG_CALLBACKS

        ALOGD("%p ~ rebuildEpollLocked - rebuilding epoll set", this);

#endif

        close(mEpollFd);

        mEpollFd.reset();

    }



    // Allocate the new epoll instance and register the wake pipe.

    mEpollFd = epoll_create(EPOLL_SIZE_HINT);

    mEpollFd.reset(epoll_create(EPOLL_SIZE_HINT));

    LOG_ALWAYS_FATAL_IF(mEpollFd < 0, "Could not create epoll instance: %s", strerror(errno));



    struct epoll_event eventItem;

    memset(& eventItem, 0, sizeof(epoll_event)); // zero out unused members of data field union

    eventItem.events = EPOLLIN;

    eventItem.data.fd = mWakeEventFd;

    int result = epoll_ctl(mEpollFd, EPOLL_CTL_ADD, mWakeEventFd, & eventItem);

    eventItem.data.fd = mWakeEventFd.get();

    int result = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mWakeEventFd.get(), &eventItem);

    LOG_ALWAYS_FATAL_IF(result != 0, "Could not add wake event fd to epoll instance: %s",

                        strerror(errno));
@@ -157,7 +155,7 @@ void Looper::rebuildEpollLocked() { 
        struct epoll_event eventItem;

        request.initEventItem(&eventItem);



        int epollResult = epoll_ctl(mEpollFd, EPOLL_CTL_ADD, request.fd, & eventItem);

        int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, request.fd, &eventItem);

        if (epollResult < 0) {

            ALOGE("Error adding epoll events for fd %d while rebuilding epoll set: %s",

                  request.fd, strerror(errno));
@@ -239,7 +237,7 @@ int Looper::pollInner(int timeoutMillis) { 
    mPolling = true;



    struct epoll_event eventItems[EPOLL_MAX_EVENTS];

    int eventCount = epoll_wait(mEpollFd, eventItems, EPOLL_MAX_EVENTS, timeoutMillis);

    int eventCount = epoll_wait(mEpollFd.get(), eventItems, EPOLL_MAX_EVENTS, timeoutMillis);



    // No longer idling.

    mPolling = false;
@@ -281,7 +279,7 @@ int Looper::pollInner(int timeoutMillis) { 
    for (int i = 0; i < eventCount; i++) {

        int fd = eventItems[i].data.fd;

        uint32_t epollEvents = eventItems[i].events;

        if (fd == mWakeEventFd) {

        if (fd == mWakeEventFd.get()) {

            if (epollEvents & EPOLLIN) {

                awoken();

            } else {
@@ -401,11 +399,11 @@ void Looper::wake() { 
#endif



    uint64_t inc = 1;

    ssize_t nWrite = TEMP_FAILURE_RETRY(write(mWakeEventFd, &inc, sizeof(uint64_t)));

    ssize_t nWrite = TEMP_FAILURE_RETRY(write(mWakeEventFd.get(), &inc, sizeof(uint64_t)));

    if (nWrite != sizeof(uint64_t)) {

        if (errno != EAGAIN) {

            LOG_ALWAYS_FATAL("Could not write wake signal to fd %d: %s",

                    mWakeEventFd, strerror(errno));

            LOG_ALWAYS_FATAL("Could not write wake signal to fd %d: %s", mWakeEventFd.get(),

                             strerror(errno));

        }

    }

}
@@ -416,7 +414,7 @@ void Looper::awoken() { 
#endif



    uint64_t counter;

    TEMP_FAILURE_RETRY(read(mWakeEventFd, &counter, sizeof(uint64_t)));

    TEMP_FAILURE_RETRY(read(mWakeEventFd.get(), &counter, sizeof(uint64_t)));

}



void Looper::pushResponse(int events, const Request& request) {
@@ -467,14 +465,14 @@ int Looper::addFd(int fd, int ident, int events, const sp<LooperCallback>& callb 


        ssize_t requestIndex = mRequests.indexOfKey(fd);

        if (requestIndex < 0) {

            int epollResult = epoll_ctl(mEpollFd, EPOLL_CTL_ADD, fd, & eventItem);

            int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, fd, &eventItem);

            if (epollResult < 0) {

                ALOGE("Error adding epoll events for fd %d: %s", fd, strerror(errno));

                return -1;

            }

            mRequests.add(fd, request);

        } else {

            int epollResult = epoll_ctl(mEpollFd, EPOLL_CTL_MOD, fd, & eventItem);

            int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_MOD, fd, &eventItem);

            if (epollResult < 0) {

                if (errno == ENOENT) {

                    // Tolerate ENOENT because it means that an older file descriptor was
@@ -495,7 +493,7 @@ int Looper::addFd(int fd, int ident, int events, const sp<LooperCallback>& callb 
                            "being recycled, falling back on EPOLL_CTL_ADD: %s",

                            this, strerror(errno));

#endif

                    epollResult = epoll_ctl(mEpollFd, EPOLL_CTL_ADD, fd, & eventItem);

                    epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, fd, &eventItem);

                    if (epollResult < 0) {

                        ALOGE("Error modifying or adding epoll events for fd %d: %s",

                                fd, strerror(errno));
@@ -542,7 +540,7 @@ int Looper::removeFd(int fd, int seq) { 
        // updating the epoll set so that we avoid accidentally leaking callbacks.

        mRequests.removeItemsAt(requestIndex);



        int epollResult = epoll_ctl(mEpollFd, EPOLL_CTL_DEL, fd, nullptr);

        int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_DEL, fd, nullptr);

        if (epollResult < 0) {

            if (seq != -1 && (errno == EBADF || errno == ENOENT)) {

                // Tolerate EBADF or ENOENT when the sequence number is known because it

libutils/include/utils/Looper.h

+4 −2
Original line number Diff line number Diff line
@@ -24,6 +24,8 @@ 


#include <sys/epoll.h>



#include <android-base/unique_fd.h>



namespace android {



/*
@@ -447,7 +449,7 @@ private: 


    const bool mAllowNonCallbacks; // immutable



    int mWakeEventFd;  // immutable

    android::base::unique_fd mWakeEventFd;  // immutable

    Mutex mLock;



    Vector<MessageEnvelope> mMessageEnvelopes; // guarded by mLock
@@ -457,7 +459,7 @@ private: 
    // any use of it is racy anyway.

    volatile bool mPolling;



    int mEpollFd; // guarded by mLock but only modified on the looper thread

    android::base::unique_fd mEpollFd;  // guarded by mLock but only modified on the looper thread

    bool mEpollRebuildRequired; // guarded by mLock



    // Locked list of file descriptor monitoring requests.