diff options
3 files changed, 138 insertions, 77 deletions
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 8b547c6bda42..25fda6f493d4 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -6860,6 +6860,7 @@ public class ActivityManagerService extends IActivityManager.Stub } } + ProcessRecord dyingProc = null; if (cpr != null && cpr.proc != null) { providerRunning = !cpr.proc.killed; @@ -6869,14 +6870,9 @@ public class ActivityManagerService extends IActivityManager.Stub // (See the commit message on I2c4ba1e87c2d47f2013befff10c49b3dc337a9a7 to see // how to test this case.) if (cpr.proc.killed && cpr.proc.killedByAm) { - final long iden = Binder.clearCallingIdentity(); - try { - mProcessList.killProcAndWaitIfNecessaryLocked(cpr.proc, false, - cpr.uid == cpr.proc.uid || cpr.proc.isolated, - "getContentProviderImpl: %s (killedByAm)", startTime); - } finally { - Binder.restoreCallingIdentity(iden); - } + Slog.wtf(TAG, cpr.proc.toString() + " was killed by AM but isn't really dead"); + // Now we are going to wait for the death before starting the new process. + dyingProc = cpr.proc; } } @@ -6977,18 +6973,18 @@ public class ActivityManagerService extends IActivityManager.Stub // has been killed on us. We need to wait for a new // process to be started, and make sure its death // doesn't kill our process. - Slog.i(TAG, "Existing provider " + cpr.name.flattenToShortString() + Slog.wtf(TAG, "Existing provider " + cpr.name.flattenToShortString() + " is crashing; detaching " + r); boolean lastRef = decProviderCountLocked(conn, cpr, token, stable); - mProcessList.killProcAndWaitIfNecessaryLocked(cpr.proc, - false, true, "getContentProviderImpl: %s", startTime); if (!lastRef) { // This wasn't the last ref our process had on - // the provider... we have now been killed, bail. + // the provider... we will be killed during cleaning up, bail. return null; } + // We'll just start a new process to host the content provider providerRunning = false; conn = null; + dyingProc = cpr.proc; } else { cpr.proc.verifiedAdj = cpr.proc.setAdj; } @@ -7064,7 +7060,7 @@ public class ActivityManagerService extends IActivityManager.Stub checkTime(startTime, "getContentProviderImpl: before getProviderByClass"); cpr = mProviderMap.getProviderByClass(comp, userId); checkTime(startTime, "getContentProviderImpl: after getProviderByClass"); - final boolean firstClass = cpr == null; + boolean firstClass = cpr == null; if (firstClass) { final long ident = Binder.clearCallingIdentity(); @@ -7095,6 +7091,13 @@ public class ActivityManagerService extends IActivityManager.Stub } finally { Binder.restoreCallingIdentity(ident); } + } else if (dyingProc == cpr.proc) { + // The old stable connection's client should be killed during proc cleaning up, + // so do not re-use the old ContentProviderRecord, otherwise the new clients + // could get killed unexpectedly. + cpr = new ContentProviderRecord(cpr); + // This is sort of "firstClass" + firstClass = true; } checkTime(startTime, "getContentProviderImpl: now have ContentProviderRecord"); @@ -14208,10 +14211,19 @@ public class ActivityManagerService extends IActivityManager.Stub cpr.launchingApp = null; cpr.notifyAll(); } - mProviderMap.removeProviderByClass(cpr.name, UserHandle.getUserId(cpr.uid)); + final int userId = UserHandle.getUserId(cpr.uid); + // Don't remove from provider map if it doesn't match + // could be a new content provider is starting + if (mProviderMap.getProviderByClass(cpr.name, userId) == cpr) { + mProviderMap.removeProviderByClass(cpr.name, userId); + } String names[] = cpr.info.authority.split(";"); for (int j = 0; j < names.length; j++) { - mProviderMap.removeProviderByName(names[j], UserHandle.getUserId(cpr.uid)); + // Don't remove from provider map if it doesn't match + // could be a new content provider is starting + if (mProviderMap.getProviderByName(names[j], userId) == cpr) { + mProviderMap.removeProviderByName(names[j], userId); + } } } @@ -14306,6 +14318,10 @@ public class ActivityManagerService extends IActivityManager.Stub // Remove published content providers. for (int i = app.pubProviders.size() - 1; i >= 0; i--) { ContentProviderRecord cpr = app.pubProviders.valueAt(i); + if (cpr.proc != app) { + // If the hosting process record isn't really us, bail out + continue; + } final boolean alwaysRemove = app.bad || !allowRestart; final boolean inLaunching = removeDyingProviderLocked(app, cpr, alwaysRemove); if (!alwaysRemove && inLaunching && cpr.hasConnectionOrHandle()) { @@ -14391,6 +14407,27 @@ public class ActivityManagerService extends IActivityManager.Stub mUiHandler.obtainMessage(DISPATCH_PROCESS_DIED_UI_MSG, app.pid, app.info.uid, null).sendToTarget(); + // If this is a precede 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.removed) { + if (mPersistentStartingProcesses.indexOf(app.mSuccessor) < 0) { + mPersistentStartingProcesses.add(app.mSuccessor); + } + } + // clean up the field so the successor's proc starter could proceed. + app.mSuccessor.mPrecedence = null; + app.mSuccessor = null; + // Notify if anyone is waiting for it. + app.notifyAll(); + } + } + // If the caller is restarting this app, then leave it in its // current lists and let the caller take care of it. if (restarting) { @@ -14420,7 +14457,7 @@ public class ActivityManagerService extends IActivityManager.Stub mAtmInternal.onCleanUpApplicationRecord(app.getWindowProcessController()); mProcessList.noteProcessDiedLocked(app); - if (restart && !app.isolated) { + if (restart && allowRestart && !app.isolated) { // We have components that still need to be running in the // process, so re-launch it. if (index < 0) { diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index 73ca39b22de1..f592824e42be 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -1842,26 +1842,9 @@ public final class ProcessList { if (mService.mConstants.FLAG_PROCESS_START_ASYNC) { if (DEBUG_PROCESSES) Slog.i(TAG_PROCESSES, "Posting procStart msg for " + app.toShortString()); - mService.mProcStartHandler.post(() -> { - try { - final Process.ProcessStartResult startResult = startProcess(app.hostingRecord, - entryPoint, app, app.startUid, gids, runtimeFlags, mountExternal, - app.seInfo, requiredAbi, instructionSet, invokeWith, app.startTime); - synchronized (mService) { - handleProcessStartedLocked(app, startResult, startSeq); - } - } catch (RuntimeException e) { - synchronized (mService) { - Slog.e(ActivityManagerService.TAG, "Failure starting process " - + app.processName, e); - mPendingStarts.remove(startSeq); - app.pendingStart = false; - mService.forceStopPackageLocked(app.info.packageName, - UserHandle.getAppId(app.uid), - false, false, true, false, false, app.userId, "start failure"); - } - } - }); + mService.mProcStartHandler.post(() -> handleProcessStart( + app, entryPoint, gids, runtimeFlags, mountExternal, requiredAbi, + instructionSet, invokeWith, startSeq)); return true; } else { try { @@ -1882,6 +1865,66 @@ public final class ProcessList { } } + /** + * Main handler routine to start the given process from the ProcStartHandler. + * + * <p>Note: this function doesn't hold the global AM lock intentionally.</p> + */ + private void handleProcessStart(final ProcessRecord app, final String entryPoint, + final int[] gids, final int runtimeFlags, final int mountExternal, + final String requiredAbi, final String instructionSet, + final String invokeWith, final long startSeq) { + // If there is a precede instance of the process, wait for its death with a timeout. + // Use local reference since we are not using locks here + final ProcessRecord precedence = app.mPrecedence; + if (precedence != null) { + final int pid = precedence.pid; + long now = System.currentTimeMillis(); + final long end = now + PROC_KILL_TIMEOUT; + try { + Process.waitForProcessDeath(pid, PROC_KILL_TIMEOUT); + // It's killed successfully, but we'd make sure the cleanup work is done. + synchronized (precedence) { + if (app.mPrecedence != null) { + now = System.currentTimeMillis(); + if (now < end) { + try { + precedence.wait(end - now); + } catch (InterruptedException e) { + } + } + } + if (app.mPrecedence != null) { + // The cleanup work hasn't be done yet, let's log it and continue. + Slog.w(TAG, precedence + " has died, but its cleanup isn't done"); + } + } + } catch (Exception e) { + // It's still alive... + Slog.wtf(TAG, precedence.toString() + " refused to die, but we need to launch " + + app); + } + } + try { + final Process.ProcessStartResult startResult = startProcess(app.hostingRecord, + entryPoint, app, app.startUid, gids, runtimeFlags, mountExternal, + app.seInfo, requiredAbi, instructionSet, invokeWith, app.startTime); + synchronized (mService) { + handleProcessStartedLocked(app, startResult, startSeq); + } + } catch (RuntimeException e) { + synchronized (mService) { + Slog.e(ActivityManagerService.TAG, "Failure starting process " + + app.processName, e); + mPendingStarts.remove(startSeq); + app.pendingStart = false; + mService.forceStopPackageLocked(app.info.packageName, + UserHandle.getAppId(app.uid), + false, false, true, false, false, app.userId, "start failure"); + } + } + } + @GuardedBy("mService") public void killAppZygoteIfNeededLocked(AppZygote appZygote, boolean force) { final ApplicationInfo appInfo = appZygote.getAppInfo(); @@ -2136,6 +2179,7 @@ public final class ProcessList { + " app=" + app + " knownToBeDead=" + knownToBeDead + " thread=" + (app != null ? app.thread : null) + " pid=" + (app != null ? app.pid : -1)); + ProcessRecord precedence = null; if (app != null && app.pid > 0) { if ((!knownToBeDead && !app.killed) || app.thread == null) { // We already have the app running, or are waiting for it to @@ -2150,9 +2194,15 @@ public final class ProcessList { // An application record is attached to a previous process, // clean it up now. if (DEBUG_PROCESSES) Slog.v(TAG_PROCESSES, "App died: " + app); - // do the killing - killProcAndWaitIfNecessaryLocked(app, true, app.uid == info.uid || app.isolated, - "startProcess: bad proc running, killing: %s", startTime); + checkSlow(startTime, "startProcess: bad proc running, killing"); + ProcessList.killProcessGroup(app.uid, app.pid); + checkSlow(startTime, "startProcess: done killing old proc"); + + Slog.wtf(TAG_PROCESSES, app.toString() + " is attached to a previous process"); + // We are not going to re-use the ProcessRecord, as we haven't dealt with the cleanup + // routine of it yet, but we'd set it as the precedence of the new process. + precedence = app; + app = null; } if (app == null) { @@ -2166,6 +2216,10 @@ public final class ProcessList { app.crashHandler = crashHandler; app.isolatedEntryPoint = entryPoint; app.isolatedEntryPointArgs = entryPointArgs; + if (precedence != null) { + app.mPrecedence = precedence; + precedence.mSuccessor = app; + } checkSlow(startTime, "startProcess: done creating new process record"); } else { // If this is a new package in the process, add the package to the list @@ -2193,44 +2247,6 @@ public final class ProcessList { return success ? app : null; } - /** - * Kill (if asked to) and wait for the given process died if necessary - * @param app - The process record to kill - * @param doKill - Kill the given process record - * @param wait - Wait for the death of the given process - * @param formatString - The log message for slow operation - * @param startTime - The start timestamp of the operation - */ - @GuardedBy("mService") - void killProcAndWaitIfNecessaryLocked(final ProcessRecord app, final boolean doKill, - final boolean wait, final String formatString, final long startTime) { - - checkSlow(startTime, String.format(formatString, "before appDied")); - - if (doKill) { - // do the killing - ProcessList.killProcessGroup(app.uid, app.pid); - noteAppKill(app, ApplicationExitInfo.REASON_OTHER, - ApplicationExitInfo.SUBREASON_UNKNOWN, - String.format(formatString, "")); - } - - // wait for the death - if (wait) { - try { - Process.waitForProcessDeath(app.pid, PROC_KILL_TIMEOUT); - } catch (Exception e) { - // Maybe the process goes into zombie, use an expensive API to check again. - if (mService.isProcessAliveLocked(app)) { - Slog.w(TAG, String.format(formatString, - "waiting for app killing timed out")); - } - } - } - - checkSlow(startTime, String.format(formatString, "after appDied")); - } - @GuardedBy("mService") private String isProcStartValidLocked(ProcessRecord app, long expectedStartSeq) { StringBuilder sb = null; diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java index 1e2dd2deb568..0e3c6d367c74 100644 --- a/services/core/java/com/android/server/am/ProcessRecord.java +++ b/services/core/java/com/android/server/am/ProcessRecord.java @@ -320,6 +320,14 @@ class ProcessRecord implements WindowProcessListener { // set of disabled compat changes for the process (all others are enabled) long[] mDisabledCompatChanges; + // The precede instance of the process, which would exist when the previous process is killed + // but not fully dead yet; in this case, the new instance of the process should be held until + // this precede instance is fully dead. + volatile ProcessRecord mPrecedence; + // The succeeding instance of the process, which is going to be started after this process + // is killed successfully. + volatile ProcessRecord mSuccessor; + // Cached task info for OomAdjuster private static final int VALUE_INVALID = -1; private static final int VALUE_FALSE = 0; |