diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 2bceb20fa86fe..0f72c3ef9f97e 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -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()) { diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index 2cde423bd0a35..fae941d172a21 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -685,6 +685,12 @@ public final class ProcessList { @GuardedBy("mService") final ArrayList mRemovedProcesses = new ArrayList(); + /** + * Processes that are killed by us and being waiting for the death notification. + */ + @GuardedBy("mService") + final ProcessMap mDyingProcesses = new ProcessMap<>(); + // Self locked with the inner lock within the RemoteCallbackList private final RemoteCallbackList 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); } diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java index cb23b898a4676..ed136afcc592a 100644 --- a/services/core/java/com/android/server/am/ProcessRecord.java +++ b/services/core/java/com/android/server/am/ProcessRecord.java @@ -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);