SF: Fix deadlock during display hotplug
I30ec756f134d2d67a70ac8797008dc792eac035e laid the foundation to remove
the offending lock, but declared victory prematurely.
Fixes: 329450361
Flag: EXEMPT bugfix
Test: presubmit
Change-Id: Iaae02d95e175e55f52f65da1481b64f1f533a04c
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d00e762..6f30eb2 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4259,6 +4259,11 @@
getHwComposer().setVsyncEnabled(displayId, enable ? hal::Vsync::ENABLE : hal::Vsync::DISABLE);
}
+// This callback originates from Scheduler::applyPolicy, whose thread context may be the main thread
+// (via Scheduler::chooseRefreshRateForContent) or a OneShotTimer thread. The latter case imposes a
+// deadlock prevention rule: If the main thread is processing hotplug, then mStateLock is locked as
+// the main thread stops the OneShotTimer and joins with its thread. Hence, the OneShotTimer thread
+// must not lock mStateLock in this callback, which would deadlock with the join.
void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest> modeRequests) {
if (mBootStage != BootStage::FINISHED) {
ALOGV("Currently in the boot stage, skipping display mode changes");
@@ -4267,21 +4272,14 @@
SFTRACE_CALL();
- // If this is called from the main thread mStateLock must be locked before
- // Currently the only way to call this function from the main thread is from
- // Scheduler::chooseRefreshRateForContent
-
- ConditionalLock lock(mStateLock, std::this_thread::get_id() != mMainThreadId);
-
for (auto& request : modeRequests) {
const auto& modePtr = request.mode.modePtr;
-
const auto displayId = modePtr->getPhysicalDisplayId();
- const auto display = getDisplayDeviceLocked(displayId);
- if (!display) continue;
+ const auto selectorPtr = mDisplayModeController.selectorPtrFor(displayId);
+ if (!selectorPtr) continue;
- if (display->refreshRateSelector().isModeAllowed(request.mode)) {
+ if (selectorPtr->isModeAllowed(request.mode)) {
setDesiredMode(std::move(request));
} else {
ALOGV("%s: Mode %d is disallowed for display %s", __func__,
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 8242844..65bfce2 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -733,7 +733,7 @@
// Show hdr sdr ratio overlay
bool mHdrSdrRatioOverlay = false;
- void setDesiredMode(display::DisplayModeRequest&&) REQUIRES(mStateLock);
+ void setDesiredMode(display::DisplayModeRequest&&);
status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps,
Fps maxFps);