Snap for 10453563 from c362e05a76d7238f684234fe4ad6768c0c461b8d to mainline-tzdata5-releaseaml_tz5_341510050aml_tz5_341510010aml_tz5_341510010

Change-Id: I7c8251be118ef8d3b4e3bb353f0eef526422f6c2

5 files changed, 120 insertions, 16 deletions

diff --git a/daemon/jdwp_service.cpp b/daemon/jdwp_service.cpp
index d03573fb..2501688c 100644
--- a/daemon/jdwp_service.cpp
+++ b/daemon/jdwp_service.cpp
@@ -459,12 +459,16 @@ static int jdwp_tracker_enqueue(asocket* s, apacket::payload_type) {
 }
 
 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 b0758b81..e20a2953 100644
--- a/fdevent/fdevent.cpp
+++ b/fdevent/fdevent.cpp
@@ -83,6 +83,7 @@ fdevent* fdevent_context::Create(unique_fd fd, std::variant<fd_func, fd_func2> f
         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 @@ unique_fd fdevent_context::Destroy(fdevent* fde) {
 
     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 @@ std::optional<std::chrono::milliseconds> fdevent_context::CalculatePollDuration(
 
 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 ff86f1fe..7f8e1a2c 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 @@ 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;
     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 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();
diff --git a/fdevent/fdevent_test.cpp b/fdevent/fdevent_test.cpp
index 20a82ea4..4c334061 100644
--- a/fdevent/fdevent_test.cpp
+++ b/fdevent/fdevent_test.cpp
@@ -315,3 +315,86 @@ TEST_F(FdeventTest, timeout) {
     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);
+}