From a2d34f4f058ccaabf64e3e35a97df3e24c0e2939 Mon Sep 17 00:00:00 2001
From: Sergey Nikolaienkov <sergeynv@google.com>
Date: Fri, 18 Feb 2022 11:25:42 +0100
Subject: [PATCH] Return a copy of the "real" set in AssociationStore

Return a copy of the "real" set AssociationSet from
AssociationStoreImpl.getAssociations(), which fixes
ConcurrentModificationException-s that arise if a direct reference to
the mIdMap.values() collection is returned.

Bug: 220018043
Test: atest CtsCompanionDeviceManagerCoreTestCases
Test: atest CtsCompanionDeviceManagerUiAutomationTestCases
Test: manually install companion app, create a CDM Association with a
      BLE companion device, uninstall the app.
Change-Id: Ie0ecab7fe61923ecef1541cb9f6450742c4b83b2
---
 .../server/companion/AssociationStoreImpl.java     | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/services/companion/java/com/android/server/companion/AssociationStoreImpl.java b/services/companion/java/com/android/server/companion/AssociationStoreImpl.java
index 21a677b8383c8..cb28254f70db6 100644
--- a/services/companion/java/com/android/server/companion/AssociationStoreImpl.java
+++ b/services/companion/java/com/android/server/companion/AssociationStoreImpl.java
@@ -171,12 +171,20 @@ class AssociationStoreImpl implements AssociationStore {
         broadcastChange(CHANGE_TYPE_REMOVED, association);
     }
 
+    /**
+     * @return a "snapshot" of the current state of the existing associations.
+     */
     public @NonNull Collection<AssociationInfo> getAssociations() {
-        final Collection<AssociationInfo> allAssociations;
         synchronized (mLock) {
-            allAssociations = mIdMap.values();
+            // IMPORTANT: make and return a COPY of the mIdMap.values(), NOT a "direct" reference.
+            // The HashMap.values() returns a collection which is backed by the HashMap, so changes
+            // to the HashMap are reflected in this collection.
+            // For us this means that if mIdMap is modified while the iteration over mIdMap.values()
+            // is in progress it may lead to "undefined results" (according to the HashMap's
+            // documentation) or cause ConcurrentModificationExceptions in the iterator (according
+            // to the bugreports...).
+            return List.copyOf(mIdMap.values());
         }
-        return Collections.unmodifiableCollection(allAssociations);
     }
 
     public @NonNull List<AssociationInfo> getAssociationsForUser(@UserIdInt int userId) {