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

Commit 10cd45d2 authored by Rupesh Bansal's avatar Rupesh Bansal
Browse files

Clone and pass worksource to the Notifier 

This addresses the edge case scenario where multiple subsequent updates to the
worksource of a wakelock can change the ordering of the values processed
by the Notifier thread to update the BatteryStatsService. This could
ultimately lead to cases where BatteryStatsService receives the wakelock
stop notifications for uids whose start was never recorded.

Bug: 354303438
Test: Manual
Test: atest NotifierTest
Flag: EXEMPT Bugfix
Change-Id: I95b9fecdc67fc808d46c696edf958b9dbe9b49ef
parent 502cec5a

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

+7 −1
Original line number Diff line number Diff line
@@ -360,7 +360,13 @@ public class Notifier { 
            IWakeLockCallback callback, int newFlags, String newTag, String newPackageName,

            int newOwnerUid, int newOwnerPid, WorkSource newWorkSource, String newHistoryTag,

            IWakeLockCallback newCallback) {



        // Todo(b/359154665): We do this because the newWorkSource can potentially be updated

        // before the request is processed on the notifier thread. This would generally happen is

        // the Worksource's set method is called, which as of this comment happens only in

        // PowerManager#setWorksource and WifiManager#WifiLock#setWorksource. Both these places

        // need to be updated and the WorkSource#set should be deprecated to avoid falling into

        // such traps

        newWorkSource = (newWorkSource == null) ? null : new WorkSource(newWorkSource);

        final int monitorType = getBatteryStatsWakeLockMonitorType(flags);

        final int newMonitorType = getBatteryStatsWakeLockMonitorType(newFlags);

        if (workSource != null && newWorkSource != null

services/tests/powerservicetests/src/com/android/server/power/NotifierTest.java

+36 −3
Original line number Diff line number Diff line
@@ -61,7 +61,6 @@ import com.android.server.statusbar.StatusBarManagerInternal; 
import org.junit.Before;

import org.junit.Test;

import org.mockito.Mock;

import org.mockito.Mockito;

import org.mockito.MockitoAnnotations;



import java.util.concurrent.Executor;
@@ -264,8 +263,8 @@ public class NotifierTest { 
                BatteryStats.WAKE_TYPE_PARTIAL, false);



        verifyNoMoreInteractions(mWakeLockLog, mBatteryStats);

        WorkSource worksourceOld = Mockito.mock(WorkSource.class);

        WorkSource worksourceNew = Mockito.mock(WorkSource.class);

        WorkSource worksourceOld = new WorkSource(/*uid=*/ 1);

        WorkSource worksourceNew = new WorkSource(/*uid=*/ 2);



        mNotifier.onWakeLockChanging(PowerManager.PARTIAL_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, worksourceOld, /* historyTag= */ null,
@@ -309,6 +308,40 @@ public class NotifierTest { 
        verify(mWakeLockLog).onWakeLockReleased("wakelockTag", uid, -1);

    }



    @Test

    public void

            test_notifierProcessesWorkSourceDeepCopy_OnWakelockChanging() throws RemoteException {

        when(mPowerManagerFlags.improveWakelockLatency()).thenReturn(true);

        createNotifier();

        clearInvocations(mWakeLockLog, mBatteryStats, mAppOpsManager);

        IWakeLockCallback exceptingCallback = new IWakeLockCallback.Stub() {

            @Override public void onStateChanged(boolean enabled) throws RemoteException {

                throw new RemoteException("Just testing");

            }

        };



        final int uid = 1234;

        final int pid = 5678;

        mTestLooper.dispatchAll();

        WorkSource worksourceOld = new WorkSource(/*uid=*/ 1);

        WorkSource worksourceNew =  new WorkSource(/*uid=*/ 2);



        mNotifier.onWakeLockChanging(PowerManager.PARTIAL_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, worksourceOld, /* historyTag= */ null,

                exceptingCallback,

                PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, worksourceNew, /* newHistoryTag= */ null,

                exceptingCallback);

        // The newWorksource is modified before notifier could process it.

        worksourceNew.set(/*uid=*/ 3);



        mTestLooper.dispatchAll();

        verify(mBatteryStats).noteChangeWakelockFromSource(worksourceOld, pid,

                "wakelockTag", null, BatteryStats.WAKE_TYPE_PARTIAL,

                new WorkSource(/*uid=*/ 2), pid, "wakelockTag", null,

                BatteryStats.WAKE_TYPE_FULL, false);

    }





    @Test

    public void testOnWakeLockListener_FullWakeLock_ProcessesOnHandler() throws RemoteException {