Merge "Fixing threading around mConnections in Scheduler"
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 9c145cc..7b8448f 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -229,6 +229,7 @@
auto connection =
createConnectionInternal(eventThread.get(), ISurfaceComposer::eConfigChangedSuppress);
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
mConnections.emplace(handle, Connection{connection, std::move(eventThread)});
return handle;
}
@@ -240,29 +241,47 @@
sp<IDisplayEventConnection> Scheduler::createDisplayEventConnection(
ConnectionHandle handle, ISurfaceComposer::ConfigChanged configChanged) {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
RETURN_IF_INVALID_HANDLE(handle, nullptr);
return createConnectionInternal(mConnections[handle].thread.get(), configChanged);
}
sp<EventThreadConnection> Scheduler::getEventConnection(ConnectionHandle handle) {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
RETURN_IF_INVALID_HANDLE(handle, nullptr);
return mConnections[handle].connection;
}
void Scheduler::onHotplugReceived(ConnectionHandle handle, PhysicalDisplayId displayId,
bool connected) {
- RETURN_IF_INVALID_HANDLE(handle);
- mConnections[handle].thread->onHotplugReceived(displayId, connected);
+ android::EventThread* thread;
+ {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
+ RETURN_IF_INVALID_HANDLE(handle);
+ thread = mConnections[handle].thread.get();
+ }
+
+ thread->onHotplugReceived(displayId, connected);
}
void Scheduler::onScreenAcquired(ConnectionHandle handle) {
- RETURN_IF_INVALID_HANDLE(handle);
- mConnections[handle].thread->onScreenAcquired();
+ android::EventThread* thread;
+ {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
+ RETURN_IF_INVALID_HANDLE(handle);
+ thread = mConnections[handle].thread.get();
+ }
+ thread->onScreenAcquired();
}
void Scheduler::onScreenReleased(ConnectionHandle handle) {
- RETURN_IF_INVALID_HANDLE(handle);
- mConnections[handle].thread->onScreenReleased();
+ android::EventThread* thread;
+ {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
+ RETURN_IF_INVALID_HANDLE(handle);
+ thread = mConnections[handle].thread.get();
+ }
+ thread->onScreenReleased();
}
void Scheduler::onPrimaryDisplayConfigChanged(ConnectionHandle handle, PhysicalDisplayId displayId,
@@ -274,6 +293,16 @@
}
void Scheduler::dispatchCachedReportedConfig() {
+ // Check optional fields first.
+ if (!mFeatures.configId.has_value()) {
+ ALOGW("No config ID found, not dispatching cached config.");
+ return;
+ }
+ if (!mFeatures.cachedConfigChangedParams.has_value()) {
+ ALOGW("No config changed params found, not dispatching cached config.");
+ return;
+ }
+
const auto configId = *mFeatures.configId;
const auto vsyncPeriod =
mRefreshRateConfigs.getRefreshRateFromConfigId(configId).getVsyncPeriod();
@@ -295,24 +324,40 @@
void Scheduler::onNonPrimaryDisplayConfigChanged(ConnectionHandle handle,
PhysicalDisplayId displayId,
HwcConfigIndexType configId, nsecs_t vsyncPeriod) {
- RETURN_IF_INVALID_HANDLE(handle);
- mConnections[handle].thread->onConfigChanged(displayId, configId, vsyncPeriod);
+ android::EventThread* thread;
+ {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
+ RETURN_IF_INVALID_HANDLE(handle);
+ thread = mConnections[handle].thread.get();
+ }
+ thread->onConfigChanged(displayId, configId, vsyncPeriod);
}
size_t Scheduler::getEventThreadConnectionCount(ConnectionHandle handle) {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
RETURN_IF_INVALID_HANDLE(handle, 0);
return mConnections[handle].thread->getEventThreadConnectionCount();
}
void Scheduler::dump(ConnectionHandle handle, std::string& result) const {
- RETURN_IF_INVALID_HANDLE(handle);
- mConnections.at(handle).thread->dump(result);
+ android::EventThread* thread;
+ {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
+ RETURN_IF_INVALID_HANDLE(handle);
+ thread = mConnections.at(handle).thread.get();
+ }
+ thread->dump(result);
}
void Scheduler::setDuration(ConnectionHandle handle, std::chrono::nanoseconds workDuration,
std::chrono::nanoseconds readyDuration) {
- RETURN_IF_INVALID_HANDLE(handle);
- mConnections[handle].thread->setDuration(workDuration, readyDuration);
+ android::EventThread* thread;
+ {
+ std::lock_guard<std::mutex> lock(mConnectionsLock);
+ RETURN_IF_INVALID_HANDLE(handle);
+ thread = mConnections[handle].thread.get();
+ }
+ thread->setDuration(workDuration, readyDuration);
}
void Scheduler::getDisplayStatInfo(DisplayStatInfo* stats, nsecs_t now) {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index da25f5c..3ecda58 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -228,7 +228,8 @@
};
ConnectionHandle::Id mNextConnectionHandleId = 0;
- std::unordered_map<ConnectionHandle, Connection> mConnections;
+ mutable std::mutex mConnectionsLock;
+ std::unordered_map<ConnectionHandle, Connection> mConnections GUARDED_BY(mConnectionsLock);
bool mInjectVSyncs = false;
InjectVSyncSource* mVSyncInjector = nullptr;
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