Merge "Add a lock around mBuffer in Archive." into sc-dev (e9e836df) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/notification/NotificationManagerService.java

+40 −29

Original line number	Diff line number	Diff line
		@@ -629,6 +629,8 @@ public class NotificationManagerService extends SystemService {
		static class Archive {
		final SparseArray<Boolean> mEnabled;
		final int mBufferSize;
		final Object mBufferLock = new Object();
		@GuardedBy("mBufferLock")
		final LinkedList<Pair<StatusBarNotification, Integer>> mBuffer;

		public Archive(int size) {
		@@ -651,6 +653,7 @@ public class NotificationManagerService extends SystemService {
		if (!mEnabled.get(sbn.getNormalizedUserId(), false)) {
		return;
		}
		synchronized (mBufferLock) {
		if (mBuffer.size() == mBufferSize) {
		mBuffer.removeFirst();
		}
		@@ -660,12 +663,14 @@ public class NotificationManagerService extends SystemService {
		// store a (lightened) copy.
		mBuffer.addLast(new Pair<>(sbn.cloneLight(), reason));
		}
		}

		public Iterator<Pair<StatusBarNotification, Integer>> descendingIterator() {
		return mBuffer.descendingIterator();
		}

		public StatusBarNotification[] getArray(int count, boolean includeSnoozed) {
		synchronized (mBufferLock) {
		if (count == 0) count = mBufferSize;
		List<StatusBarNotification> a = new ArrayList();
		Iterator<Pair<StatusBarNotification, Integer>> iter = descendingIterator();
		@@ -679,11 +684,13 @@ public class NotificationManagerService extends SystemService {
		}
		return a.toArray(new StatusBarNotification[a.size()]);
		}
		}

		public void updateHistoryEnabled(@UserIdInt int userId, boolean enabled) {
		mEnabled.put(userId, enabled);

		if (!enabled) {
		synchronized (mBufferLock) {
		for (int i = mBuffer.size() - 1; i >= 0; i--) {
		if (userId == mBuffer.get(i).first.getNormalizedUserId()) {
		mBuffer.remove(i);
		@@ -691,23 +698,27 @@ public class NotificationManagerService extends SystemService {
		}
		}
		}
		}

		// Remove notifications with the specified user & channel ID.
		public void removeChannelNotifications(String pkg, @UserIdInt int userId,
		String channelId) {
		Iterator<Pair<StatusBarNotification, Integer>> bufferIter = mBuffer.iterator();
		synchronized (mBufferLock) {
		Iterator<Pair<StatusBarNotification, Integer>> bufferIter = descendingIterator();
		while (bufferIter.hasNext()) {
		final Pair<StatusBarNotification, Integer> pair = bufferIter.next();
		if (pair.first != null
		&& userId == pair.first.getNormalizedUserId()
		&& pkg != null && pkg.equals(pair.first.getPackageName())
		&& pair.first.getNotification() != null
		&& Objects.equals(channelId, pair.first.getNotification().getChannelId())) {
		&& Objects.equals(channelId,
		pair.first.getNotification().getChannelId())) {
		bufferIter.remove();
		}
		}
		}
		}
		}

		void loadDefaultApprovedServices(int userId) {
		mListeners.loadDefaultsFromConfig();

services/tests/uiservicestests/src/com/android/server/notification/ArchiveTest.java

+56 −0

Original line number	Diff line number	Diff line
		@@ -21,6 +21,8 @@ import static android.service.notification.NotificationListenerService.REASON_CA

		import static com.google.common.truth.Truth.assertThat;

		import static org.junit.Assert.fail;

		import android.app.Notification;
		import android.os.UserHandle;
		import android.service.notification.StatusBarNotification;
		@@ -37,7 +39,11 @@ import org.mockito.MockitoAnnotations;

		import java.util.ArrayList;
		import java.util.Arrays;
		import java.util.ConcurrentModificationException;
		import java.util.List;
		import java.util.concurrent.CountDownLatch;
		import java.util.concurrent.TimeUnit;
		import java.util.concurrent.atomic.AtomicBoolean;

		@SmallTest
		@RunWith(AndroidJUnit4.class)
		@@ -165,4 +171,54 @@ public class ArchiveTest extends UiServiceTestCase {
		assertThat(expected).contains(sbn.getKey());
		}
		}

		@Test
		public void testRemoveChannelNotifications_concurrently() throws InterruptedException {
		List<String> expected = new ArrayList<>();
		// Add one extra notification to the beginning to test when 2 adjacent notifications will be
		// removed in the same pass.
		StatusBarNotification sbn0 = getNotification("pkg", 0, UserHandle.of(USER_CURRENT));
		mArchive.record(sbn0, REASON_CANCEL);
		for (int i = 0; i < SIZE; i++) {
		StatusBarNotification sbn = getNotification("pkg", i, UserHandle.of(USER_CURRENT));
		mArchive.record(sbn, REASON_CANCEL);
		if (i >= SIZE - 2) {
		// Remove everything < SIZE - 2
		expected.add(sbn.getKey());
		}
		}

		// Remove these in multiple threads to try to get them to happen at the same time
		int numThreads = SIZE - 2;
		AtomicBoolean error = new AtomicBoolean(false);
		CountDownLatch startThreadsLatch = new CountDownLatch(1);
		CountDownLatch threadsDone = new CountDownLatch(numThreads);
		for (int i = 0; i < numThreads; i++) {
		final int idx = i;
		new Thread(() -> {
		try {
		startThreadsLatch.await(10, TimeUnit.SECONDS);
		} catch (InterruptedException e) {
		throw new RuntimeException(e);
		}
		try {
		mArchive.removeChannelNotifications("pkg", USER_CURRENT, "test" + idx);
		} catch (ConcurrentModificationException e) {
		error.compareAndSet(false, true);
		}
		}).start();
		}

		startThreadsLatch.countDown();
		threadsDone.await(10, TimeUnit.SECONDS);
		if (error.get()) {
		fail("Concurrent modification exception");
		}

		List<StatusBarNotification> actual = Arrays.asList(mArchive.getArray(SIZE, true));
		assertThat(actual).hasSize(expected.size());
		for (StatusBarNotification sbn : actual) {
		assertThat(expected).contains(sbn.getKey());
		}
		}
		}