summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNicolo' Mazzucato <nicomazz@google.com>2023-01-09 08:04:05 +0000
committerAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-01-11 20:51:12 +0000
commit3c90e2ea398c31ad67ff2d883071804de5fe46bd (patch)
tree4e73cc58d4734630a05ab3ff46fab934dd8e9918
parent068ea566f5fe166fc0f805b5e9497b8c23f411e3 (diff)
downloadbase-3c90e2ea398c31ad67ff2d883071804de5fe46bd.tar.gz
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 (cherry picked from commit e4fccc2d9fd7382dacb4a546858eca93127c4c50) Merged-In: I165b260d60e03c8e4707c7704847c77a225fd43d
-rw-r--r--services/core/java/com/android/server/display/DisplayPowerController.java6
-rw-r--r--services/core/java/com/android/server/display/DisplayPowerState.java16
2 files changed, 8 insertions, 14 deletions
diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java
index d6e78a195be4..f88a3372a4ac 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 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
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 7d1396d7e413..2c257a17af91 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 @@ final class DisplayPowerState {
}
/**
- * 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 @@ final class DisplayPowerState {
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) {