Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 78fe2d8e authored Apr 17, 2023 by Riddle Hsu

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

parent 0bb91e1d

services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java

+5 −3

Original line number	Diff line number	Diff line
		@@ -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));
		}
		}

services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java

+4 −1

Original line number	Diff line number	Diff line
		@@ -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]);