Fix DisplayManager race condition when state is unknown
The race was happening when:
1. STATE_UNKNOWN arrives to PhotonicModulator#setState.
2. Inside PhotonicModulator#setState, mStateChangeInprogress is set to true, and the setState method finishes.
3. in the run loop, in one iteration, we reach mLock.wait() (with mStateChangeInprogress still set to true, as the state changed from something to unknown). Now, setState can be executed again as the lock is not held anymore.
4. A new PhotonicModulator#setState arrives. however, mStateChangeInProgress is true, so mLock.notifyAll() is not called anymore.
It was previously fixed by I903f6392cbe42031aab8ffdbfbe541463d6b5e01 making the display not going to UNKNOWN state anymore, but this cl fixes the general case and revert the changes to set the display to off in `onDisplayChanged`.
Bug: 262294651
Test: Running foldable test suite for 20 times with abtd: before the fix it had high flakiness, after it had 0% flakiness. There are no tests for DisplayPowerState currently.
Change-Id: I165b260d60e03c8e4707c7704847c77a225fd43d
diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java
index d6e78a1..f88a337 100644
--- a/services/core/java/com/android/server/display/DisplayPowerController.java
+++ b/services/core/java/com/android/server/display/DisplayPowerController.java
@@ -817,9 +817,9 @@
mDisplayDeviceConfig = config;
loadFromDisplayDeviceConfig(token, info);
- // Since the underlying display-device changed, we really don't know the
- // last command that was sent to change it's state. Lets assume it is off and we
- // trigger a change immediately.
+ /// Since the underlying display-device changed, we really don't know the
+ // last command that was sent to change it's state. Lets assume it is unknown so
+ // that we trigger a change immediately.
mPowerState.resetScreenState();
}
if (mIsEnabled != isEnabled || mIsInTransition != isInTransition) {
diff --git a/services/core/java/com/android/server/display/DisplayPowerState.java b/services/core/java/com/android/server/display/DisplayPowerState.java
index 7d1396d..2c257a1 100644
--- a/services/core/java/com/android/server/display/DisplayPowerState.java
+++ b/services/core/java/com/android/server/display/DisplayPowerState.java
@@ -340,20 +340,12 @@
}
/**
- * Resets the screen state to {@link Display#STATE_OFF}. Even though we do not know the last
- * state that was sent to the underlying display-device, we assume it is off.
- *
- * We do not set the screen state to {@link Display#STATE_UNKNOWN} to avoid getting in the state
- * where PhotonicModulator holds onto the lock. This happens because we currently try to keep
- * the mScreenState and mPendingState in sync, however if the screenState is set to
- * {@link Display#STATE_UNKNOWN} here, mPendingState will get progressed to this, which will
- * force the PhotonicModulator thread to wait onto the lock to take it out of that state.
- * b/262294651 for more info.
+ * Resets the screen state to unknown. Useful when the underlying display-device changes for the
+ * LogicalDisplay and we do not know the last state that was sent to it.
*/
void resetScreenState() {
- mScreenState = Display.STATE_OFF;
+ mScreenState = Display.STATE_UNKNOWN;
mScreenReady = false;
- scheduleScreenUpdate();
}
private void scheduleScreenUpdate() {
@@ -514,6 +506,8 @@
boolean valid = state != Display.STATE_UNKNOWN && !Float.isNaN(brightnessState);
boolean changed = stateChanged || backlightChanged;
if (!valid || !changed) {
+ mStateChangeInProgress = false;
+ mBacklightChangeInProgress = false;
try {
mLock.wait();
} catch (InterruptedException ex) {