Invoke service connection consumer outside the object lock

The consumer may contain arbitrary operations which may
acquire the local lock again and lead to dead lock.

Currently forEachConnection is only called from
disconnectActivityFromServices which is very rare to be called
(activity destroyed without unbind service). So a local copy
should be fine.

Bug: 275277537
Test: atest ActivityRecordTests#testActivityServiceConnectionsHolder
Change-Id: If26a0cb9970cf5318cd0477adcc0fe07e98aaed9

2 files changed, 9 insertions, 4 deletions

diff --git a/services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java b/services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java
index 5f56af7fd4e0..1208b6ef396f 100644
--- a/services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java
+++ b/services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java
@@ -99,13 +99,15 @@ public class ActivityServiceConnectionsHolder<T> {
     }
 
     public void forEachConnection(Consumer<T> consumer) {
+        final ArraySet<T> connections;
         synchronized (mActivity) {
             if (mConnections == null || mConnections.isEmpty()) {
                 return;
             }
-            for (int i = mConnections.size() - 1; i >= 0; i--) {
-                consumer.accept(mConnections.valueAt(i));
-            }
+            connections = new ArraySet<>(mConnections);
+        }
+        for (int i = connections.size() - 1; i >= 0; i--) {
+            consumer.accept(connections.valueAt(i));
         }
     }
 
diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java
index 8f2b470908c4..0033e3e368c6 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java
@@ -2399,7 +2399,10 @@ public class ActivityRecordTests extends WindowTestsBase {
         holder.addConnection(connection);
         assertTrue(holder.isActivityVisible());
         final int[] count = new int[1];
-        final Consumer<Object> c = conn -> count[0]++;
+        final Consumer<Object> c = conn -> {
+            count[0]++;
+            assertFalse(Thread.holdsLock(activity));
+        };
         holder.forEachConnection(c);
         assertEquals(1, count[0]);