Ensure force-stopped process do not start again during cleanup
By setting all activities of the removed process as finishing,
those activities haven’t been removed won't be the candidate of
top or visible activity.
Also adjust the conditions to set ProcessRecord.removed to fit the
original meaning a bit. That should be used to indicate the process
is killed for the change of package state. Now ProcessRecord.removed
will only be set when force-stop or crash too many times.
Fixes: 114117787
Fixes: 37070733
Test: atest ActivityManagerTest#testForceStopPackageWontRestartProcess
Test: atest ActivityStackTests
Change-Id: Ia2975c2e4ece8600307b317f124354ecdca55d31
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index 5d6c2f0..3e41eb5 100644
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -3316,8 +3316,7 @@
}
private boolean collectPackageServicesLocked(String packageName, Set<String> filterByClasses,
- boolean evenPersistent, boolean doit, boolean killProcess,
- ArrayMap<ComponentName, ServiceRecord> services) {
+ boolean evenPersistent, boolean doit, ArrayMap<ComponentName, ServiceRecord> services) {
boolean didSomething = false;
for (int i = services.size() - 1; i >= 0; i--) {
ServiceRecord service = services.valueAt(i);
@@ -3332,13 +3331,10 @@
}
didSomething = true;
Slog.i(TAG, " Force stopping service " + service);
- if (service.app != null) {
- service.app.removed = killProcess;
- if (!service.app.isPersistent()) {
- service.app.services.remove(service);
- if (service.whitelistManager) {
- updateWhitelistManagerLocked(service.app);
- }
+ if (service.app != null && !service.app.isPersistent()) {
+ service.app.services.remove(service);
+ if (service.whitelistManager) {
+ updateWhitelistManagerLocked(service.app);
}
}
service.setProcess(null);
@@ -3353,7 +3349,7 @@
}
boolean bringDownDisabledPackageServicesLocked(String packageName, Set<String> filterByClasses,
- int userId, boolean evenPersistent, boolean killProcess, boolean doit) {
+ int userId, boolean evenPersistent, boolean doit) {
boolean didSomething = false;
if (mTmpCollectionResults != null) {
@@ -3363,8 +3359,7 @@
if (userId == UserHandle.USER_ALL) {
for (int i = mServiceMap.size() - 1; i >= 0; i--) {
didSomething |= collectPackageServicesLocked(packageName, filterByClasses,
- evenPersistent, doit, killProcess,
- mServiceMap.valueAt(i).mServicesByInstanceName);
+ evenPersistent, doit, mServiceMap.valueAt(i).mServicesByInstanceName);
if (!doit && didSomething) {
return true;
}
@@ -3377,7 +3372,7 @@
if (smap != null) {
ArrayMap<ComponentName, ServiceRecord> items = smap.mServicesByInstanceName;
didSomething = collectPackageServicesLocked(packageName, filterByClasses,
- evenPersistent, doit, killProcess, items);
+ evenPersistent, doit, items);
}
if (doit && filterByClasses == null) {
forceStopPackageLocked(packageName, userId);
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index eb643b6..6124b3f 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -3875,7 +3875,7 @@
}
synchronized (this) {
mProcessList.killPackageProcessesLocked(packageName, appId, targetUserId,
- ProcessList.SERVICE_ADJ, false, true, true, false, "kill background");
+ ProcessList.SERVICE_ADJ, "kill background");
}
}
} finally {
@@ -4236,7 +4236,7 @@
}
private void cleanupDisabledPackageComponentsLocked(
- String packageName, int userId, boolean killProcess, String[] changedClasses) {
+ String packageName, int userId, String[] changedClasses) {
Set<String> disabledClasses = null;
boolean packageDisabled = false;
@@ -4299,7 +4299,7 @@
// Clean-up disabled services.
mServices.bringDownDisabledPackageServicesLocked(
- packageName, disabledClasses, userId, false, killProcess, true);
+ packageName, disabledClasses, userId, false /* evenPersistent */, true /* doIt */);
// Clean-up disabled providers.
ArrayList<ContentProviderRecord> providers = new ArrayList<>();
@@ -4356,14 +4356,15 @@
}
boolean didSomething = mProcessList.killPackageProcessesLocked(packageName, appId, userId,
- ProcessList.INVALID_ADJ, callerWillRestart, true, doit, evenPersistent,
+ ProcessList.INVALID_ADJ, callerWillRestart, true /* allowRestart */, doit,
+ evenPersistent, true /* setRemoved */,
packageName == null ? ("stop user " + userId) : ("stop " + packageName));
didSomething |=
mAtmInternal.onForceStopPackage(packageName, doit, evenPersistent, userId);
if (mServices.bringDownDisabledPackageServicesLocked(
- packageName, null, userId, evenPersistent, true, doit)) {
+ packageName, null /* filterByClasses */, userId, evenPersistent, doit)) {
if (!doit) {
return true;
}
@@ -8312,9 +8313,10 @@
synchronized (this) {
final long identity = Binder.clearCallingIdentity();
try {
- mProcessList.killPackageProcessesLocked(null, appId, userId,
- ProcessList.PERSISTENT_PROC_ADJ, false, true, true, true,
- reason != null ? reason : "kill uid");
+ mProcessList.killPackageProcessesLocked(null /* packageName */, appId, userId,
+ ProcessList.PERSISTENT_PROC_ADJ, false /* callerWillRestart */,
+ true /* callerWillRestart */, true /* doit */, true /* evenPersistent */,
+ false /* setRemoved */, reason != null ? reason : "kill uid");
} finally {
Binder.restoreCallingIdentity(identity);
}
@@ -14543,10 +14545,9 @@
-1);
mProcessList.killPackageProcessesLocked(ssp,
UserHandle.getAppId(extraUid),
- userId, ProcessList.INVALID_ADJ,
- false, true, true, false, "change " + ssp);
+ userId, ProcessList.INVALID_ADJ, "change " + ssp);
}
- cleanupDisabledPackageComponentsLocked(ssp, userId, killProcess,
+ cleanupDisabledPackageComponentsLocked(ssp, userId,
intent.getStringArrayExtra(
Intent.EXTRA_CHANGED_COMPONENT_NAME_LIST));
}
@@ -17203,10 +17204,8 @@
// We don't kill persistent processes.
continue;
}
- if (app.removed) {
- procs.add(app);
- } else if (app.userId == userHandle && app.hasForegroundActivities()) {
- app.removed = true;
+ if (app.removed
+ || (app.userId == userHandle && app.hasForegroundActivities())) {
procs.add(app);
}
}
@@ -18051,8 +18050,7 @@
try {
synchronized(this) {
mProcessList.killPackageProcessesLocked(packageName, UserHandle.getAppId(pkgUid),
- userId, ProcessList.FOREGROUND_APP_ADJ, false, true, true, false,
- "dep: " + packageName);
+ userId, ProcessList.FOREGROUND_APP_ADJ, "dep: " + packageName);
}
} finally {
Binder.restoreCallingIdentity(callingId);
diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java
index 003ddd1..be6569c 100644
--- a/services/core/java/com/android/server/am/ProcessList.java
+++ b/services/core/java/com/android/server/am/ProcessList.java
@@ -2038,25 +2038,25 @@
// We don't kill persistent processes.
continue;
}
- if (app.removed) {
- procs.add(app);
- } else if (app.setAdj >= ProcessList.CACHED_APP_MIN_ADJ) {
- app.removed = true;
+ if (app.removed || app.setAdj >= ProcessList.CACHED_APP_MIN_ADJ) {
procs.add(app);
}
}
}
+ }
- final int N = procs.size();
- for (int i = 0; i < N; i++) {
- removeProcessLocked(procs.get(i), false, true, "kill all background");
- }
+ @GuardedBy("mService")
+ boolean killPackageProcessesLocked(String packageName, int appId, int userId, int minOomAdj,
+ String reason) {
+ return killPackageProcessesLocked(packageName, appId, userId, minOomAdj,
+ false /* callerWillRestart */, true /* allowRestart */, true /* doit */,
+ false /* evenPersistent */, false /* setRemoved */, reason);
}
@GuardedBy("mService")
final boolean killPackageProcessesLocked(String packageName, int appId,
int userId, int minOomAdj, boolean callerWillRestart, boolean allowRestart,
- boolean doit, boolean evenPersistent, String reason) {
+ boolean doit, boolean evenPersistent, boolean setRemoved, String reason) {
ArrayList<ProcessRecord> procs = new ArrayList<>();
// Remove all processes this package may have touched: all with the
@@ -2114,7 +2114,9 @@
if (!doit) {
return true;
}
- app.removed = true;
+ if (setRemoved) {
+ app.removed = true;
+ }
procs.add(app);
}
}
@@ -2348,11 +2350,8 @@
final int NA = apps.size();
for (int ia = 0; ia < NA; ia++) {
final ProcessRecord app = apps.valueAt(ia);
- if (app.removed) {
- procs.add(app);
- } else if ((minTargetSdk < 0 || app.info.targetSdkVersion < minTargetSdk)
- && (maxProcState < 0 || app.setProcState > maxProcState)) {
- app.removed = true;
+ if (app.removed || ((minTargetSdk < 0 || app.info.targetSdkVersion < minTargetSdk)
+ && (maxProcState < 0 || app.setProcState > maxProcState))) {
procs.add(app);
}
}
diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java
index 054c830..8d8cf1e 100644
--- a/services/core/java/com/android/server/am/ProcessRecord.java
+++ b/services/core/java/com/android/server/am/ProcessRecord.java
@@ -264,7 +264,9 @@
boolean forceCrashReport; // suppress normal auto-dismiss of crash dialog & report UI?
private boolean mNotResponding; // does the app have a not responding dialog?
Dialog anrDialog; // dialog being displayed due to app not resp.
- boolean removed; // has app package been removed from device?
+ volatile boolean removed; // Whether this process should be killed and removed from process
+ // list. It is set when the package is force-stopped or the process
+ // has crashed too many times.
private boolean mDebugging; // was app launched for debugging?
boolean waitedForDebugger; // has process show wait for debugger dialog?
Dialog waitDialog; // current wait for debugger dialog
@@ -1228,10 +1230,8 @@
}
@Override
- public void setRemoved(boolean removed) {
- synchronized (mService) {
- this.removed = removed;
- }
+ public boolean isRemoved() {
+ return removed;
}
/**
diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java
index c97e4e8..4cd5e82 100644
--- a/services/core/java/com/android/server/wm/ActivityStack.java
+++ b/services/core/java/com/android/server/wm/ActivityStack.java
@@ -4673,6 +4673,14 @@
removeHistoryRecordsForAppLocked(mStackSupervisor.mFinishingActivities, app,
"mFinishingActivities");
+ final boolean isProcessRemoved = app.isRemoved();
+ if (isProcessRemoved) {
+ // The package of the died process should be force-stopped, so make its activities as
+ // finishing to prevent the process from being started again if the next top (or being
+ // visible) activity also resides in the same process.
+ app.makeFinishingForProcessRemoved();
+ }
+
boolean hasVisibleActivities = false;
// Clean out the history list.
@@ -4725,7 +4733,7 @@
+ " stateNotNeeded=" + r.stateNotNeeded
+ " finishing=" + r.finishing
+ " state=" + r.getState() + " callers=" + Debug.getCallers(5));
- if (!r.finishing) {
+ if (!r.finishing || isProcessRemoved) {
Slog.w(TAG, "Force removing " + r + ": app died, no saved state");
EventLog.writeEvent(EventLogTags.AM_FINISH_ACTIVITY,
r.mUserId, System.identityHashCode(r),
@@ -5130,12 +5138,6 @@
}
didSomething = true;
Slog.i(TAG, " Force finishing activity " + r);
- if (sameComponent) {
- if (r.hasProcess()) {
- r.app.setRemoved(true);
- }
- r.app = null;
- }
lastTask = r.getTaskRecord();
finishActivityLocked(r, Activity.RESULT_CANCELED, null, "force-stop",
true);
diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java
index 07f26b4..030cc05 100644
--- a/services/core/java/com/android/server/wm/WindowProcessController.java
+++ b/services/core/java/com/android/server/wm/WindowProcessController.java
@@ -416,6 +416,12 @@
mActivities.remove(r);
}
+ void makeFinishingForProcessRemoved() {
+ for (int i = mActivities.size() - 1; i >= 0; --i) {
+ mActivities.get(i).makeFinishingLocked();
+ }
+ }
+
public void clearActivities() {
synchronized (mAtm.mGlobalLock) {
mActivities.clear();
@@ -693,12 +699,8 @@
mAtm.mH.sendMessage(m);
}
- void setRemoved(boolean removed) {
- if (mListener == null) return;
- // Posting on handler so WM lock isn't held when we call into AM.
- final Message m = PooledLambda.obtainMessage(
- WindowProcessListener::setRemoved, mListener, removed);
- mAtm.mH.sendMessage(m);
+ boolean isRemoved() {
+ return mListener == null ? false : mListener.isRemoved();
}
void clearWaitingToKill() {
diff --git a/services/core/java/com/android/server/wm/WindowProcessListener.java b/services/core/java/com/android/server/wm/WindowProcessListener.java
index bce5e2d..d732e4e 100644
--- a/services/core/java/com/android/server/wm/WindowProcessListener.java
+++ b/services/core/java/com/android/server/wm/WindowProcessListener.java
@@ -44,8 +44,11 @@
void updateProcessInfo(boolean updateServiceConnectionActivities, boolean updateLru,
boolean activityChange, boolean updateOomAdj);
- /** Set process package been removed from device. */
- void setRemoved(boolean removed);
+ /**
+ * Returns true if the process is removed and we should completely clean up the related records
+ * belonging to this process.
+ */
+ boolean isRemoved();
/** Returns the total time (in milliseconds) spent executing in both user and system code. */
long getCpuTime();
diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java
index 35c1ede..986943a 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java
@@ -34,6 +34,7 @@
import static com.android.server.wm.ActivityStack.ActivityState.PAUSED;
import static com.android.server.wm.ActivityStack.ActivityState.PAUSING;
import static com.android.server.wm.ActivityStack.ActivityState.RESUMED;
+import static com.android.server.wm.ActivityStack.ActivityState.STOPPED;
import static com.android.server.wm.ActivityStack.ActivityState.STOPPING;
import static com.android.server.wm.ActivityStack.REMOVE_TASK_MODE_DESTROYING;
import static com.android.server.wm.ActivityTaskManagerService.RELAUNCH_REASON_FREE_RESIZE;
@@ -51,6 +52,7 @@
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
+import android.content.ComponentName;
import android.content.pm.ActivityInfo;
import android.os.UserHandle;
import android.platform.test.annotations.Presubmit;
@@ -647,20 +649,50 @@
}
@Test
- public void testFinishDisabledPackageActivities() {
+ public void testFinishDisabledPackageActivities_FinishAliveActivities() {
final ActivityRecord firstActivity = new ActivityBuilder(mService).setTask(mTask).build();
final ActivityRecord secondActivity = new ActivityBuilder(mService).setTask(mTask).build();
+ firstActivity.setState(STOPPED, "testFinishDisabledPackageActivities");
+ secondActivity.setState(RESUMED, "testFinishDisabledPackageActivities");
+ mStack.mResumedActivity = secondActivity;
- // Making the second activity a task overlay without an app means it will be removed from
- // the task's activities as well once first activity is removed.
- secondActivity.mTaskOverlay = true;
- secondActivity.app = null;
+ // Note the activities have non-null ActivityRecord.app, so it won't remove directly.
+ mStack.finishDisabledPackageActivitiesLocked(firstActivity.packageName,
+ null /* filterByClasses */, true /* doit */, true /* evenPersistent */,
+ UserHandle.USER_ALL);
+
+ // If the activity is disabled with {@link android.content.pm.PackageManager#DONT_KILL_APP}
+ // the activity should still follow the normal flow to finish and destroy.
+ assertThat(firstActivity.getState()).isEqualTo(DESTROYING);
+ assertThat(secondActivity.getState()).isEqualTo(PAUSING);
+ assertTrue(secondActivity.finishing);
+ }
+
+ @Test
+ public void testFinishDisabledPackageActivities_RemoveNonAliveActivities() {
+ final ActivityRecord activity = new ActivityBuilder(mService).setTask(mTask).build();
+
+ // The overlay activity is not in the disabled package but it is in the same task.
+ final ActivityRecord overlayActivity = new ActivityBuilder(mService).setTask(mTask)
+ .setComponent(new ComponentName("package.overlay", ".OverlayActivity")).build();
+ // If the task only remains overlay activity, the task should also be removed.
+ // See {@link ActivityStack#removeActivityFromHistoryLocked}.
+ overlayActivity.mTaskOverlay = true;
+
+ // The activity without an app means it will be removed immediately.
+ // See {@link ActivityStack#destroyActivityLocked}.
+ activity.app = null;
+ overlayActivity.app = null;
assertEquals(2, mTask.mActivities.size());
- mStack.finishDisabledPackageActivitiesLocked(firstActivity.packageName, null,
- true /* doit */, true /* evenPersistent */, UserHandle.USER_ALL);
+ mStack.finishDisabledPackageActivitiesLocked(activity.packageName,
+ null /* filterByClasses */, true /* doit */, true /* evenPersistent */,
+ UserHandle.USER_ALL);
+ // Although the overlay activity is in another package, the non-overlay activities are
+ // removed from the task. Since the overlay activity should be removed as well, the task
+ // should be empty.
assertThat(mTask.mActivities).isEmpty();
assertThat(mStack.getAllTasks()).isEmpty();
}
diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java b/services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java
index ea8f33f..3b399ff 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java
@@ -474,6 +474,10 @@
}
@Override
+ void updateCpuStats() {
+ }
+
+ @Override
void updateBatteryStats(ActivityRecord component, boolean resumed) {
}