diff --git a/apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java b/apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java index b7cf2978bb2c9..3fc87d394574e 100644 --- a/apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java +++ b/apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java @@ -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); } } diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index 26c0eef5542dc..c76a43fa38f58 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -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; - } - // 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); + delay = mConstants.CACHED_LISTENER_REMOVAL_DELAY; + } + 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); } } }; diff --git a/services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java b/services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java index 31cfa780a1330..47ae97fc5d27a 100644 --- a/services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java @@ -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); } diff --git a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java index 174141101dfc2..f9f53251fa7ee 100644 --- a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java @@ -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)); } }