Skip to content

Commit 1de7384

Browse files
dvandercorpvishniakou
authored andcommitted
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 aea9781)
1 parent 8f654d8 commit 1de7384

File tree

4 files changed

+90
-8
lines changed

4 files changed

+90
-8
lines changed

init/Android.bp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,7 @@ cc_test {
387387

388388
srcs: [
389389
"devices_test.cpp",
390+
"epoll_test.cpp",
390391
"firmware_handler_test.cpp",
391392
"init_test.cpp",
392393
"keychords_test.cpp",

init/epoll.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,12 @@ Result<void> Epoll::Open() {
3838
return {};
3939
}
4040

41-
Result<void> Epoll::RegisterHandler(int fd, std::function<void()> handler, uint32_t events) {
41+
Result<void> Epoll::RegisterHandler(int fd, Handler handler, uint32_t events) {
4242
if (!events) {
4343
return Error() << "Must specify events";
4444
}
45-
auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(handler));
45+
auto sp = std::make_shared<decltype(handler)>(std::move(handler));
46+
auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(sp));
4647
if (!inserted) {
4748
return Error() << "Cannot specify two epoll handlers for a given FD";
4849
}
@@ -69,7 +70,7 @@ Result<void> Epoll::UnregisterHandler(int fd) {
6970
return {};
7071
}
7172

72-
Result<std::vector<std::function<void()>*>> Epoll::Wait(
73+
Result<std::vector<std::shared_ptr<Epoll::Handler>>> Epoll::Wait(
7374
std::optional<std::chrono::milliseconds> timeout) {
7475
int timeout_ms = -1;
7576
if (timeout && timeout->count() < INT_MAX) {
@@ -81,9 +82,10 @@ Result<std::vector<std::function<void()>*>> Epoll::Wait(
8182
if (num_events == -1) {
8283
return ErrnoError() << "epoll_wait failed";
8384
}
84-
std::vector<std::function<void()>*> pending_functions;
85+
std::vector<std::shared_ptr<Handler>> pending_functions;
8586
for (int i = 0; i < num_events; ++i) {
86-
pending_functions.emplace_back(reinterpret_cast<std::function<void()>*>(ev[i].data.ptr));
87+
auto sp = *reinterpret_cast<std::shared_ptr<Handler>*>(ev[i].data.ptr);
88+
pending_functions.emplace_back(std::move(sp));
8789
}
8890

8991
return pending_functions;

init/epoll.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <chrono>
2323
#include <functional>
2424
#include <map>
25+
#include <memory>
2526
#include <optional>
2627
#include <vector>
2728

@@ -36,15 +37,17 @@ class Epoll {
3637
public:
3738
Epoll();
3839

40+
typedef std::function<void()> Handler;
41+
3942
Result<void> Open();
40-
Result<void> RegisterHandler(int fd, std::function<void()> handler, uint32_t events = EPOLLIN);
43+
Result<void> RegisterHandler(int fd, Handler handler, uint32_t events = EPOLLIN);
4144
Result<void> UnregisterHandler(int fd);
42-
Result<std::vector<std::function<void()>*>> Wait(
45+
Result<std::vector<std::shared_ptr<Handler>>> Wait(
4346
std::optional<std::chrono::milliseconds> timeout);
4447

4548
private:
4649
android::base::unique_fd epoll_fd_;
47-
std::map<int, std::function<void()>> epoll_handlers_;
50+
std::map<int, std::shared_ptr<Handler>> epoll_handlers_;
4851
};
4952

5053
} // namespace init

init/epoll_test.cpp

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright (C) 2021 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include "epoll.h"
18+
19+
#include <sys/unistd.h>
20+
21+
#include <unordered_set>
22+
23+
#include <android-base/file.h>
24+
#include <gtest/gtest.h>
25+
26+
namespace android {
27+
namespace init {
28+
29+
std::unordered_set<void*> sValidObjects;
30+
31+
class CatchDtor final {
32+
public:
33+
CatchDtor() { sValidObjects.emplace(this); }
34+
CatchDtor(const CatchDtor&) { sValidObjects.emplace(this); }
35+
~CatchDtor() {
36+
auto iter = sValidObjects.find(this);
37+
if (iter != sValidObjects.end()) {
38+
sValidObjects.erase(iter);
39+
}
40+
}
41+
};
42+
43+
TEST(epoll, UnregisterHandler) {
44+
Epoll epoll;
45+
ASSERT_RESULT_OK(epoll.Open());
46+
47+
int fds[2];
48+
ASSERT_EQ(pipe(fds), 0);
49+
50+
CatchDtor catch_dtor;
51+
bool handler_invoked;
52+
auto handler = [&, catch_dtor]() -> void {
53+
auto result = epoll.UnregisterHandler(fds[0]);
54+
ASSERT_EQ(result.ok(), !handler_invoked);
55+
handler_invoked = true;
56+
ASSERT_NE(sValidObjects.find((void*)&catch_dtor), sValidObjects.end());
57+
};
58+
59+
epoll.RegisterHandler(fds[0], std::move(handler));
60+
61+
uint8_t byte = 0xee;
62+
ASSERT_TRUE(android::base::WriteFully(fds[1], &byte, sizeof(byte)));
63+
64+
auto results = epoll.Wait({});
65+
ASSERT_RESULT_OK(results);
66+
ASSERT_EQ(results->size(), size_t(1));
67+
68+
for (const auto& function : *results) {
69+
(*function)();
70+
(*function)();
71+
}
72+
ASSERT_TRUE(handler_invoked);
73+
}
74+
75+
} // namespace init
76+
} // namespace android

0 commit comments

Comments
 (0)