diff options
author | Colin Cross <ccross@android.com> | 2023-04-20 11:49:48 -0700 |
---|---|---|
committer | Colin Cross <ccross@android.com> | 2023-05-09 21:55:54 +0000 |
commit | 2f9bb95303904c77bed2e427c3149c96f7fc86cd (patch) | |
tree | 02701e57ffaaf3a0b514d13e9786bd10a3625bdf | |
parent | 840364e62b243315e6586b8b7e902a29a756139b (diff) | |
download | adb-2f9bb95303904c77bed2e427c3149c96f7fc86cd.tar.gz |
Order events returned by fdevent_epoll
The events returned by fdevent_epoll are ordered by
installed_fdevents_, which is an unordered map. This causes
the order to be non-determinsitic, but generally stable within a
process. Switch installed_fdevents_ to an ordered map, which will
return the events ordered by fd number. This will make tests more
deterministic and allow writing a test that depends on event handling
order.
Bug: 268740023
Test: adb_test
Ignore-AOSP-First: Resolution for potential security exploit.
Change-Id: I65bef05d4edd6057bf9b2cebc4952b2f1e14b4d1
-rw-r--r-- | fdevent/fdevent.h | 4 | ||||
-rw-r--r-- | fdevent/fdevent_epoll.cpp | 28 |
2 files changed, 19 insertions, 13 deletions
diff --git a/fdevent/fdevent.h b/fdevent/fdevent.h index ff86f1fe..84d086b6 100644 --- a/fdevent/fdevent.h +++ b/fdevent/fdevent.h @@ -24,9 +24,9 @@ #include <chrono> #include <deque> #include <functional> +#include <map> #include <mutex> #include <optional> -#include <unordered_map> #include <variant> #include <android-base/thread_annotations.h> @@ -125,7 +125,7 @@ struct fdevent_context { std::optional<uint64_t> looper_thread_id_ = std::nullopt; std::atomic<bool> terminate_loop_ = false; - std::unordered_map<int, fdevent> installed_fdevents_; + std::map<int, fdevent> installed_fdevents_; private: uint64_t fdevent_id_ = 0; diff --git a/fdevent/fdevent_epoll.cpp b/fdevent/fdevent_epoll.cpp index d5d4d668..147913a5 100644 --- a/fdevent/fdevent_epoll.cpp +++ b/fdevent/fdevent_epoll.cpp @@ -108,14 +108,18 @@ void fdevent_context_epoll::Loop() { looper_thread_id_ = android::base::GetThreadId(); std::vector<fdevent_event> fde_events; + std::unordered_map<fdevent*, fdevent_event*> event_map; std::vector<epoll_event> epoll_events; - epoll_events.resize(this->installed_fdevents_.size()); while (true) { if (terminate_loop_) { break; } + if (epoll_events.size() < this->installed_fdevents_.size()) { + epoll_events.resize(this->installed_fdevents_.size()); + } + int rc = -1; while (rc == -1) { std::optional<std::chrono::milliseconds> timeout = CalculatePollDuration(); @@ -133,7 +137,10 @@ void fdevent_context_epoll::Loop() { } auto post_poll = std::chrono::steady_clock::now(); - std::unordered_map<fdevent*, unsigned> event_map; + fde_events.reserve(installed_fdevents_.size()); + fde_events.clear(); + event_map.clear(); + for (int i = 0; i < rc; ++i) { fdevent* fde = static_cast<fdevent*>(epoll_events[i].data.ptr); @@ -152,13 +159,16 @@ void fdevent_context_epoll::Loop() { events |= FDE_READ | FDE_ERROR; } - event_map[fde] = events; + LOG(DEBUG) << dump_fde(fde) << " got events " << std::hex << std::showbase << events; + auto& fde_event = fde_events.emplace_back(fde, events); + event_map[fde] = &fde_event; + fde->last_active = post_poll; } for (auto& [fd, fde] : installed_fdevents_) { unsigned events = 0; if (auto it = event_map.find(&fde); it != event_map.end()) { - events = it->second; + events = it->second->events; } if (events == 0) { @@ -166,16 +176,12 @@ void fdevent_context_epoll::Loop() { auto deadline = fde.last_active + *fde.timeout; if (deadline < post_poll) { events |= FDE_TIMEOUT; + LOG(DEBUG) << dump_fde(&fde) << " timed out"; + fde_events.emplace_back(&fde, events); + fde.last_active = post_poll; } } } - - if (events != 0) { - LOG(DEBUG) << dump_fde(&fde) << " got events " << std::hex << std::showbase - << events; - fde_events.push_back({&fde, events}); - fde.last_active = post_poll; - } } this->HandleEvents(fde_events); fde_events.clear(); |