Snap for 10453563 from c362e05a76d7238f684234fe4ad6768c0c461b8d to mainline-tzdata5-release
Change-Id: I7c8251be118ef8d3b4e3bb353f0eef526422f6c2
diff --git a/daemon/jdwp_service.cpp b/daemon/jdwp_service.cpp
index d03573f..2501688 100644
--- a/daemon/jdwp_service.cpp
+++ b/daemon/jdwp_service.cpp
@@ -459,12 +459,16 @@
}
static asocket* create_process_tracker_service_socket(TrackerKind kind) {
- auto t = std::make_unique<JdwpTracker>(kind, true);
+ std::unique_ptr<JdwpTracker> t = std::make_unique<JdwpTracker>(kind, true);
if (!t) {
LOG(FATAL) << "failed to allocate JdwpTracker";
}
- memset(t.get(), 0, sizeof(asocket));
+ /* Object layout (with an inheritance hierarchy) varies across arch (e.g
+ * armv7a/Android TV vs aarch64), so no assumptions can be made about
+ * accessing fields based on offsets (e.g memset(t.get(), 0, sizeof(asocket))
+ * might clobber an unintended memory location).
+ */
install_local_socket(t.get());
D("LS(%d): created new jdwp tracker service", t->id);
diff --git a/fdevent/fdevent.cpp b/fdevent/fdevent.cpp
index b0758b8..e20a295 100644
--- a/fdevent/fdevent.cpp
+++ b/fdevent/fdevent.cpp
@@ -83,6 +83,7 @@
LOG(ERROR) << "failed to set non-blocking mode for fd " << fde->fd.get();
}
+ this->fdevent_set_.insert(fde);
this->Register(fde);
return fde;
}
@@ -100,6 +101,8 @@
auto erased = this->installed_fdevents_.erase(fd.get());
CHECK_EQ(1UL, erased);
+ erased = this->fdevent_set_.erase(fde);
+ CHECK_EQ(1UL, erased);
return fd;
}
@@ -150,7 +153,12 @@
void fdevent_context::HandleEvents(const std::vector<fdevent_event>& events) {
for (const auto& event : events) {
- invoke_fde(event.fde, event.events);
+ // Verify the fde is still installed before invoking it. It could have been unregistered
+ // and destroyed inside an earlier event handler.
+ if (this->fdevent_set_.find(event.fde) != this->fdevent_set_.end()) {
+ invoke_fde(event.fde, event.events);
+ break;
+ }
}
FlushRunQueue();
}
diff --git a/fdevent/fdevent.h b/fdevent/fdevent.h
index ff86f1f..7f8e1a2 100644
--- a/fdevent/fdevent.h
+++ b/fdevent/fdevent.h
@@ -24,9 +24,10 @@
#include <chrono>
#include <deque>
#include <functional>
+#include <map>
#include <mutex>
#include <optional>
-#include <unordered_map>
+#include <set>
#include <variant>
#include <android-base/thread_annotations.h>
@@ -125,12 +126,14 @@
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;
std::mutex run_queue_mutex_;
std::deque<std::function<void()>> run_queue_ GUARDED_BY(run_queue_mutex_);
+
+ std::set<fdevent*> fdevent_set_;
};
// Backwards compatibility shims that forward to the global fdevent_context.
diff --git a/fdevent/fdevent_epoll.cpp b/fdevent/fdevent_epoll.cpp
index d5d4d66..147913a 100644
--- a/fdevent/fdevent_epoll.cpp
+++ b/fdevent/fdevent_epoll.cpp
@@ -108,14 +108,18 @@
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 @@
}
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 @@
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 @@
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();
diff --git a/fdevent/fdevent_test.cpp b/fdevent/fdevent_test.cpp
index 20a82ea..4c33406 100644
--- a/fdevent/fdevent_test.cpp
+++ b/fdevent/fdevent_test.cpp
@@ -315,3 +315,86 @@
ASSERT_LT(diff[1], delta.count() * 0.5);
ASSERT_LT(diff[2], delta.count() * 0.5);
}
+
+TEST_F(FdeventTest, unregister_with_pending_event) {
+ fdevent_reset();
+
+ int fds1[2];
+ int fds2[2];
+ ASSERT_EQ(0, adb_socketpair(fds1));
+ ASSERT_EQ(0, adb_socketpair(fds2));
+
+ struct Test {
+ fdevent* fde1;
+ fdevent* fde2;
+ bool should_not_happen;
+ };
+ Test test{};
+
+ test.fde1 = fdevent_create(
+ fds1[0],
+ [](fdevent* fde, unsigned events, void* arg) {
+ auto test = static_cast<Test*>(arg);
+ // Unregister fde2 from inside the fde1 event
+ fdevent_destroy(test->fde2);
+ // Unregister fde1 so it doesn't get called again
+ fdevent_destroy(test->fde1);
+ },
+ &test);
+
+ test.fde2 = fdevent_create(
+ fds2[0],
+ [](fdevent* fde, unsigned events, void* arg) {
+ auto test = static_cast<Test*>(arg);
+ test->should_not_happen = true;
+ },
+ &test);
+
+ fdevent_add(test.fde1, FDE_READ | FDE_ERROR);
+ fdevent_add(test.fde2, FDE_READ | FDE_ERROR);
+
+ PrepareThread();
+ WaitForFdeventLoop();
+
+ std::mutex m;
+ std::condition_variable cv;
+ bool main_thread_latch = false;
+ bool looper_thread_latch = false;
+
+ fdevent_run_on_looper([&]() {
+ std::unique_lock lk(m);
+ // Notify the main thread that the looper is in this lambda
+ main_thread_latch = true;
+ cv.notify_one();
+ // Pause the looper to ensure both events occur in the same epoll_wait
+ cv.wait(lk, [&] { return looper_thread_latch; });
+ });
+
+ // Wait for the looper thread to pause to ensure it is not in epoll_wait
+ {
+ std::unique_lock lk(m);
+ cv.wait(lk, [&] { return main_thread_latch; });
+ }
+
+ // Write to one end of the sockets to trigger events on the other ends
+ adb_write(fds1[1], "a", 1);
+ adb_write(fds2[1], "a", 1);
+
+ // Unpause the looper thread to let it loop back into epoll_wait, which should return
+ // both fde1 and fde2.
+ {
+ std::lock_guard lk(m);
+ looper_thread_latch = true;
+ }
+ cv.notify_one();
+
+ WaitForFdeventLoop();
+ TerminateThread();
+
+ adb_close(fds1[0]);
+ adb_close(fds1[1]);
+ adb_close(fds2[0]);
+ adb_close(fds2[1]);
+
+ ASSERT_FALSE(test.should_not_happen);
+}
