From d2aa4e1913c05d8c21e81eef3ad941565aded7d6 Mon Sep 17 00:00:00 2001 From: Amith Yamasani Date: Mon, 9 Jul 2018 12:07:35 -0700 Subject: [PATCH] Fix for incorrect cycle evaluation in computeOomAdj DO NOT MERGE Use the conservative value of adj and procstate if at least one evaluation pass was completed, even if the value is not final. The later iterations through the procs that have cycles will elevate the apps if necessary. Otherwise the dependencies will just get stuck in a low state. Bug: 79643956 Test: Manual test of connecting to AA and turning off screen atest CtsAppTestCases:ActivityManagerProcessStateTest Change-Id: If520eb239935782e2487b16e8bb650ded775f184 --- .../server/am/ActivityManagerService.java | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index d8d63927f5e13..e032afb408e10 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -23042,6 +23042,7 @@ public class ActivityManagerService extends IActivityManager.Stub // The process is being computed, so there is a cycle. We cannot // rely on this process's state. app.containsCycle = true; + return false; } } @@ -23066,6 +23067,7 @@ public class ActivityManagerService extends IActivityManager.Stub final int logUid = mCurOomAdjUid; int prevAppAdj = app.curAdj; + int prevProcState = app.curProcState; if (app.maxAdj <= ProcessList.FOREGROUND_APP_ADJ) { // The max adjustment doesn't allow this app to be anything @@ -23544,11 +23546,16 @@ public class ActivityManagerService extends IActivityManager.Stub ProcessRecord client = cr.binding.client; computeOomAdjLocked(client, cachedAdj, TOP_APP, doingAll, now); if (client.containsCycle) { - // We've detected a cycle. We should ignore this connection and allow - // this process to retry computeOomAdjLocked later in case a later-checked - // connection from a client would raise its priority legitimately. + // We've detected a cycle. We should retry computeOomAdjLocked later in + // case a later-checked connection from a client would raise its + // priority legitimately. app.containsCycle = true; - continue; + // If the client has not been completely evaluated, skip using its + // priority. Else use the conservative value for now and look for a + // better state in the next iteration. + if (client.completedAdjSeq < mAdjSeq) { + continue; + } } int clientAdj = client.curRawAdj; int clientProcState = client.curProcState; @@ -23771,11 +23778,16 @@ public class ActivityManagerService extends IActivityManager.Stub } computeOomAdjLocked(client, cachedAdj, TOP_APP, doingAll, now); if (client.containsCycle) { - // We've detected a cycle. We should ignore this connection and allow - // this process to retry computeOomAdjLocked later in case a later-checked - // connection from a client would raise its priority legitimately. + // We've detected a cycle. We should retry computeOomAdjLocked later in + // case a later-checked connection from a client would raise its + // priority legitimately. app.containsCycle = true; - continue; + // If the client has not been completely evaluated, skip using its + // priority. Else use the conservative value for now and look for a + // better state in the next iteration. + if (client.completedAdjSeq < mAdjSeq) { + continue; + } } int clientAdj = client.curRawAdj; int clientProcState = client.curProcState; @@ -24007,8 +24019,8 @@ public class ActivityManagerService extends IActivityManager.Stub app.foregroundActivities = foregroundActivities; app.completedAdjSeq = mAdjSeq; - // if curAdj is less than prevAppAdj, then this process was promoted - return app.curAdj < prevAppAdj; + // if curAdj or curProcState improved, then this process was promoted + return app.curAdj < prevAppAdj || app.curProcState < prevProcState; } /** @@ -25061,7 +25073,7 @@ public class ActivityManagerService extends IActivityManager.Stub // - Continue retrying until no process was promoted. // - Iterate from least important to most important. int cycleCount = 0; - while (retryCycles) { + while (retryCycles && cycleCount < 10) { cycleCount++; retryCycles = false; @@ -25076,12 +25088,14 @@ public class ActivityManagerService extends IActivityManager.Stub for (int i=0; i=0; i--) { ProcessRecord app = mLruProcesses.get(i); if (!app.killedByAm && app.thread != null) {