Fixing threading around mConnections in Scheduler
The following happens when we create connection
1. initScheduler()
2. getFactory().createScheduler()
3. mTouchTimer.emplace
4. mScheduler->createConnection
Which means that timer created in step 3 expires exactly
while we are in step 4, so then the EventThread is half baked
in rare cases and we crash.
Also, do not continue to calling functions if the
optional fields do not have a value.
Test: libsurfaceflinger_unittest --gtest_filter=SchedulerTest.testDispatchCachedReportedConfig
Bug: 160926398
Change-Id: Ib2617b914145bc4180cc7ca27203c59dbd625c94
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 35619a3..eee9400 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -175,4 +175,25 @@
mScheduler.chooseRefreshRateForContent();
}
+TEST_F(SchedulerTest, testDispatchCachedReportedConfig) {
+ // If the optional fields are cleared, the function should return before
+ // onConfigChange is called.
+ mScheduler.clearOptionalFieldsInFeatures();
+ EXPECT_NO_FATAL_FAILURE(mScheduler.dispatchCachedReportedConfig());
+ EXPECT_CALL(*mEventThread, onConfigChanged(_, _, _)).Times(0);
+}
+
+TEST_F(SchedulerTest, onNonPrimaryDisplayConfigChanged_invalidParameters) {
+ HwcConfigIndexType configId = HwcConfigIndexType(111);
+ nsecs_t vsyncPeriod = 111111;
+
+ // If the handle is incorrect, the function should return before
+ // onConfigChange is called.
+ Scheduler::ConnectionHandle invalidHandle = {.id = 123};
+ EXPECT_NO_FATAL_FAILURE(mScheduler.onNonPrimaryDisplayConfigChanged(invalidHandle,
+ PHYSICAL_DISPLAY_ID,
+ configId, vsyncPeriod));
+ EXPECT_CALL(*mEventThread, onConfigChanged(_, _, _)).Times(0);
+}
+
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index db3e0bd..a9d9dc0 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -16,6 +16,7 @@
#pragma once
+#include <Scheduler/Scheduler.h>
#include <gmock/gmock.h>
#include <gui/ISurfaceComposer.h>
@@ -93,6 +94,22 @@
return mFeatures.touch == Scheduler::TouchState::Active;
}
+ void dispatchCachedReportedConfig() {
+ std::lock_guard<std::mutex> lock(mFeatureStateLock);
+ return Scheduler::dispatchCachedReportedConfig();
+ }
+
+ void clearOptionalFieldsInFeatures() {
+ std::lock_guard<std::mutex> lock(mFeatureStateLock);
+ mFeatures.cachedConfigChangedParams.reset();
+ }
+
+ void onNonPrimaryDisplayConfigChanged(ConnectionHandle handle, PhysicalDisplayId displayId,
+ HwcConfigIndexType configId, nsecs_t vsyncPeriod) {
+ return Scheduler::onNonPrimaryDisplayConfigChanged(handle, displayId, configId,
+ vsyncPeriod);
+ }
+
~TestableScheduler() {
// All these pointer and container clears help ensure that GMock does
// not report a leaked object, since the Scheduler instance may