Skip to content

Commit 01b2e17

Browse files
author
Suprabh Shukla
committed
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
1 parent 2e3b8fe commit 01b2e17

2 files changed

Lines changed: 35 additions & 1 deletion

File tree

services/core/java/com/android/server/AlarmManagerService.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3369,7 +3369,8 @@ void interactiveStateChangedLocked(boolean interactive) {
33693369
if (mMaxDelayTime < thisDelayTime) {
33703370
mMaxDelayTime = thisDelayTime;
33713371
}
3372-
deliverAlarmsLocked(mPendingNonWakeupAlarms, nowELAPSED);
3372+
final ArrayList<Alarm> triggerList = new ArrayList<>(mPendingNonWakeupAlarms);
3373+
deliverAlarmsLocked(triggerList, nowELAPSED);
33733374
mPendingNonWakeupAlarms.clear();
33743375
}
33753376
if (mNonInteractiveStartTime > 0) {

services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@
100100
import org.mockito.quality.Strictness;
101101

102102
import java.util.ArrayList;
103+
import java.util.concurrent.atomic.AtomicInteger;
103104

104105
@Presubmit
105106
@RunWith(AndroidJUnit4.class)
@@ -1077,6 +1078,38 @@ public void nonWakeupAlarmsDeferred() throws Exception {
10771078
}
10781079
}
10791080

1081+
/**
1082+
* This tests that all non wakeup alarms are sent even when the mPendingNonWakeupAlarms gets
1083+
* modified before the send is complete. This avoids bugs like b/175701084.
1084+
*/
1085+
@Test
1086+
public void allNonWakeupAlarmsSentAtomically() throws Exception {
1087+
final int numAlarms = 5;
1088+
final AtomicInteger alarmsFired = new AtomicInteger(0);
1089+
for (int i = 0; i < numAlarms; i++) {
1090+
final IAlarmListener listener = new IAlarmListener.Stub() {
1091+
@Override
1092+
public void doAlarm(IAlarmCompleteListener callback) throws RemoteException {
1093+
alarmsFired.incrementAndGet();
1094+
mService.mPendingNonWakeupAlarms.clear();
1095+
}
1096+
};
1097+
setTestAlarmWithListener(ELAPSED_REALTIME, mNowElapsedTest + i + 5, listener);
1098+
}
1099+
doReturn(true).when(mService).checkAllowNonWakeupDelayLocked(anyLong());
1100+
// Advance time past all expirations.
1101+
mNowElapsedTest += numAlarms + 5;
1102+
mTestTimer.expire();
1103+
assertEquals(numAlarms, mService.mPendingNonWakeupAlarms.size());
1104+
1105+
// All of these alarms should be sent on interactive state change to true
1106+
mService.interactiveStateChangedLocked(false);
1107+
mService.interactiveStateChangedLocked(true);
1108+
1109+
assertEquals(numAlarms, alarmsFired.get());
1110+
assertEquals(0, mService.mPendingNonWakeupAlarms.size());
1111+
}
1112+
10801113
@Test
10811114
public void alarmCountOnPendingNonWakeupAlarmsRemoved() throws Exception {
10821115
final int numAlarms = 10;

0 commit comments

Comments
 (0)