From 2c2e1bf150f203cfb5299ba0e2a69f33cbec1aa8 Mon Sep 17 00:00:00 2001
From: Suprabh Shukla <suprabh@google.com>
Date: Tue, 22 Dec 2020 13:14:12 -0800
Subject: [PATCH] Copy the list before passing to deliverAlarms

Downstream code from deliverAlarmsLocked can cause removeLocked or
removeImpl to be called which changes the size of the list.

Test: atest FrameworksMockingServicesTests:com.android.server.alarm

Bug: 175701084
Change-Id: I5228c323bb9698864c467e9e4c400459ca404b3c
---
 .../server/alarm/AlarmManagerService.java     |  3 +-
 .../server/alarm/AlarmManagerServiceTest.java | 33 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

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 20c77da53f2ce..7842d487dfb2b 100644
--- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java
+++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java
@@ -3061,7 +3061,8 @@ public class AlarmManagerService extends SystemService {
                     if (mMaxDelayTime < thisDelayTime) {
                         mMaxDelayTime = thisDelayTime;
                     }
-                    deliverAlarmsLocked(mPendingNonWakeupAlarms, nowELAPSED);
+                    final ArrayList<Alarm> triggerList = new ArrayList<>(mPendingNonWakeupAlarms);
+                    deliverAlarmsLocked(triggerList, nowELAPSED);
                     mPendingNonWakeupAlarms.clear();
                 }
                 if (mNonInteractiveStartTime > 0) {
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 8e4942e1ad5c6..8edac4f8ce58a 100644
--- a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java
@@ -121,6 +121,7 @@ import org.mockito.stubbing.Answer;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.concurrent.Executor;
+import java.util.concurrent.atomic.AtomicInteger;
 
 @Presubmit
 @RunWith(AndroidJUnit4.class)
@@ -1133,6 +1134,38 @@ public class AlarmManagerServiceTest {
         }
     }
 
+    /**
+     * This tests that all non wakeup alarms are sent even when the mPendingNonWakeupAlarms gets
+     * modified before the send is complete. This avoids bugs like b/175701084.
+     */
+    @Test
+    public void allNonWakeupAlarmsSentAtomically() throws Exception {
+        final int numAlarms = 5;
+        final AtomicInteger alarmsFired = new AtomicInteger(0);
+        for (int i = 0; i < numAlarms; i++) {
+            final IAlarmListener listener = new IAlarmListener.Stub() {
+                @Override
+                public void doAlarm(IAlarmCompleteListener callback) throws RemoteException {
+                    alarmsFired.incrementAndGet();
+                    mService.mPendingNonWakeupAlarms.clear();
+                }
+            };
+            setTestAlarmWithListener(ELAPSED_REALTIME, mNowElapsedTest + i + 5, listener);
+        }
+        doReturn(true).when(mService).checkAllowNonWakeupDelayLocked(anyLong());
+        // Advance time past all expirations.
+        mNowElapsedTest += numAlarms + 5;
+        mTestTimer.expire();
+        assertEquals(numAlarms, mService.mPendingNonWakeupAlarms.size());
+
+        // All of these alarms should be sent on interactive state change to true
+        mService.interactiveStateChangedLocked(false);
+        mService.interactiveStateChangedLocked(true);
+
+        assertEquals(numAlarms, alarmsFired.get());
+        assertEquals(0, mService.mPendingNonWakeupAlarms.size());
+    }
+
     @Test
     public void alarmCountOnPendingNonWakeupAlarmsRemoved() throws Exception {
         final int numAlarms = 10;