Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Handle a race condition between process killing and restarting" into sc-dev

Browse Source
This commit is contained in:
Jing Ji
2021-03-25 22:39:13 +00:00
committed by Android (Google) Code Review
parent c7c1446c11 464c84abd7
commit 5d4effc253
3 changed files with 154 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
services/core/java/com/android/server/am/ActivityManagerService.java
View File

@@ -2970,9 +2970,9 @@ public class ActivityManagerService extends IActivityManager.Stub
*/
@GuardedBy("this")
final void handleAppDiedLocked(ProcessRecord app, int pid,
boolean restarting, boolean allowRestart) {
boolean restarting, boolean allowRestart, boolean fromBinderDied) {
boolean kept = cleanUpApplicationRecordLocked(app, pid, restarting, allowRestart, -1,
false /*replacingPid*/);
false /*replacingPid*/, fromBinderDied);
if (!kept && !restarting) {
removeLruProcessLocked(app);
if (pid > 0) {
@@ -3030,12 +3030,15 @@ public class ActivityManagerService extends IActivityManager.Stub
final void appDiedLocked(ProcessRecord app, int pid, IApplicationThread thread,
boolean fromBinderDied, String reason) {
// First check if this ProcessRecord is actually active for the pid.
final ProcessRecord curProc;
synchronized (mPidsSelfLocked) {
ProcessRecord curProc = mPidsSelfLocked.get(pid);
if (curProc != app) {
curProc = mPidsSelfLocked.get(pid);
}
if (curProc != app) {
if (!fromBinderDied || !mProcessList.handleDyingAppDeathLocked(app, pid)) {
Slog.w(TAG, "Spurious death for " + app + ", curProc for " + pid + ": " + curProc);
return;
}
return;
}
mBatteryStatsService.noteProcessDied(app.info.uid, pid);
@@ -3075,7 +3078,7 @@ public class ActivityManagerService extends IActivityManager.Stub
EventLogTags.writeAmProcDied(app.userId, pid, app.processName, setAdj, setProcState);
if (DEBUG_CLEANUP) Slog.v(TAG_CLEANUP,
"Dying app: " + app + ", pid: " + pid + ", thread: " + thread.asBinder());
handleAppDiedLocked(app, pid, false, true);
handleAppDiedLocked(app, pid, false, true, fromBinderDied);
if (doOomAdj) {
updateOomAdjLocked(OomAdjuster.OOM_ADJ_REASON_PROCESS_END);
@@ -4232,7 +4235,7 @@ public class ActivityManagerService extends IActivityManager.Stub
EventLog.writeEvent(0x534e4554, "131105245", app.getStartUid(), msg);
// If there is already an app occupying that pid that hasn't been cleaned up
cleanUpApplicationRecordLocked(app, pid, false, false, -1,
true /*replacingPid*/);
true /*replacingPid*/, false /* fromBinderDied */);
removePidLocked(pid, app);
app = null;
}
@@ -4274,7 +4277,7 @@ public class ActivityManagerService extends IActivityManager.Stub
// If this application record is still attached to a previous
// process, clean it up now.
if (app.getThread() != null) {
handleAppDiedLocked(app, pid, true, true);
handleAppDiedLocked(app, pid, true, true, false /* fromBinderDied */);
}
// Tell the process all about itself.
@@ -4477,7 +4480,7 @@ public class ActivityManagerService extends IActivityManager.Stub
app.unlinkDeathRecipient();
app.killLocked("error during bind", ApplicationExitInfo.REASON_INITIALIZATION_FAILURE,
true);
handleAppDiedLocked(app, pid, false, true);
handleAppDiedLocked(app, pid, false, true, false /* fromBinderDied */);
return false;
}
@@ -4542,7 +4545,7 @@ public class ActivityManagerService extends IActivityManager.Stub
if (badApp) {
app.killLocked("error during init", ApplicationExitInfo.REASON_INITIALIZATION_FAILURE,
true);
handleAppDiedLocked(app, pid, false, true);
handleAppDiedLocked(app, pid, false, true, false /* fromBinderDied */);
return false;
}
@@ -11500,7 +11503,8 @@ public class ActivityManagerService extends IActivityManager.Stub
*/
@GuardedBy("this")
final boolean cleanUpApplicationRecordLocked(ProcessRecord app, int pid,
boolean restarting, boolean allowRestart, int index, boolean replacingPid) {
boolean restarting, boolean allowRestart, int index, boolean replacingPid,
boolean fromBinderDied) {
boolean restart;
synchronized (mProcLock) {
if (index >= 0) {
@@ -11508,7 +11512,10 @@ public class ActivityManagerService extends IActivityManager.Stub
ProcessList.remove(pid);
}
restart = app.onCleanupApplicationRecordLSP(mProcessStats, allowRestart);
// We don't want to unlinkDeathRecipient immediately, if it's not called from binder
// and it's not isolated, as we'd need the signal to bookkeeping the dying process list.
restart = app.onCleanupApplicationRecordLSP(mProcessStats, allowRestart,
fromBinderDied || app.isolated /* unlinkDeath */);
}
mAppProfiler.onCleanupApplicationRecordLocked(app);
skipCurrentReceiverLocked(app);
@@ -11538,25 +11545,7 @@ public class ActivityManagerService extends IActivityManager.Stub
mProcessList.scheduleDispatchProcessDiedLocked(pid, app.info.uid);
// If this is a preceding instance of another process instance
allowRestart = true;
synchronized (app) {
if (app.mSuccessor != null) {
// We don't allow restart with this ProcessRecord now,
// because we have created a new one already.
allowRestart = false;
// If it's persistent, add the successor to mPersistentStartingProcesses
if (app.isPersistent() && !app.isRemoved()) {
if (mPersistentStartingProcesses.indexOf(app.mSuccessor) < 0) {
mPersistentStartingProcesses.add(app.mSuccessor);
}
}
// clean up the field so the successor's proc starter could proceed.
app.mSuccessor.mPredecessor = null;
app.mSuccessor = null;
// Notify if anyone is waiting for it.
app.notifyAll();
}
}
allowRestart = mProcessList.handlePrecedingAppDiedLocked(app);
// If the caller is restarting this app, then leave it in its
// current lists and let the caller take care of it.
@@ -14652,7 +14641,8 @@ public class ActivityManagerService extends IActivityManager.Stub
}
}
didSomething = true;
cleanUpApplicationRecordLocked(app, pid, false, true, -1, false /*replacingPid*/);
cleanUpApplicationRecordLocked(app, pid, false, true, -1, false /*replacingPid*/,
false /* fromBinderDied */);
mProcessList.mRemovedProcesses.remove(i);
if (app.isPersistent()) {

115
services/core/java/com/android/server/am/ProcessList.java
View File

@@ -685,6 +685,12 @@ public final class ProcessList {
@GuardedBy("mService")
final ArrayList<ProcessRecord> mRemovedProcesses = new ArrayList<ProcessRecord>();
/**
* Processes that are killed by us and being waiting for the death notification.
*/
@GuardedBy("mService")
final ProcessMap<ProcessRecord> mDyingProcesses = new ProcessMap<>();
// Self locked with the inner lock within the RemoteCallbackList
private final RemoteCallbackList<IProcessObserver> mProcessObservers =
new RemoteCallbackList<>();
@@ -1754,6 +1760,9 @@ public final class ProcessList {
app.setPid(0);
app.setStartSeq(0);
}
// Clear any residual death recipient link as the ProcessRecord could be reused.
app.unlinkDeathRecipient();
app.setDyingPid(0);
if (DEBUG_PROCESSES && mService.mProcessesOnHold.contains(app)) Slog.v(
TAG_PROCESSES,
@@ -2050,14 +2059,14 @@ public final class ProcessList {
// If there is a preceding instance of the process, wait for its death with a timeout.
// Use local reference since we are not using locks here
final ProcessRecord predecessor = app.mPredecessor;
if (predecessor != null) {
final int pid = predecessor.getPid();
int prevPid;
if (predecessor != null && (prevPid = predecessor.getDyingPid()) > 0) {
long now = System.currentTimeMillis();
final long end = now + PROC_KILL_TIMEOUT;
final int oldPolicy = StrictMode.getThreadPolicyMask();
try {
StrictMode.setThreadPolicyMask(0);
Process.waitForProcessDeath(pid, PROC_KILL_TIMEOUT);
Process.waitForProcessDeath(prevPid, PROC_KILL_TIMEOUT);
// It's killed successfully, but we'd make sure the cleanup work is done.
synchronized (predecessor) {
if (app.mPredecessor != null) {
@@ -2067,17 +2076,22 @@ public final class ProcessList {
predecessor.wait(end - now);
} catch (InterruptedException e) {
}
if (System.currentTimeMillis() >= end) {
Slog.w(TAG, predecessor + " " + prevPid
+ " has died but its obituary delivery is slow.");
}
}
}
if (app.mPredecessor != null) {
if (app.mPredecessor != null && app.mPredecessor.getPid() > 0) {
// The cleanup work hasn't be done yet, let's log it and continue.
Slog.w(TAG, predecessor + " has died, but its cleanup isn't done");
Slog.w(TAG, predecessor + " " + prevPid
+ " has died, but its cleanup isn't done");
}
}
} catch (Exception e) {
// It's still alive... maybe blocked at uninterruptible sleep ?
Slog.wtf(TAG, predecessor.toString() + " refused to die, but we need to launch "
+ app, e);
Slog.wtf(TAG, predecessor.toString() + " " + prevPid
+ " refused to die, but we need to launch " + app, e);
} finally {
StrictMode.setThreadPolicyMask(oldPolicy);
}
@@ -2449,6 +2463,18 @@ public final class ProcessList {
// routine of it yet, but we'd set it as the predecessor of the new process.
predecessor = app;
app = null;
} else if (!isolated) {
// This app may have been removed from process name maps, probably because we killed it
// and did the cleanup before the actual death notification. Check the dying processes.
predecessor = mDyingProcesses.get(processName, info.uid);
if (predecessor != null) {
if (app != null) {
app.mPredecessor = predecessor;
predecessor.mSuccessor = app;
}
Slog.w(TAG_PROCESSES, predecessor.toString() + " is attached to a previous process "
+ predecessor.getDyingPid());
}
}
if (app == null) {
@@ -2617,7 +2643,7 @@ public final class ProcessList {
+ " belongs to another existing app:" + oldApp.processName
+ " startSeq:" + oldApp.getStartSeq());
mService.cleanUpApplicationRecordLocked(oldApp, pid, false, false, -1,
true /*replacingPid*/);
true /*replacingPid*/, false /* fromBinderDied */);
}
mService.addPidLocked(app);
synchronized (mService.mPidsSelfLocked) {
@@ -2831,7 +2857,8 @@ public final class ProcessList {
}
}
app.killLocked(reason, reasonCode, subReason, true);
mService.handleAppDiedLocked(app, pid, willRestart, allowRestart);
mService.handleAppDiedLocked(app, pid, willRestart, allowRestart,
false /* fromBinderDied */);
if (willRestart) {
removeLruProcessLocked(app);
mService.addAppLocked(app.info, null, false, null /* ABI override */,
@@ -4861,6 +4888,55 @@ public final class ProcessList {
return EVENT_INPUT;
}
/**
* Handle the death notification if it's a dying app.
*
* @return {@code true} if it's a dying app that we were tracking.
*/
@GuardedBy("mService")
boolean handleDyingAppDeathLocked(ProcessRecord app, int pid) {
if (mProcessNames.get(app.processName, app.uid) != app
&& mDyingProcesses.get(app.processName, app.uid) == app) {
// App has been removed already, meaning cleanup has done.
Slog.v(TAG, "Got obituary of " + pid + ":" + app.processName);
app.unlinkDeathRecipient();
handlePrecedingAppDiedLocked(app);
// It's really gone now, let's remove from the dying process list.
mDyingProcesses.remove(app.processName, app.uid);
app.setDyingPid(0);
return true;
}
return false;
}
/**
* Handle the case where the given app is a preceding instance of another process instance.
*
* @return {@code false} if this given app should not be allowed to restart.
*/
@GuardedBy("mService")
boolean handlePrecedingAppDiedLocked(ProcessRecord app) {
synchronized (app) {
if (app.mSuccessor != null) {
// We don't allow restart with this ProcessRecord now,
// because we have created a new one already.
// If it's persistent, add the successor to mPersistentStartingProcesses
if (app.isPersistent() && !app.isRemoved()) {
if (mService.mPersistentStartingProcesses.indexOf(app.mSuccessor) < 0) {
mService.mPersistentStartingProcesses.add(app.mSuccessor);
}
}
// clean up the field so the successor's proc starter could proceed.
app.mSuccessor.mPredecessor = null;
app.mSuccessor = null;
// Notify if anyone is waiting for it.
app.notifyAll();
return false;
}
}
return true;
}
/**
* Called by ActivityManagerService when a process died.
*/
@@ -4871,21 +4947,33 @@ public final class ProcessList {
}
Watchdog.getInstance().processDied(app.processName, app.getPid());
if (app.getDeathRecipient() == null) {
// If we've done unlinkDeathRecipient before calling into this, remove from dying list.
mDyingProcesses.remove(app.processName, app.uid);
app.setDyingPid(0);
}
mAppExitInfoTracker.scheduleNoteProcessDied(app);
}
/**
* Called by ActivityManagerService when it decides to kill an application process.
*/
@GuardedBy("mService")
void noteAppKill(final ProcessRecord app, final @Reason int reason,
final @SubReason int subReason, final String msg) {
if (DEBUG_PROCESSES) {
Slog.i(TAG, "note: " + app + " is being killed, reason: " + reason
+ ", sub-reason: " + subReason + ", message: " + msg);
}
if (app.getPid() > 0 && !app.isolated && app.getDeathRecipient() != null) {
// We are killing it, put it into the dying process list.
mDyingProcesses.put(app.processName, app.uid, app);
app.setDyingPid(app.getPid());
}
mAppExitInfoTracker.scheduleNoteAppKill(app, reason, subReason, msg);
}
@GuardedBy("mService")
void noteAppKill(final int pid, final int uid, final @Reason int reason,
final @SubReason int subReason, final String msg) {
if (DEBUG_PROCESSES) {
@@ -4893,6 +4981,15 @@ public final class ProcessList {
+ ", sub-reason: " + subReason + ", message: " + msg);
}
final ProcessRecord app;
synchronized (mService.mPidsSelfLocked) {
app = mService.mPidsSelfLocked.get(pid);
}
if (app != null && app.uid == uid && !app.isolated && app.getDeathRecipient() != null) {
// We are killing it, put it into the dying process list.
mDyingProcesses.put(app.processName, uid, app);
app.setDyingPid(app.getPid());
}
mAppExitInfoTracker.scheduleNoteAppKill(pid, uid, reason, subReason, msg);
}

28
services/core/java/com/android/server/am/ProcessRecord.java
View File

@@ -106,6 +106,12 @@ class ProcessRecord implements WindowProcessListener {
@CompositeRWLock({"mService", "mProcLock"})
int mPid;
/**
* The process ID which will be set when we're killing this process.
*/
@GuardedBy("mService")
private int mDyingPid;
/**
* The gids this process was launched with.
*/
@@ -571,6 +577,16 @@ class ProcessRecord implements WindowProcessListener {
mProfile.onProcessInactive(tracker);
}
@GuardedBy("mService")
int getDyingPid() {
return mDyingPid;
}
@GuardedBy("mService")
void setDyingPid(int dyingPid) {
mDyingPid = dyingPid;
}
@GuardedBy("mService")
int[] getGids() {
return mGids;
@@ -732,6 +748,11 @@ class ProcessRecord implements WindowProcessListener {
mDeathRecipient = deathRecipient;
}
@GuardedBy("mService")
IBinder.DeathRecipient getDeathRecipient() {
return mDeathRecipient;
}
@GuardedBy({"mService", "mProcLock"})
void setActiveInstrumentation(ActiveInstrumentation instr) {
mInstr = instr;
@@ -896,11 +917,14 @@ class ProcessRecord implements WindowProcessListener {
}
@GuardedBy({"mService", "mProcLock"})
boolean onCleanupApplicationRecordLSP(ProcessStatsService processStats, boolean allowRestart) {
boolean onCleanupApplicationRecordLSP(ProcessStatsService processStats, boolean allowRestart,
boolean unlinkDeath) {
mErrorState.onCleanupApplicationRecordLSP();
resetPackageList(processStats);
unlinkDeathRecipient();
if (unlinkDeath) {
unlinkDeathRecipient();
}
makeInactive(processStats);
setWaitingToKill(null);