From ac3d84392a4a0e70df21515ae205aca5017cc650 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Thu, 3 Sep 2020 12:54:39 -0700 Subject: [PATCH] RESTRICT AUTOMERGE Remove alarms from mPendingNonWakeupAlarms While setting an alarm, we should remove any existing occurrences of the same, but we were skipping on removing any past due non-wakeup alarms. In rare cases when a caller is spamming the alarm manager, we might end up accumulating alarms inside mPendingNonWakeupAlarms that count towards the per-uid limit of the caller, and may result in an exception. Test: atest FrameworksMockingServicesTests:AlarmManagerServiceTest atest CtsAlarmManagerTestCases Bug: 167595061 Change-Id: I2d2b08019a3b3a1e63b60d3a6e2909db7b1a1864 --- .../android/server/AlarmManagerService.java | 8 ++++ .../server/AlarmManagerServiceTest.java | 43 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/services/core/java/com/android/server/AlarmManagerService.java b/services/core/java/com/android/server/AlarmManagerService.java index 4fab067d1d3ba..651f941be0b18 100644 --- a/services/core/java/com/android/server/AlarmManagerService.java +++ b/services/core/java/com/android/server/AlarmManagerService.java @@ -3109,6 +3109,14 @@ class AlarmManagerService extends SystemService { mPendingBackgroundAlarms.removeAt(i); } } + for (int i = mPendingNonWakeupAlarms.size() - 1; i >= 0; i--) { + final Alarm a = mPendingNonWakeupAlarms.get(i); + if (a.matches(operation, directReceiver)) { + // Don't set didRemove, since this doesn't impact the scheduled alarms. + mPendingNonWakeupAlarms.remove(i); + decrementAlarmCount(a.uid, 1); + } + } if (didRemove) { if (DEBUG_BATCH) { Slog.v(TAG, "remove(operation) changed bounds; rebatching"); diff --git a/services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java index ca8e50aa19c99..7bd0201b997a5 100644 --- a/services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java @@ -1053,6 +1053,49 @@ public class AlarmManagerServiceTest { } } + @Test + public void nonWakeupAlarmsDeferred() throws Exception { + final int numAlarms = 10; + final PendingIntent[] pis = new PendingIntent[numAlarms]; + for (int i = 0; i < numAlarms; i++) { + pis[i] = getNewMockPendingIntent(); + setTestAlarm(ELAPSED_REALTIME, mNowElapsedTest + i + 5, pis[i]); + } + doReturn(true).when(mService).checkAllowNonWakeupDelayLocked(anyLong()); + // Advance time past all expirations. + mNowElapsedTest += numAlarms + 5; + mTestTimer.expire(); + assertEquals(numAlarms, mService.mPendingNonWakeupAlarms.size()); + + // These alarms should be sent on interactive state change to true + mService.interactiveStateChangedLocked(false); + mService.interactiveStateChangedLocked(true); + + for (int i = 0; i < numAlarms; i++) { + verify(pis[i]).send(eq(mMockContext), eq(0), any(Intent.class), any(), + any(Handler.class), isNull(), any()); + } + } + + @Test + public void alarmCountOnPendingNonWakeupAlarmsRemoved() throws Exception { + final int numAlarms = 10; + final PendingIntent[] pis = new PendingIntent[numAlarms]; + for (int i = 0; i < numAlarms; i++) { + pis[i] = getNewMockPendingIntent(); + setTestAlarm(ELAPSED_REALTIME, mNowElapsedTest + i + 5, pis[i]); + } + doReturn(true).when(mService).checkAllowNonWakeupDelayLocked(anyLong()); + // Advance time past all expirations. + mNowElapsedTest += numAlarms + 5; + mTestTimer.expire(); + assertEquals(numAlarms, mService.mPendingNonWakeupAlarms.size()); + for (int i = 0; i < numAlarms; i++) { + mService.removeLocked(pis[i], null); + assertEquals(numAlarms - i - 1, mService.mAlarmsPerUid.get(TEST_CALLING_UID, 0)); + } + } + @After public void tearDown() { if (mMockingSession != null) {