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

Commit 7e7cefa2 authored by Josh Gao's avatar Josh Gao Committed by Gerrit Code Review
Browse files

Merge changes from topic "looper_unique_fd" 

* changes:
  libutils: switch Looper's fds to unique_fd.
  libziparchive: use fdsan in ZipArchive.
parents 67c5ca20 2d08ae57

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.

libziparchive/zip_archive.cc

+42 −0
Original line number Diff line number Diff line
@@ -33,6 +33,10 @@ 
#include <memory>

#include <vector>



#if defined(__BIONIC__)

#include <android/fdsan.h>

#endif



#include <android-base/file.h>

#include <android-base/logging.h>

#include <android-base/macros.h>  // TEMP_FAILURE_RETRY may or may not be in unistd
@@ -165,6 +169,44 @@ static int32_t AddToHash(ZipString* hash_table, const uint64_t hash_table_size, 
  return 0;

}



ZipArchive::ZipArchive(const int fd, bool assume_ownership)

    : mapped_zip(fd),

      close_file(assume_ownership),

      directory_offset(0),

      central_directory(),

      directory_map(new android::FileMap()),

      num_entries(0),

      hash_table_size(0),

      hash_table(nullptr) {

#if defined(__BIONIC__)

  if (assume_ownership) {

    android_fdsan_exchange_owner_tag(fd, 0, reinterpret_cast<uint64_t>(this));

  }

#endif

}



ZipArchive::ZipArchive(void* address, size_t length)

    : mapped_zip(address, length),

      close_file(false),

      directory_offset(0),

      central_directory(),

      directory_map(new android::FileMap()),

      num_entries(0),

      hash_table_size(0),

      hash_table(nullptr) {}



ZipArchive::~ZipArchive() {

  if (close_file && mapped_zip.GetFileDescriptor() >= 0) {

#if defined(__BIONIC__)

    android_fdsan_close_with_tag(mapped_zip.GetFileDescriptor(), reinterpret_cast<uint64_t>(this));

#else

    close(mapped_zip.GetFileDescriptor());

#endif

  }



  free(hash_table);

}



static int32_t MapCentralDirectory0(const char* debug_file_name, ZipArchive* archive,

                                    off64_t file_length, off64_t read_amount, uint8_t* scan_buffer) {

  const off64_t search_start = file_length - read_amount;

libziparchive/zip_archive_private.h

+3 −27
Original line number Diff line number Diff line
@@ -156,33 +156,9 @@ struct ZipArchive { 
  uint32_t hash_table_size;

  ZipString* hash_table;



  ZipArchive(const int fd, bool assume_ownership)

      : mapped_zip(fd),

        close_file(assume_ownership),

        directory_offset(0),

        central_directory(),

        directory_map(new android::FileMap()),

        num_entries(0),

        hash_table_size(0),

        hash_table(nullptr) {}



  ZipArchive(void* address, size_t length)

      : mapped_zip(address, length),

        close_file(false),

        directory_offset(0),

        central_directory(),

        directory_map(new android::FileMap()),

        num_entries(0),

        hash_table_size(0),

        hash_table(nullptr) {}



  ~ZipArchive() {

    if (close_file && mapped_zip.GetFileDescriptor() >= 0) {

      close(mapped_zip.GetFileDescriptor());

    }



    free(hash_table);

  }

  ZipArchive(const int fd, bool assume_ownership);

  ZipArchive(void* address, size_t length);

  ~ZipArchive();



  bool InitializeCentralDirectory(const char* debug_file_name, off64_t cd_start_offset,

                                  size_t cd_size);