From 0d3bf483bc8f288a1c9d231ac1980c0a203322f6 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Thu, 13 May 2021 21:09:27 -0700 Subject: [PATCH] Relaxing minimum alarm window requirements Minimum allowed window is now a function of the futurity of the alarm. So for alarms in the near future, it is smaller and it increases as time to alarm increases maxing out at 10 minutes. Test: atest AlarmManagerServiceTest Bug: 185199076 Change-Id: I57f275adab7f9bcc7f0f7535bb810aa9b359287b --- .../java/android/app/AlarmManager.java | 14 +++-- .../server/alarm/AlarmManagerService.java | 55 +++++++++++-------- .../server/alarm/AlarmManagerServiceTest.java | 34 +++++++++++- 3 files changed, 73 insertions(+), 30 deletions(-) diff --git a/apex/jobscheduler/framework/java/android/app/AlarmManager.java b/apex/jobscheduler/framework/java/android/app/AlarmManager.java index b096537821caa..719212dbde049 100644 --- a/apex/jobscheduler/framework/java/android/app/AlarmManager.java +++ b/apex/jobscheduler/framework/java/android/app/AlarmManager.java @@ -530,9 +530,10 @@ public class AlarmManager { * modest timeliness requirements for its alarms. * *

- * Note: Starting with API {@link Build.VERSION_CODES#S}, the system will ensure that the window - * specified is at least a few minutes, as smaller windows are considered practically exact - * and should use the other APIs provided for exact alarms. + * Note: Starting with API {@link Build.VERSION_CODES#S}, apps should not pass in a window of + * less than 10 minutes. The system will try its best to accommodate smaller windows if the + * alarm is supposed to fire in the near future, but there are no guarantees and the app should + * expect any window smaller than 10 minutes to get elongated to 10 minutes. * *

* This method can also be used to achieve strict ordering guarantees among @@ -586,9 +587,10 @@ public class AlarmManager { * if {@code null} is passed as the {@code targetHandler} parameter. * *

- * Note: Starting with API {@link Build.VERSION_CODES#S}, the system will ensure that the window - * specified is at least a few minutes, as smaller windows are considered practically exact - * and should use the other APIs provided for exact alarms. + * Note: Starting with API {@link Build.VERSION_CODES#S}, apps should not pass in a window of + * less than 10 minutes. The system will try its best to accommodate smaller windows if the + * alarm is supposed to fire in the near future, but there are no guarantees and the app should + * expect any window smaller than 10 minutes to get elongated to 10 minutes. * * @see #setWindow(int, long, long, PendingIntent) */ 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 0eb260986472a..5365218203d99 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -530,8 +530,7 @@ public class AlarmManagerService extends SystemService { private static final long DEFAULT_MIN_FUTURITY = 5 * 1000; private static final long DEFAULT_MIN_INTERVAL = 60 * 1000; private static final long DEFAULT_MAX_INTERVAL = 365 * INTERVAL_DAY; - // TODO (b/185199076): Tune based on breakage reports. - private static final long DEFAULT_MIN_WINDOW = 30 * 60 * 1000; + private static final long DEFAULT_MIN_WINDOW = 10 * 60 * 1000; private static final long DEFAULT_ALLOW_WHILE_IDLE_WHITELIST_DURATION = 10 * 1000; private static final long DEFAULT_LISTENER_TIMEOUT = 5 * 1000; private static final int DEFAULT_MAX_ALARMS_PER_UID = 500; @@ -1147,6 +1146,18 @@ public class AlarmManagerService extends SystemService { return when; } + /** + * This is the minimum window that can be requested for the given alarm. Windows smaller than + * this value will be elongated to match it. + * Current heuristic is similar to {@link #maxTriggerTime(long, long, long)}, the minimum + * allowed window is either {@link Constants#MIN_WINDOW} or 75% of the alarm's futurity, + * whichever is smaller. + */ + long getMinimumAllowedWindow(long nowElapsed, long triggerElapsed) { + final long futurity = triggerElapsed - nowElapsed; + return Math.min((long) (futurity * 0.75), mConstants.MIN_WINDOW); + } + // Apply a heuristic to { recurrence interval, futurity of the trigger time } to // calculate the end of our nominal delivery window for the alarm. static long maxTriggerTime(long now, long triggerAtTime, long interval) { @@ -1833,25 +1844,6 @@ public class AlarmManagerService extends SystemService { } } - // Snap the window to reasonable limits. - if (windowLength > INTERVAL_DAY) { - Slog.w(TAG, "Window length " + windowLength - + "ms suspiciously long; limiting to 1 day"); - windowLength = INTERVAL_DAY; - } else if (windowLength > 0 && windowLength < mConstants.MIN_WINDOW - && (flags & FLAG_PRIORITIZE) == 0) { - if (CompatChanges.isChangeEnabled(AlarmManager.ENFORCE_MINIMUM_WINDOW_ON_INEXACT_ALARMS, - callingPackage, UserHandle.getUserHandleForUid(callingUid))) { - Slog.w(TAG, "Window length " + windowLength + "ms too short; expanding to " - + mConstants.MIN_WINDOW + "ms."); - windowLength = mConstants.MIN_WINDOW; - } else { - // TODO (b/185199076): Remove log once we have some data about what apps will break - Slog.wtf(TAG, "Short window " + windowLength + "ms specified by " - + callingPackage); - } - } - // Sanity check the recurrence interval. This will catch people who supply // seconds when the API expects milliseconds, or apps trying shenanigans // around intentional period overflow, etc. @@ -1883,7 +1875,7 @@ public class AlarmManagerService extends SystemService { // Try to prevent spamming by making sure apps aren't firing alarms in the immediate future final long minTrigger = nowElapsed + (UserHandle.isCore(callingUid) ? 0L : mConstants.MIN_FUTURITY); - final long triggerElapsed = (nominalTrigger > minTrigger) ? nominalTrigger : minTrigger; + final long triggerElapsed = Math.max(minTrigger, nominalTrigger); final long maxElapsed; if (windowLength == 0) { @@ -1893,6 +1885,25 @@ public class AlarmManagerService extends SystemService { // Fix this window in place, so that as time approaches we don't collapse it. windowLength = maxElapsed - triggerElapsed; } else { + // The window was explicitly requested. Snap it to allowable limits. + final long minAllowedWindow = getMinimumAllowedWindow(nowElapsed, triggerElapsed); + if (windowLength > INTERVAL_DAY) { + Slog.w(TAG, "Window length " + windowLength + "ms too long; limiting to 1 day"); + windowLength = INTERVAL_DAY; + } else if ((flags & FLAG_PRIORITIZE) == 0 && windowLength < minAllowedWindow) { + // Prioritized alarms are exempt from minimum window limits. + if (CompatChanges.isChangeEnabled( + AlarmManager.ENFORCE_MINIMUM_WINDOW_ON_INEXACT_ALARMS, callingPackage, + UserHandle.getUserHandleForUid(callingUid))) { + Slog.w(TAG, "Window length " + windowLength + "ms too short; expanding to " + + minAllowedWindow + "ms."); + windowLength = minAllowedWindow; + } else { + // TODO (b/185199076): Remove temporary log to catch breaking apps. + Slog.wtf(TAG, "Short window " + windowLength + "ms specified by " + + callingPackage); + } + } maxElapsed = triggerElapsed + windowLength; } synchronized (mLock) { 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 5222511cc479d..d5e1cd6e04157 100644 --- a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java @@ -175,6 +175,7 @@ import org.mockito.stubbing.Answer; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.Random; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.LongConsumer; @@ -2269,17 +2270,46 @@ public class AlarmManagerServiceTest { () -> CompatChanges.isChangeEnabled( eq(AlarmManager.ENFORCE_MINIMUM_WINDOW_ON_INEXACT_ALARMS), anyString(), any(UserHandle.class))); - final long minWindow = 73; + final int minWindow = 73; setDeviceConfigLong(KEY_MIN_WINDOW, minWindow); + final Random random = new Random(42); + // 0 is WINDOW_EXACT and < 0 is WINDOW_HEURISTIC. for (int window = 1; window <= minWindow; window++) { + final PendingIntent pi = getNewMockPendingIntent(); + final long futurity = random.nextInt(minWindow); + + setTestAlarm(ELAPSED_REALTIME, mNowElapsedTest + futurity, window, pi, 0, 0, + TEST_CALLING_UID, null); + + final long minAllowed = (long) (futurity * 0.75); // This will always be <= minWindow. + + assertEquals(1, mService.mAlarmStore.size()); + final Alarm a = mService.mAlarmStore.remove(unused -> true).get(0); + assertEquals(Math.max(minAllowed, window), a.windowLength); + } + + for (int window = 1; window <= minWindow; window++) { + final PendingIntent pi = getNewMockPendingIntent(); + final long futurity = 2 * minWindow + window; // implies (0.75 * futurity) > minWindow + + setTestAlarm(ELAPSED_REALTIME, mNowElapsedTest + futurity, window, pi, 0, 0, + TEST_CALLING_UID, null); + + assertEquals(1, mService.mAlarmStore.size()); + final Alarm a = mService.mAlarmStore.remove(unused -> true).get(0); + assertEquals(minWindow, a.windowLength); + } + + for (int i = 0; i < 20; i++) { + final long window = minWindow + random.nextInt(100); final PendingIntent pi = getNewMockPendingIntent(); setTestAlarm(ELAPSED_REALTIME, 0, window, pi, 0, 0, TEST_CALLING_UID, null); assertEquals(1, mService.mAlarmStore.size()); final Alarm a = mService.mAlarmStore.remove(unused -> true).get(0); - assertEquals(minWindow, a.windowLength); + assertEquals(window, a.windowLength); } }