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

Commit 8b6a9987 authored by Suprabh Shukla's avatar Suprabh Shukla
Browse files

Add a delay to the removal of cached listener alarms 

Sometimes a uid may quickly transition between process-states. To allow
for this without causing severe breakages, alarm maanger will wait for
a short duration before removing its exact listener alarms. If the uid
gets uncached during this time, the removal will be cancelled.

To start with, this delay is equal to the default freezer timeout but
this is left to be configurable via device_config.

Test: atest FrameworksMockingServicesTests:AppStateTrackerTest
Test: atest FrameworksMockingServicesTests:AlarmManagerServiceTest

Bug: 265195908
Change-Id: Ia1e57bf5163a0d543bd767ef5a44301331e7c48d
parent c740917d

apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java

+5 −7
Original line number Diff line number Diff line
@@ -429,7 +429,7 @@ public class AppStateTrackerImpl implements AppStateTracker { 
        /**

         * Called when a uid goes into cached, so its alarms using a listener should be removed.

         */

        public void removeListenerAlarmsForCachedUid(int uid) {

        public void handleUidCachedChanged(int uid, boolean cached) {

        }

    }
@@ -870,9 +870,7 @@ public class AppStateTrackerImpl implements AppStateTracker { 
        }



        public void onUidCachedChanged(int uid, boolean cached) {

            if (cached) {

                obtainMessage(MSG_ON_UID_CACHED, uid, 0).sendToTarget();

            }

            obtainMessage(MSG_ON_UID_CACHED, uid, cached ? 1 : 0).sendToTarget();

        }



        @Override
@@ -969,14 +967,14 @@ public class AppStateTrackerImpl implements AppStateTracker { 
                    }

                    return;

                case MSG_ON_UID_CACHED:

                    handleUidCached(msg.arg1);

                    handleUidCached(msg.arg1, (msg.arg2 != 0));

                    return;

            }

        }



        private void handleUidCached(int uid) {

        private void handleUidCached(int uid, boolean cached) {

            for (Listener l : cloneListeners()) {

                l.removeListenerAlarmsForCachedUid(uid);

                l.handleUidCachedChanged(uid, cached);

            }

        }

apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java

+58 −10
Original line number Diff line number Diff line
@@ -58,6 +58,7 @@ import static com.android.server.alarm.Alarm.EXACT_ALLOW_REASON_POLICY_PERMISSIO 
import static com.android.server.alarm.Alarm.EXACT_ALLOW_REASON_PRIORITIZED;

import static com.android.server.alarm.Alarm.REQUESTER_POLICY_INDEX;

import static com.android.server.alarm.Alarm.TARE_POLICY_INDEX;

import static com.android.server.alarm.AlarmManagerService.AlarmHandler.REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED;

import static com.android.server.alarm.AlarmManagerService.RemovedAlarm.REMOVE_REASON_ALARM_CANCELLED;

import static com.android.server.alarm.AlarmManagerService.RemovedAlarm.REMOVE_REASON_DATA_CLEARED;

import static com.android.server.alarm.AlarmManagerService.RemovedAlarm.REMOVE_REASON_EXACT_PERMISSION_REVOKED;
@@ -739,6 +740,8 @@ public class AlarmManagerService extends SystemService { 
                "kill_on_schedule_exact_alarm_revoked";

        @VisibleForTesting

        static final String KEY_TEMPORARY_QUOTA_BUMP = "temporary_quota_bump";

        @VisibleForTesting

        static final String KEY_CACHED_LISTENER_REMOVAL_DELAY = "cached_listener_removal_delay";



        private static final long DEFAULT_MIN_FUTURITY = 5 * 1000;

        private static final long DEFAULT_MIN_INTERVAL = 60 * 1000;
@@ -784,6 +787,8 @@ public class AlarmManagerService extends SystemService { 


        private static final boolean DEFAULT_DELAY_NONWAKEUP_ALARMS_WHILE_SCREEN_OFF = true;



        private static final long DEFAULT_CACHED_LISTENER_REMOVAL_DELAY = 10_000;



        // Minimum futurity of a new alarm

        public long MIN_FUTURITY = DEFAULT_MIN_FUTURITY;
@@ -880,6 +885,13 @@ public class AlarmManagerService extends SystemService { 
        public boolean DELAY_NONWAKEUP_ALARMS_WHILE_SCREEN_OFF =

                DEFAULT_DELAY_NONWAKEUP_ALARMS_WHILE_SCREEN_OFF;



        /**

         * Exact listener alarms for apps that get cached are removed after this duration. This is

         * a grace period to allow for transient procstate changes, e.g., when the app switches

         * between different lifecycles.

         */

        public long CACHED_LISTENER_REMOVAL_DELAY = DEFAULT_CACHED_LISTENER_REMOVAL_DELAY;



        private long mLastAllowWhileIdleWhitelistDuration = -1;

        private int mVersion = 0;
@@ -1063,6 +1075,11 @@ public class AlarmManagerService extends SystemService { 
                                    KEY_DELAY_NONWAKEUP_ALARMS_WHILE_SCREEN_OFF,

                                    DEFAULT_DELAY_NONWAKEUP_ALARMS_WHILE_SCREEN_OFF);

                            break;

                        case KEY_CACHED_LISTENER_REMOVAL_DELAY:

                            CACHED_LISTENER_REMOVAL_DELAY = properties.getLong(

                                    KEY_CACHED_LISTENER_REMOVAL_DELAY,

                                    DEFAULT_CACHED_LISTENER_REMOVAL_DELAY);

                            break;

                        default:

                            if (name.startsWith(KEY_PREFIX_STANDBY_QUOTA) && !standbyQuotaUpdated) {

                                // The quotas need to be updated in order, so we can't just rely
@@ -1307,6 +1324,11 @@ public class AlarmManagerService extends SystemService { 
                    DELAY_NONWAKEUP_ALARMS_WHILE_SCREEN_OFF);

            pw.println();



            pw.print(KEY_CACHED_LISTENER_REMOVAL_DELAY);

            pw.print("=");

            TimeUtils.formatDuration(CACHED_LISTENER_REMOVAL_DELAY, pw);

            pw.println();



            pw.decreaseIndent();

        }
@@ -4968,6 +4990,7 @@ public class AlarmManagerService extends SystemService { 
        public static final int TARE_AFFORDABILITY_CHANGED = 12;

        public static final int CHECK_EXACT_ALARM_PERMISSION_ON_UPDATE = 13;

        public static final int TEMPORARY_QUOTA_CHANGED = 14;

        public static final int REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED = 15;



        AlarmHandler() {

            super(Looper.myLooper());
@@ -5088,6 +5111,21 @@ public class AlarmManagerService extends SystemService { 
                        removeExactAlarmsOnPermissionRevoked(uid, packageName, /*killUid = */false);

                    }

                    break;

                case REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED:

                    uid = (Integer) msg.obj;

                    synchronized (mLock) {

                        removeAlarmsInternalLocked(a -> {

                            if (a.uid != uid || a.listener == null || a.windowLength != 0) {

                                return false;

                            }

                            // TODO (b/265195908): Change to .w once we have some data on breakages.

                            Slog.wtf(TAG, "Alarm " + a.listenerTag + " being removed for "

                                    + UserHandle.formatUid(a.uid) + ":" + a.packageName

                                    + " because the app went into cached state");

                            return true;

                        }, REMOVE_REASON_LISTENER_CACHED);

                    }

                    break;

                default:

                    // nope, just ignore it

                    break;
@@ -5444,20 +5482,30 @@ public class AlarmManagerService extends SystemService { 
        }



        @Override

        public void removeListenerAlarmsForCachedUid(int uid) {

        public void handleUidCachedChanged(int uid, boolean cached) {

            if (!CompatChanges.isChangeEnabled(EXACT_LISTENER_ALARMS_DROPPED_ON_CACHED, uid)) {

                return;

            }

            // Apps can quickly get frozen after being cached, breaking the exactness guarantee on

            // listener alarms. So going forward, the contract of exact listener alarms explicitly

            // states that they will be removed as soon as the app goes out of lifecycle. We still

            // allow a short grace period for quick shuffling of proc-states that may happen

            // unexpectedly when switching between different lifecycles and is generally hard for

            // apps to avoid.



            final long delay;

            synchronized (mLock) {

                removeAlarmsInternalLocked(a -> {

                    if (a.uid != uid || a.listener == null || a.windowLength != 0) {

                        return false;

                delay = mConstants.CACHED_LISTENER_REMOVAL_DELAY;

            }

                    // TODO (b/265195908): Change to a .w once we have some data on breakages.

                    Slog.wtf(TAG, "Alarm " + a.listenerTag + " being removed for " + a.packageName

                            + " because the app went into cached state");

                    return true;

                }, REMOVE_REASON_LISTENER_CACHED);

            final Integer uidObj = uid;



            if (cached && !mHandler.hasEqualMessages(REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED,

                    uidObj)) {

                mHandler.sendMessageDelayed(

                        mHandler.obtainMessage(REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED, uidObj),

                        delay);

            } else {

                mHandler.removeEqualMessages(REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED, uidObj);

            }

        }

    };

services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java

+22 −22
Original line number Diff line number Diff line
@@ -1365,8 +1365,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(1)).unblockAlarmsForUid(eq(UID_10_1));

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidGone(UID_10_1, true);
@@ -1385,7 +1385,7 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(1)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidActive(UID_10_1);
@@ -1403,8 +1403,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(1)).unblockAlarmsForUid(eq(UID_10_1));

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidIdle(UID_10_1, true);
@@ -1423,7 +1423,7 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(1)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidCachedChanged(UID_10_1, true);
@@ -1441,8 +1441,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(1)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(1)).handleUidCachedChanged(UID_10_1, true);

        reset(l);



        mIUidObserver.onUidCachedChanged(UID_10_1, false);
@@ -1460,8 +1460,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(1)).handleUidCachedChanged(UID_10_1, false);

        reset(l);
@@ -1481,8 +1481,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidActive(UID_10_1);
@@ -1500,8 +1500,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(1)).unblockAlarmsForUid(eq(UID_10_1));

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidGone(UID_10_1, true);
@@ -1520,7 +1520,7 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(1)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidActive(UID_10_1);
@@ -1538,8 +1538,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(1)).unblockAlarmsForUid(eq(UID_10_1));

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidIdle(UID_10_1, true);
@@ -1558,7 +1558,7 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(1)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).handleUidCachedChanged(anyInt(), anyBoolean());

        reset(l);



        mIUidObserver.onUidCachedChanged(UID_10_1, true);
@@ -1576,8 +1576,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(1)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(1)).handleUidCachedChanged(UID_10_1, true);

        reset(l);



        mIUidObserver.onUidCachedChanged(UID_10_1, false);
@@ -1595,8 +1595,8 @@ public class AppStateTrackerTest { 
        verify(l, times(0)).unblockAllUnrestrictedAlarms();

        verify(l, times(0)).unblockAlarmsForUid(anyInt());

        verify(l, times(0)).unblockAlarmsForUidPackage(anyInt(), anyString());

        verify(l, times(0)).removeAlarmsForUid(UID_10_1);

        verify(l, times(0)).removeListenerAlarmsForCachedUid(UID_10_1);

        verify(l, times(0)).removeAlarmsForUid(anyInt());

        verify(l, times(1)).handleUidCachedChanged(UID_10_1, false);

        reset(l);

    }

services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java

+9 −4
Original line number Diff line number Diff line
@@ -65,6 +65,7 @@ import static com.android.server.alarm.AlarmManagerService.AlarmHandler.EXACT_AL 
import static com.android.server.alarm.AlarmManagerService.AlarmHandler.EXACT_ALARM_DENY_LIST_PACKAGES_REMOVED;

import static com.android.server.alarm.AlarmManagerService.AlarmHandler.REFRESH_EXACT_ALARM_CANDIDATES;

import static com.android.server.alarm.AlarmManagerService.AlarmHandler.REMOVE_EXACT_ALARMS;

import static com.android.server.alarm.AlarmManagerService.AlarmHandler.REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED;

import static com.android.server.alarm.AlarmManagerService.AlarmHandler.REMOVE_FOR_CANCELED;

import static com.android.server.alarm.AlarmManagerService.AlarmHandler.TARE_AFFORDABILITY_CHANGED;

import static com.android.server.alarm.AlarmManagerService.AlarmHandler.TEMPORARY_QUOTA_CHANGED;
@@ -3811,10 +3812,12 @@ public final class AlarmManagerServiceTest { 


        assertEquals(8, mService.mAlarmStore.size());



        mListener.removeListenerAlarmsForCachedUid(TEST_CALLING_UID);

        mListener.handleUidCachedChanged(TEST_CALLING_UID, true);

        assertAndHandleMessageSync(REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED);

        assertEquals(7, mService.mAlarmStore.size());



        mListener.removeListenerAlarmsForCachedUid(TEST_CALLING_UID_2);

        mListener.handleUidCachedChanged(TEST_CALLING_UID_2, true);

        assertAndHandleMessageSync(REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED);

        assertEquals(6, mService.mAlarmStore.size());

    }
@@ -3845,10 +3848,12 @@ public final class AlarmManagerServiceTest { 
        assertEquals(numExactListenerUid1 + 3, mService.mAlarmsPerUid.get(TEST_CALLING_UID));

        assertEquals(numExactListenerUid2 + 2, mService.mAlarmsPerUid.get(TEST_CALLING_UID_2));



        mListener.removeListenerAlarmsForCachedUid(TEST_CALLING_UID);

        mListener.handleUidCachedChanged(TEST_CALLING_UID, true);

        assertAndHandleMessageSync(REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED);

        assertEquals(3, mService.mAlarmsPerUid.get(TEST_CALLING_UID));



        mListener.removeListenerAlarmsForCachedUid(TEST_CALLING_UID_2);

        mListener.handleUidCachedChanged(TEST_CALLING_UID_2, true);

        assertAndHandleMessageSync(REMOVE_EXACT_LISTENER_ALARMS_ON_CACHED);

        assertEquals(2, mService.mAlarmsPerUid.get(TEST_CALLING_UID_2));

    }

}