From 06aff34a8f02d43ab963c04cdeeb3a0fffbee8c2 Mon Sep 17 00:00:00 2001
From: Michael Wright <michaelwr@google.com>
Date: Fri, 6 Aug 2021 20:05:33 +0100
Subject: [PATCH] Introduce new lock to avoid lock-ordering issues.

Because of the way DMD was designed, technically the class lock is below
the syncroot in the lock ordering, meaning we can't call into any
general display classes while holding the class lock without creating
the possibility of deadlock. There's no real reason to use the class
lock in this situation though, so just introduce a local lock to
SensorObserver that can be above the syncroot and thus call into the
rest of the display stack at will.

Bug: 195713475
Test: atest DisplayModeDirectorTest
Change-Id: Ie32660ae3a3935f0fd4001ca73fecba1e4bbba7d
---
 .../server/display/DisplayModeDirector.java   | 33 +++++++++++--------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/services/core/java/com/android/server/display/DisplayModeDirector.java b/services/core/java/com/android/server/display/DisplayModeDirector.java
index 186986e17c142..0d1b3d6f0ffd4 100644
--- a/services/core/java/com/android/server/display/DisplayModeDirector.java
+++ b/services/core/java/com/android/server/display/DisplayModeDirector.java
@@ -52,6 +52,7 @@ import android.view.Display;
 import android.view.DisplayInfo;
 
 import com.android.internal.R;
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.os.BackgroundThread;
 import com.android.server.LocalServices;
@@ -2127,7 +2128,7 @@ public class DisplayModeDirector {
         }
     }
 
-    private final class SensorObserver implements ProximityActiveListener,
+    private static final class SensorObserver implements ProximityActiveListener,
             DisplayManager.DisplayListener {
         private final String mProximitySensorName = null;
         private final String mProximitySensorType = Sensor.STRING_TYPE_PROXIMITY;
@@ -2135,22 +2136,24 @@ public class DisplayModeDirector {
         private final BallotBox mBallotBox;
         private final Context mContext;
         private final Injector mInjector;
+        @GuardedBy("mSensorObserverLock")
+        private final SparseBooleanArray mDozeStateByDisplay = new SparseBooleanArray();
+        private final Object mSensorObserverLock = new Object();
 
         private DisplayManager mDisplayManager;
         private DisplayManagerInternal mDisplayManagerInternal;
+        @GuardedBy("mSensorObserverLock")
         private boolean mIsProxActive = false;
-        private final SparseBooleanArray mDozeStateByDisplay;
 
         SensorObserver(Context context, BallotBox ballotBox, Injector injector) {
             mContext = context;
             mBallotBox = ballotBox;
             mInjector = injector;
-            mDozeStateByDisplay = new SparseBooleanArray();
         }
 
         @Override
         public void onProximityActive(boolean isActive) {
-            synchronized (mLock) {
+            synchronized (mSensorObserverLock) {
                 if (mIsProxActive != isActive) {
                     mIsProxActive = isActive;
                     recalculateVotesLocked();
@@ -2166,7 +2169,7 @@ public class DisplayModeDirector {
                     LocalServices.getService(SensorManagerInternal.class);
             sensorManager.addProximityActiveListener(BackgroundThread.getExecutor(), this);
 
-            synchronized (mLock) {
+            synchronized (mSensorObserverLock) {
                 for (Display d : mDisplayManager.getDisplays()) {
                     mDozeStateByDisplay.put(d.getDisplayId(), mInjector.isDozeState(d));
                 }
@@ -2196,19 +2199,21 @@ public class DisplayModeDirector {
 
         void dumpLocked(PrintWriter pw) {
             pw.println("  SensorObserver");
-            pw.println("    mIsProxActive=" + mIsProxActive);
-            pw.println("    mDozeStateByDisplay:");
-            for (int i = 0; i < mDozeStateByDisplay.size(); i++) {
-                final int id = mDozeStateByDisplay.keyAt(i);
-                final boolean dozed = mDozeStateByDisplay.valueAt(i);
-                pw.println("      " + id + " -> " + dozed);
+            synchronized (mSensorObserverLock) {
+                pw.println("    mIsProxActive=" + mIsProxActive);
+                pw.println("    mDozeStateByDisplay:");
+                for (int i = 0; i < mDozeStateByDisplay.size(); i++) {
+                    final int id = mDozeStateByDisplay.keyAt(i);
+                    final boolean dozed = mDozeStateByDisplay.valueAt(i);
+                    pw.println("      " + id + " -> " + dozed);
+                }
             }
         }
 
         @Override
         public void onDisplayAdded(int displayId) {
             boolean isDozeState = mInjector.isDozeState(mDisplayManager.getDisplay(displayId));
-            synchronized (mLock) {
+            synchronized (mSensorObserverLock) {
                 mDozeStateByDisplay.put(displayId, isDozeState);
                 recalculateVotesLocked();
             }
@@ -2217,7 +2222,7 @@ public class DisplayModeDirector {
         @Override
         public void onDisplayChanged(int displayId) {
             boolean wasDozeState = mDozeStateByDisplay.get(displayId);
-            synchronized (mLock) {
+            synchronized (mSensorObserverLock) {
                 mDozeStateByDisplay.put(displayId,
                         mInjector.isDozeState(mDisplayManager.getDisplay(displayId)));
                 if (wasDozeState != mDozeStateByDisplay.get(displayId)) {
@@ -2228,7 +2233,7 @@ public class DisplayModeDirector {
 
         @Override
         public void onDisplayRemoved(int displayId) {
-            synchronized (mLock) {
+            synchronized (mSensorObserverLock) {
                 mDozeStateByDisplay.delete(displayId);
                 recalculateVotesLocked();
             }