Merge changes from topic "looper_unique_fd"

* changes:
  libutils: switch Looper's fds to unique_fd.
  libziparchive: use fdsan in ZipArchive.
This commit is contained in:
Josh Gao 2018-07-23 21:26:11 +00:00 committed by Gerrit Code Review
commit 7e7cefa2c7
5 changed files with 75 additions and 56 deletions

View File

@ -155,6 +155,7 @@ cc_library {
],
},
linux: {
shared_libs: ["libbase"],
srcs: [
"Looper.cpp",
],

View File

@ -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

View File

@ -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.

View File

@ -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;

View File

@ -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);