From 1de7384c109a318c0b11bbb98bac50a10b093947 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 13 May 2021 20:18:14 -0700 Subject: [PATCH] Use std::shared_ptr in Epoll's callback list. Ignore-AOSP-First: Awaiting security triage Bug: 187862380 Bug: 190126442 Test: CtsInitTestCases Change-Id: Ibb34a6b8a5675dbc515b7f8a43d7eecf2084510c (cherry picked from commit aea97815308ab98faf1599c16d6190b787d34941) --- init/Android.bp | 1 + init/epoll.cpp | 12 ++++--- init/epoll.h | 9 ++++-- init/epoll_test.cpp | 76 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 init/epoll_test.cpp diff --git a/init/Android.bp b/init/Android.bp index 98e62fe0e..7eeafa24b 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -387,6 +387,7 @@ cc_test { srcs: [ "devices_test.cpp", + "epoll_test.cpp", "firmware_handler_test.cpp", "init_test.cpp", "keychords_test.cpp", diff --git a/init/epoll.cpp b/init/epoll.cpp index 17d63fa5d..74d8aac96 100644 --- a/init/epoll.cpp +++ b/init/epoll.cpp @@ -38,11 +38,12 @@ Result Epoll::Open() { return {}; } -Result Epoll::RegisterHandler(int fd, std::function handler, uint32_t events) { +Result Epoll::RegisterHandler(int fd, Handler handler, uint32_t events) { if (!events) { return Error() << "Must specify events"; } - auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(handler)); + auto sp = std::make_shared(std::move(handler)); + auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(sp)); if (!inserted) { return Error() << "Cannot specify two epoll handlers for a given FD"; } @@ -69,7 +70,7 @@ Result Epoll::UnregisterHandler(int fd) { return {}; } -Result*>> Epoll::Wait( +Result>> Epoll::Wait( std::optional timeout) { int timeout_ms = -1; if (timeout && timeout->count() < INT_MAX) { @@ -81,9 +82,10 @@ Result*>> Epoll::Wait( if (num_events == -1) { return ErrnoError() << "epoll_wait failed"; } - std::vector*> pending_functions; + std::vector> pending_functions; for (int i = 0; i < num_events; ++i) { - pending_functions.emplace_back(reinterpret_cast*>(ev[i].data.ptr)); + auto sp = *reinterpret_cast*>(ev[i].data.ptr); + pending_functions.emplace_back(std::move(sp)); } return pending_functions; diff --git a/init/epoll.h b/init/epoll.h index c32a6614f..0df528935 100644 --- a/init/epoll.h +++ b/init/epoll.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -36,15 +37,17 @@ class Epoll { public: Epoll(); + typedef std::function Handler; + Result Open(); - Result RegisterHandler(int fd, std::function handler, uint32_t events = EPOLLIN); + Result RegisterHandler(int fd, Handler handler, uint32_t events = EPOLLIN); Result UnregisterHandler(int fd); - Result*>> Wait( + Result>> Wait( std::optional timeout); private: android::base::unique_fd epoll_fd_; - std::map> epoll_handlers_; + std::map> epoll_handlers_; }; } // namespace init diff --git a/init/epoll_test.cpp b/init/epoll_test.cpp new file mode 100644 index 000000000..9236cd53e --- /dev/null +++ b/init/epoll_test.cpp @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "epoll.h" + +#include + +#include + +#include +#include + +namespace android { +namespace init { + +std::unordered_set sValidObjects; + +class CatchDtor final { + public: + CatchDtor() { sValidObjects.emplace(this); } + CatchDtor(const CatchDtor&) { sValidObjects.emplace(this); } + ~CatchDtor() { + auto iter = sValidObjects.find(this); + if (iter != sValidObjects.end()) { + sValidObjects.erase(iter); + } + } +}; + +TEST(epoll, UnregisterHandler) { + Epoll epoll; + ASSERT_RESULT_OK(epoll.Open()); + + int fds[2]; + ASSERT_EQ(pipe(fds), 0); + + CatchDtor catch_dtor; + bool handler_invoked; + auto handler = [&, catch_dtor]() -> void { + auto result = epoll.UnregisterHandler(fds[0]); + ASSERT_EQ(result.ok(), !handler_invoked); + handler_invoked = true; + ASSERT_NE(sValidObjects.find((void*)&catch_dtor), sValidObjects.end()); + }; + + epoll.RegisterHandler(fds[0], std::move(handler)); + + uint8_t byte = 0xee; + ASSERT_TRUE(android::base::WriteFully(fds[1], &byte, sizeof(byte))); + + auto results = epoll.Wait({}); + ASSERT_RESULT_OK(results); + ASSERT_EQ(results->size(), size_t(1)); + + for (const auto& function : *results) { + (*function)(); + (*function)(); + } + ASSERT_TRUE(handler_invoked); +} + +} // namespace init +} // namespace android