From 9d567a7da8bb5e4c5f1412d365a076189d331c2f Mon Sep 17 00:00:00 2001 From: Oleg Petsjonkin Date: Tue, 7 Mar 2023 13:51:27 +0000 Subject: [PATCH] Fixing race condition between deleting display and updating votes Bug: b/271973770 Test: atest SkinThermalStatusObserverTest + test in bug descritpion Change-Id: I0adfb8505040a627193d36dd9fd13c0b42e1f535 --- .../mode/SkinThermalStatusObserver.java | 8 ++++ .../mode/SkinThermalStatusObserverTest.java | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/services/core/java/com/android/server/display/mode/SkinThermalStatusObserver.java b/services/core/java/com/android/server/display/mode/SkinThermalStatusObserver.java index 1bb34abe90256..f93d9ee55d30d 100644 --- a/services/core/java/com/android/server/display/mode/SkinThermalStatusObserver.java +++ b/services/core/java/com/android/server/display/mode/SkinThermalStatusObserver.java @@ -84,7 +84,11 @@ final class SkinThermalStatusObserver extends IThermalEventListener.Stub impleme @Override public void notifyThrottling(Temperature temp) { @Temperature.ThrottlingStatus int currentStatus = temp.getStatus(); + synchronized (mThermalObserverLock) { + if (mStatus == currentStatus) { + return; // status not changed, skip update + } mStatus = currentStatus; mHandler.post(this::updateVotes); } @@ -188,6 +192,10 @@ final class SkinThermalStatusObserver extends IThermalEventListener.Stub impleme localStatus = mStatus; localMap = mThermalThrottlingByDisplay.get(displayId); } + if (localMap == null) { + Slog.d(TAG, "Updating votes, display already removed, display=" + displayId); + return; + } if (mLoggingEnabled) { Slog.d(TAG, "Updating votes for status=" + localStatus + ", display =" + displayId + ", map=" + localMap); diff --git a/services/tests/servicestests/src/com/android/server/display/mode/SkinThermalStatusObserverTest.java b/services/tests/servicestests/src/com/android/server/display/mode/SkinThermalStatusObserverTest.java index dd0cd965b8a0e..fd1889ca09821 100644 --- a/services/tests/servicestests/src/com/android/server/display/mode/SkinThermalStatusObserverTest.java +++ b/services/tests/servicestests/src/com/android/server/display/mode/SkinThermalStatusObserverTest.java @@ -18,6 +18,7 @@ package com.android.server.display.mode; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import android.hardware.display.DisplayManager; @@ -49,6 +50,7 @@ public class SkinThermalStatusObserverTest { private static final float FLOAT_TOLERANCE = 0.01f; private static final int DISPLAY_ID = 1; private static final int DISPLAY_ID_OTHER = 2; + private static final int DISPLAY_ID_ADDED = 3; SkinThermalStatusObserver mObserver; @@ -167,6 +169,42 @@ public class SkinThermalStatusObserverTest { assertEquals(120, vote.refreshRateRanges.render.max, FLOAT_TOLERANCE); } + @Test + public void testDisplayAdded() { + // GIVEN 2 displays with no thermalThrottling config AND temperature level CRITICAL + mObserver.observe(); + mObserver.notifyThrottling(createTemperature(Temperature.THROTTLING_CRITICAL)); + // WHEN new display is added + mObserver.onDisplayAdded(DISPLAY_ID_ADDED); + mHandler.flush(); + // THEN 3rd vote is added to storage with (0,60) render refresh rate(default behaviour) + assertEquals(3, mStorage.mVoteRegistry.size()); + + SparseArray displayVotes = mStorage.mVoteRegistry.get( + DISPLAY_ID_ADDED); + assertEquals(1, displayVotes.size()); + + DisplayModeDirector.Vote vote = displayVotes.get( + DisplayModeDirector.Vote.PRIORITY_SKIN_TEMPERATURE); + assertEquals(0, vote.refreshRateRanges.render.min, FLOAT_TOLERANCE); + assertEquals(60, vote.refreshRateRanges.render.max, FLOAT_TOLERANCE); + } + + @Test + public void testDisplayAddedAndThenImmediatelyRemoved() { + // GIVEN 2 displays with no thermalThrottling config AND temperature level CRITICAL + mObserver.observe(); + mObserver.notifyThrottling(createTemperature(Temperature.THROTTLING_CRITICAL)); + // WHEN new display is added and immediately removed + mObserver.onDisplayAdded(DISPLAY_ID_ADDED); + mObserver.onDisplayRemoved(DISPLAY_ID_ADDED); + mHandler.flush(); + // THEN there are 2 votes in registry + assertEquals(2, mStorage.mVoteRegistry.size()); + assertNotNull(mStorage.mVoteRegistry.get(DISPLAY_ID)); + assertNotNull(mStorage.mVoteRegistry.get(DISPLAY_ID_OTHER)); + } + private static Temperature createTemperature(@Temperature.ThrottlingStatus int status) { return new Temperature(40.0f, Temperature.TYPE_SKIN, "test_temp", status); }