From 01b2e17cbd54a782d7dc1ee97c20f7fc9b7e7894 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
Merged-In: I5228c323bb9698864c467e9e4c400459ca404b3c
---
 .../android/server/AlarmManagerService.java   |  3 +-
 .../server/AlarmManagerServiceTest.java       | 33 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/services/core/java/com/android/server/AlarmManagerService.java b/services/core/java/com/android/server/AlarmManagerService.java
index 651f941be0b18..7cdcc01bc00da 100644
--- a/services/core/java/com/android/server/AlarmManagerService.java
+++ b/services/core/java/com/android/server/AlarmManagerService.java
@@ -3369,7 +3369,8 @@ 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/AlarmManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java
index 7bd0201b997a5..bcc111faadff0 100644
--- a/services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java
@@ -100,6 +100,7 @@ import org.mockito.MockitoSession;
 import org.mockito.quality.Strictness;
 
 import java.util.ArrayList;
+import java.util.concurrent.atomic.AtomicInteger;
 
 @Presubmit
 @RunWith(AndroidJUnit4.class)
@@ -1077,6 +1078,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;