Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 630a51cd authored by Nick Chameyev's avatar Nick Chameyev
Browse files

Fix ArrayIndexOutOfBoundsException in ScreenTimeoutPolicyListenersContainer 

Guards access to mLastReportedState with mLock.

We had two usages without using mLock and on different threads:
- onCallbackDied on binder thread calls mLastReportedState.remove()
- notifyListenerIfNeeded reads/writes the map (on single thread), but as it could be also updated from a binder thread & it's not synchronized on a lock & the collection is not thread safe, we might get inconsistent state

Practically, I think right now this issue might likely happen only
when SystemUI crashes which causes onCallbackDied to be invoked
(SystemUI is the only current remote client, and we never call
 removeScreenTimeoutPolicyListener). The current bugreports contain SystemUI crash before this.

Also made mScreenTimeoutPolicy non-volatile as it is not needed anymore
(accessed only under mLock).

Test: atest NotifierTest
Flag: com.android.server.power.feature.flags.enable_screen_timeout_policy_listener_api
Bug: 414978378
Change-Id: Id32073b6e61c9104339e2fccc3006c5ff776e889
parent 06e23aca

services/core/java/com/android/server/power/Notifier.java

+30 −12
Original line number Diff line number Diff line
@@ -63,6 +63,7 @@ import android.util.SparseArray; 
import android.util.SparseBooleanArray;

import android.view.WindowManagerPolicyConstants;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.app.IBatteryStats;

import com.android.internal.logging.MetricsLogger;
@@ -1323,7 +1324,7 @@ public class Notifier { 
                mScreenTimeoutPolicyListeners.set(displayId, listenersContainer);

            }



            listenersContainer.addListener(listener);

            listenersContainer.addListenerLocked(listener);

        }

    }
@@ -1341,7 +1342,7 @@ public class Notifier { 
                return;

            }



            listenersContainer.removeListener(listener);

            listenersContainer.removeListenerLocked(listener);



            if (listenersContainer.isEmpty()) {

                mScreenTimeoutPolicyListeners.remove(displayId);
@@ -1373,7 +1374,7 @@ public class Notifier { 
                if (mDisplayManagerInternal.getGroupIdForDisplay(displayId) == displayGroupId) {

                    final ScreenTimeoutPolicyListenersContainer container =

                            mScreenTimeoutPolicyListeners.valueAt(idx);

                    container.updateScreenTimeoutPolicyAndNotifyIfNeeded(screenTimeoutPolicy);

                    container.updateScreenTimeoutPolicyAndNotifyIfNeededLocked(screenTimeoutPolicy);

                }

            }

        }
@@ -1381,23 +1382,29 @@ public class Notifier { 


    private final class ScreenTimeoutPolicyListenersContainer {

        private final RemoteCallbackList<IScreenTimeoutPolicyListener> mListeners;



        @GuardedBy("mLock")

        private final ArrayMap<IScreenTimeoutPolicyListener, Integer> mLastReportedState =

                new ArrayMap<>();



        @GuardedBy("mLock")

        @ScreenTimeoutPolicy

        private volatile int mScreenTimeoutPolicy;

        private int mScreenTimeoutPolicy;



        ScreenTimeoutPolicyListenersContainer(int screenTimeoutPolicy) {

            mScreenTimeoutPolicy = screenTimeoutPolicy;

            mListeners = new RemoteCallbackList<IScreenTimeoutPolicyListener>() {

                @Override

                public void onCallbackDied(IScreenTimeoutPolicyListener callbackInterface) {

                    synchronized (mLock) {

                        mLastReportedState.remove(callbackInterface);

                    }

                }

            };

        }



        void updateScreenTimeoutPolicyAndNotifyIfNeeded(

        @GuardedBy("mLock")

        void updateScreenTimeoutPolicyAndNotifyIfNeededLocked(

                @ScreenTimeoutPolicy int screenTimeoutPolicy) {

            mScreenTimeoutPolicy = screenTimeoutPolicy;
@@ -1410,12 +1417,14 @@ public class Notifier { 
            });

        }



        void addListener(IScreenTimeoutPolicyListener listener) {

        @GuardedBy("mLock")

        void addListenerLocked(IScreenTimeoutPolicyListener listener) {

            mListeners.register(listener);

            mHandler.post(() -> notifyListenerIfNeeded(listener));

        }



        void removeListener(IScreenTimeoutPolicyListener listener) {

        @GuardedBy("mLock")

        void removeListenerLocked(IScreenTimeoutPolicyListener listener) {

            mListeners.unregister(listener);

            mLastReportedState.remove(listener);

        }
@@ -1425,8 +1434,12 @@ public class Notifier { 
        }



        private void notifyListenerIfNeeded(IScreenTimeoutPolicyListener listener) {

            final int currentScreenTimeoutPolicy = mScreenTimeoutPolicy;

            final Integer reportedScreenTimeoutPolicy = mLastReportedState.get(listener);

            final int currentScreenTimeoutPolicy;

            final Integer reportedScreenTimeoutPolicy;

            synchronized (mLock) {

                currentScreenTimeoutPolicy = mScreenTimeoutPolicy;

                reportedScreenTimeoutPolicy = mLastReportedState.get(listener);

            }

            final boolean needsReporting = reportedScreenTimeoutPolicy == null

                    || !reportedScreenTimeoutPolicy.equals(currentScreenTimeoutPolicy);
@@ -1434,14 +1447,19 @@ public class Notifier { 


            try {

                listener.onScreenTimeoutPolicyChanged(currentScreenTimeoutPolicy);



                synchronized (mLock) {

                    mLastReportedState.put(listener, currentScreenTimeoutPolicy);

                }

            } catch (RemoteException e) {

                // The RemoteCallbackList will take care of removing

                // the dead object for us.

                Slog.e(TAG, "Remote exception when notifying screen timeout policy change", e);

            } catch (Throwable e) {

                Slog.e(TAG, "Exception when notifying screen timeout policy change", e);

                removeListener(listener);

                synchronized (mLock) {

                    removeListenerLocked(listener);

                }

            }

        }

    }