Merge "Make the disconnecting list from activity up-to-date"
diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java
index 4e47594..c60f7bd 100644
--- a/core/java/android/app/ActivityManagerInternal.java
+++ b/core/java/android/app/ActivityManagerInternal.java
@@ -278,7 +278,7 @@
String resolvedType, boolean fgRequired, String callingPackage, @UserIdInt int userId,
boolean allowBackgroundActivityStarts) throws TransactionTooLargeException;
- public abstract void disconnectActivityFromServices(Object connectionHolder, Object conns);
+ public abstract void disconnectActivityFromServices(Object connectionHolder);
public abstract void cleanUpServices(@UserIdInt int userId, ComponentName component,
Intent baseIntent);
public abstract ActivityInfo getActivityInfoForUser(ActivityInfo aInfo, @UserIdInt int userId);
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 6560777..d80ff35 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -18924,19 +18924,16 @@
}
// The arguments here are untyped because the base ActivityManagerInternal class
- // doesn't have compile-time visiblity into ActivityServiceConnectionHolder or
+ // doesn't have compile-time visibility into ActivityServiceConnectionHolder or
// ConnectionRecord.
@Override
- public void disconnectActivityFromServices(Object connectionHolder, Object conns) {
+ public void disconnectActivityFromServices(Object connectionHolder) {
// 'connectionHolder' is an untyped ActivityServiceConnectionsHolder
- // 'conns' is an untyped HashSet<ConnectionRecord>
final ActivityServiceConnectionsHolder holder =
(ActivityServiceConnectionsHolder) connectionHolder;
- final HashSet<ConnectionRecord> toDisconnect = (HashSet<ConnectionRecord>) conns;
- synchronized(ActivityManagerService.this) {
- for (ConnectionRecord cr : toDisconnect) {
- mServices.removeConnectionLocked(cr, null, holder);
- }
+ synchronized (ActivityManagerService.this) {
+ holder.forEachConnection(cr -> mServices.removeConnectionLocked(
+ (ConnectionRecord) cr, null /* skipApp */, holder /* skipAct */));
}
}
diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java
index df6dfc4..ed38e9a 100644
--- a/services/core/java/com/android/server/wm/ActivityRecord.java
+++ b/services/core/java/com/android/server/wm/ActivityRecord.java
@@ -3037,6 +3037,8 @@
}
// Throw away any services that have been bound by this activity.
mServiceConnectionsHolder.disconnectActivityFromServices();
+ // This activity record is removing, make sure not to disconnect twice.
+ mServiceConnectionsHolder = null;
}
@Override
diff --git a/services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java b/services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java
index 6e75f9c..5dfc261 100644
--- a/services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java
+++ b/services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java
@@ -18,10 +18,13 @@
import static com.android.server.wm.ActivityStack.ActivityState.PAUSING;
import static com.android.server.wm.ActivityStack.ActivityState.RESUMED;
+import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_CLEANUP;
+import static com.android.server.wm.ActivityTaskManagerDebugConfig.TAG_ATM;
+
+import android.util.ArraySet;
+import android.util.Slog;
import java.io.PrintWriter;
-import java.util.HashSet;
-import java.util.Iterator;
import java.util.function.Consumer;
/**
@@ -30,6 +33,8 @@
* instance of this per activity for tracking all services connected to that activity. AM will
* sometimes query this to bump the OOM score for the processes with services connected to visible
* activities.
+ * <p>
+ * Public methods are called in AM lock, otherwise in WM lock.
*/
public class ActivityServiceConnectionsHolder<T> {
@@ -44,7 +49,10 @@
* on the WM side since we don't perform operations on the object. Mainly here for communication
* and booking with the AM side.
*/
- private HashSet<T> mConnections;
+ private ArraySet<T> mConnections;
+
+ /** Whether all connections of {@link #mActivity} are being removed. */
+ private volatile boolean mIsDisconnecting;
ActivityServiceConnectionsHolder(ActivityTaskManagerService service, ActivityRecord activity) {
mService = service;
@@ -54,8 +62,16 @@
/** Adds a connection record that the activity has bound to a specific service. */
public void addConnection(T c) {
synchronized (mService.mGlobalLock) {
+ if (mIsDisconnecting) {
+ // This is unlikely to happen because the caller should create a new holder.
+ if (DEBUG_CLEANUP) {
+ Slog.e(TAG_ATM, "Skip adding connection " + c + " to a disconnecting holder of "
+ + mActivity);
+ }
+ return;
+ }
if (mConnections == null) {
- mConnections = new HashSet<>();
+ mConnections = new ArraySet<>();
}
mConnections.add(c);
}
@@ -67,6 +83,9 @@
if (mConnections == null) {
return;
}
+ if (DEBUG_CLEANUP && mIsDisconnecting) {
+ Slog.v(TAG_ATM, "Remove pending disconnecting " + c + " of " + mActivity);
+ }
mConnections.remove(c);
}
}
@@ -88,26 +107,33 @@
if (mConnections == null || mConnections.isEmpty()) {
return;
}
- final Iterator<T> it = mConnections.iterator();
- while (it.hasNext()) {
- T c = it.next();
- consumer.accept(c);
+ for (int i = mConnections.size() - 1; i >= 0; i--) {
+ consumer.accept(mConnections.valueAt(i));
}
}
}
- /** Removes the connection between the activity and all services that were connected to it. */
+ /**
+ * Removes the connection between the activity and all services that were connected to it. In
+ * general, this method is used to clean up if the activity didn't unbind services before it
+ * is destroyed.
+ */
void disconnectActivityFromServices() {
- if (mConnections == null || mConnections.isEmpty()) {
+ if (mConnections == null || mConnections.isEmpty() || mIsDisconnecting) {
return;
}
- // Capture and null out mConnections, to guarantee that we process
+ // Mark as disconnecting, to guarantee that we process
// disconnect of these specific connections exactly once even if
// we're racing with rapid activity lifecycle churn and this
// method is invoked more than once on this object.
- final Object disc = mConnections;
- mConnections = null;
- mService.mH.post(() -> mService.mAmInternal.disconnectActivityFromServices(this, disc));
+ // It is possible that {@link #removeConnection} is called while the disconnect-runnable is
+ // still in the message queue, so keep the reference of {@link #mConnections} to make sure
+ // the connection list is up-to-date.
+ mIsDisconnecting = true;
+ mService.mH.post(() -> {
+ mService.mAmInternal.disconnectActivityFromServices(this);
+ mIsDisconnecting = false;
+ });
}
public void dump(PrintWriter pw, String prefix) {
@@ -116,4 +142,9 @@
}
}
+ /** Used by {@link ActivityRecord#dump}. */
+ @Override
+ public String toString() {
+ return String.valueOf(mConnections);
+ }
}