SF: call the timer's callback in case of an error
Bug: 210065990
Test: new SF unit test
Change-Id: I4ae70e82e9402fc7d7eecc1a17c1b992eb3efab6
diff --git a/services/surfaceflinger/Scheduler/Timer.cpp b/services/surfaceflinger/Scheduler/Timer.cpp
index 22c3a70..68f9321 100644
--- a/services/surfaceflinger/Scheduler/Timer.cpp
+++ b/services/surfaceflinger/Scheduler/Timer.cpp
@@ -51,13 +51,25 @@
}
void Timer::reset() {
- cleanup();
- mTimerFd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK);
- mEpollFd = epoll_create1(EPOLL_CLOEXEC);
- if (pipe2(mPipes.data(), O_CLOEXEC | O_NONBLOCK)) {
- ALOGE("could not create TimerDispatch mPipes");
- return;
- };
+ std::function<void()> cb;
+ {
+ std::lock_guard lock(mMutex);
+ if (mExpectingCallback && mCallback) {
+ cb = mCallback;
+ }
+
+ cleanup();
+ mTimerFd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK);
+ mEpollFd = epoll_create1(EPOLL_CLOEXEC);
+ if (pipe2(mPipes.data(), O_CLOEXEC | O_NONBLOCK)) {
+ ALOGE("could not create TimerDispatch mPipes");
+ }
+ }
+ if (cb) {
+ setDebugState(DebugState::InCallback);
+ cb();
+ setDebugState(DebugState::Running);
+ }
setDebugState(DebugState::Reset);
}
@@ -81,6 +93,8 @@
close(mPipes[kWritePipe]);
mPipes[kWritePipe] = -1;
}
+ mExpectingCallback = false;
+ mCallback = {};
}
void Timer::endDispatch() {
@@ -99,6 +113,7 @@
std::chrono::duration_cast<std::chrono::nanoseconds>(1s).count();
mCallback = cb;
+ mExpectingCallback = true;
struct itimerspec old_timer;
struct itimerspec new_timer {
@@ -198,6 +213,7 @@
{
std::lock_guard lock(mMutex);
cb = mCallback;
+ mExpectingCallback = false;
}
if (cb) {
setDebugState(DebugState::InCallback);
diff --git a/services/surfaceflinger/Scheduler/Timer.h b/services/surfaceflinger/Scheduler/Timer.h
index 628d800..eb65954 100644
--- a/services/surfaceflinger/Scheduler/Timer.h
+++ b/services/surfaceflinger/Scheduler/Timer.h
@@ -36,6 +36,10 @@
void alarmCancel() final;
void dump(std::string& result) const final;
+protected:
+ // For unit testing
+ int mEpollFd = -1;
+
private:
enum class DebugState {
Reset,
@@ -48,12 +52,12 @@
ftl_last = Terminated
};
- void reset();
- void cleanup();
+ void reset() EXCLUDES(mMutex);
+ void cleanup() REQUIRES(mMutex);
void setDebugState(DebugState state) EXCLUDES(mMutex);
int mTimerFd = -1;
- int mEpollFd = -1;
+
std::array<int, 2> mPipes = {-1, -1};
std::thread mDispatchThread;
@@ -63,6 +67,7 @@
mutable std::mutex mMutex;
std::function<void()> mCallback GUARDED_BY(mMutex);
+ bool mExpectingCallback GUARDED_BY(mMutex) = false;
DebugState mDebugState GUARDED_BY(mMutex);
};
diff --git a/services/surfaceflinger/tests/unittests/TimerTest.cpp b/services/surfaceflinger/tests/unittests/TimerTest.cpp
index cda6bbf..0a3639d 100644
--- a/services/surfaceflinger/tests/unittests/TimerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimerTest.cpp
@@ -26,11 +26,19 @@
namespace android::scheduler {
+struct TestableTimer : public Timer {
+public:
+ void makeEpollError() {
+ // close the epoll file descriptor to cause an epoll error
+ close(mEpollFd);
+ }
+};
+
struct TimerTest : testing::Test {
static constexpr int mIterations = 20;
AsyncCallRecorder<void (*)()> mCallbackRecorder;
- Timer mTimer;
+ TestableTimer mTimer;
void timerCallback() { mCallbackRecorder.recordCall(); }
};
@@ -42,4 +50,14 @@
EXPECT_FALSE(mCallbackRecorder.waitForUnexpectedCall().has_value());
}
}
+
+TEST_F(TimerTest, recoversAfterEpollError) {
+ for (int i = 0; i < mIterations; i++) {
+ mTimer.makeEpollError();
+ mTimer.alarmAt(std::bind(&TimerTest::timerCallback, this), systemTime() - 10'000'00);
+ EXPECT_TRUE(mCallbackRecorder.waitForCall().has_value());
+ EXPECT_FALSE(mCallbackRecorder.waitForUnexpectedCall().has_value());
+ }
+}
+
} // namespace android::scheduler