SF: Fix deadlock while stopping idle timer
scope_lock `mRefreshRateConfigsLock` is acquired
during `Scheduler::setRefreshRateConfigs` to
stop & clear the existing idle timer callback.
And in the meantime, when the code in
`OneShotTimer::loop` encounters the timeout
and then timeoutcallback is requested from
OneShotTimer::loop, this callback in
`Scheduler::kernelIdleTimerCallback`
will try to acquire the lock on
`mRefreshRateConfigsLock`.
The OneShotTimer::stop which is called to reset
the idle timer from the
`Scheduler::setRefreshRateConfigs` now join on
the OneShotTimer::loop thread that is now blocked
waiting on the `mRefreshRateConfigsLock`. So this
call never finishes and deadlock happens.
When we use the fakeGuard to lock `mRefreshRateConfigsLock`
we fake the lock and stop the idle timer. This is okay to do
as we only update mRefreshRateConfig on the main thread.
Test: atest libsurfaceflinger_unittest
go/wm-smoke
BUG: 233423207
Change-Id: I146325193b57d81bc3423426355791ca0ec962f5
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 37f0fec..c9840c7 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -25,6 +25,7 @@
#include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h>
#include <android/hardware/configstore/1.1/ISurfaceFlingerConfigs.h>
#include <configstore/Utils.h>
+#include <ftl/fake_guard.h>
#include <gui/WindowInfo.h>
#include <system/window.h>
#include <ui/DisplayStatInfo.h>
@@ -94,9 +95,13 @@
}
void Scheduler::setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs> configs) {
+ // The current RefreshRateConfigs instance may outlive this call, so unbind its idle timer.
{
- // The current RefreshRateConfigs instance may outlive this call, so unbind its idle timer.
- std::scoped_lock lock(mRefreshRateConfigsLock);
+ // mRefreshRateConfigsLock is not locked here to avoid the deadlock
+ // as the callback can attempt to acquire the lock before stopIdleTimer can finish
+ // the execution. It's safe to FakeGuard as main thread is the only thread that
+ // writes to the mRefreshRateConfigs.
+ ftl::FakeGuard guard(mRefreshRateConfigsLock);
if (mRefreshRateConfigs) {
mRefreshRateConfigs->stopIdleTimer();
mRefreshRateConfigs->clearIdleTimerCallbacks();