From 78fe2d8e4207ba556d46290921235a5518a66e01 Mon Sep 17 00:00:00 2001
From: Riddle Hsu <riddlehsu@google.com>
Date: Mon, 17 Apr 2023 15:20:30 +0800
Subject: 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
---
 .../com/android/server/wm/ActivityServiceConnectionsHolder.java   | 8 +++++---
 .../wmtests/src/com/android/server/wm/ActivityRecordTests.java    | 5 ++++-
 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]);
 
-- 
cgit v1.2.3-59-g8ed1b