From 5871539df1f82ad6ff91b9b0d6422d101d76b618 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Mon, 22 Mar 2021 17:51:03 -0700 Subject: [PATCH] Make sure out-of-quota EJs stop in time. If an app runs out of EJ quota while one of its EJs is running, then the running EJ should only be allowed to run for the minimum execution limit (currently 3 minutes). Once that execution time has transpired, the out-of-quota EJ should be stopped. Bug: 171305774 Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job Test: atest CtsJobSchedulerTestCases Change-Id: Iacabac599939204783e417b700a754ffd0a5759c --- .../server/job/JobConcurrencyManager.java | 2 +- .../server/job/JobSchedulerService.java | 6 ++- .../android/server/job/JobServiceContext.java | 53 ++++++++++++++++++- .../server/job/controllers/JobStatus.java | 18 +++++-- .../job/controllers/QuotaController.java | 18 +++---- .../job/controllers/QuotaControllerTest.java | 7 ++- 6 files changed, 82 insertions(+), 22 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java index 325be1b5c3a50..d94d638a7021c 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java @@ -891,7 +891,7 @@ class JobConcurrencyManager { } // Only expedited jobs can replace expedited jobs. - if (js.shouldTreatAsExpeditedJob()) { + if (js.shouldTreatAsExpeditedJob() || js.startedAsExpeditedJob) { // Keep fg/bg user distinction. if (workType == WORK_TYPE_BGUSER_IMPORTANT || workType == WORK_TYPE_BGUSER) { // Let any important bg user job replace a bg user expedited job. diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java index a041f8c0b5127..8ac237e63877c 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -2187,6 +2187,10 @@ public class JobSchedulerService extends com.android.server.SystemService */ @VisibleForTesting boolean isReadyToBeExecutedLocked(JobStatus job) { + return isReadyToBeExecutedLocked(job, true); + } + + boolean isReadyToBeExecutedLocked(JobStatus job, boolean rejectActive) { final boolean jobReady = job.isReady(); if (DEBUG) { @@ -2225,7 +2229,7 @@ public class JobSchedulerService extends com.android.server.SystemService } final boolean jobPending = mPendingJobs.contains(job); - final boolean jobActive = isCurrentlyActiveLocked(job); + final boolean jobActive = rejectActive && isCurrentlyActiveLocked(job); if (DEBUG) { Slog.v(TAG, "isReadyToBeExecutedLocked: " + job.toShortString() diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java index e8bcbfbb2c582..790fae0860de1 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java @@ -151,6 +151,14 @@ public final class JobServiceContext implements ServiceConnection { /** The absolute maximum amount of time the job can run */ private long mMaxExecutionTimeMillis; + /** + * The stop reason for a pending cancel. If there's not pending cancel, then the value should be + * {@link JobParameters#STOP_REASON_UNDEFINED}. + */ + private int mPendingStopReason = JobParameters.STOP_REASON_UNDEFINED; + private int mPendingLegacyStopReason; + private String mPendingDebugStopReason; + // Debugging: reason this job was last stopped. public String mStoppedReason; @@ -328,6 +336,7 @@ public final class JobServiceContext implements ServiceConnection { mAvailable = false; mStoppedReason = null; mStoppedTime = 0; + job.startedAsExpeditedJob = job.shouldTreatAsExpeditedJob(); return true; } } @@ -625,6 +634,19 @@ public final class JobServiceContext implements ServiceConnection { } return; } + if (mRunningJob.startedAsExpeditedJob + && stopReasonCode == JobParameters.STOP_REASON_QUOTA) { + // EJs should be able to run for at least the min upper limit regardless of quota. + final long earliestStopTimeElapsed = + mExecutionStartTimeElapsed + mMinExecutionGuaranteeMillis; + final long nowElapsed = sElapsedRealtimeClock.millis(); + if (nowElapsed < earliestStopTimeElapsed) { + mPendingStopReason = stopReasonCode; + mPendingLegacyStopReason = legacyStopReason; + mPendingDebugStopReason = debugReason; + return; + } + } mParams.setStopReason(stopReasonCode, legacyStopReason, debugReason); if (legacyStopReason == JobParameters.REASON_PREEMPT) { mPreferredUid = mRunningJob != null ? mRunningJob.getUid() : @@ -777,6 +799,23 @@ public final class JobServiceContext implements ServiceConnection { closeAndCleanupJobLocked(true /* needsReschedule */, "timed out while stopping"); break; case VERB_EXECUTING: + if (mPendingStopReason != JobParameters.STOP_REASON_UNDEFINED) { + if (mService.isReadyToBeExecutedLocked(mRunningJob, false)) { + // Job became ready again while we were waiting to stop it (for example, + // the device was temporarily taken off the charger). Ignore the pending + // stop and see what the manager says. + mPendingStopReason = JobParameters.STOP_REASON_UNDEFINED; + mPendingLegacyStopReason = 0; + mPendingDebugStopReason = null; + } else { + Slog.i(TAG, "JS was waiting to stop this job." + + " Sending onStop: " + getRunningJobNameLocked()); + mParams.setStopReason(mPendingStopReason, mPendingLegacyStopReason, + mPendingDebugStopReason); + sendStopMessageLocked(mPendingDebugStopReason); + break; + } + } final long latestStopTimeElapsed = mExecutionStartTimeElapsed + mMaxExecutionTimeMillis; final long nowElapsed = sElapsedRealtimeClock.millis(); @@ -886,6 +925,9 @@ public final class JobServiceContext implements ServiceConnection { mCancelled = false; service = null; mAvailable = true; + mPendingStopReason = JobParameters.STOP_REASON_UNDEFINED; + mPendingLegacyStopReason = 0; + mPendingDebugStopReason = null; removeOpTimeOutLocked(); mCompletedListener.onJobCompletedLocked(completedJob, legacyStopReason, reschedule); mJobConcurrencyManager.onJobCompletedLocked(this, completedJob, workType); @@ -972,7 +1014,16 @@ public final class JobServiceContext implements ServiceConnection { pw.print(", "); TimeUtils.formatDuration( (mExecutionStartTimeElapsed + mMaxExecutionTimeMillis) - nowElapsed, pw); - pw.println("]"); + pw.print("]"); + if (mPendingStopReason != JobParameters.STOP_REASON_UNDEFINED) { + pw.print(" Pending stop because "); + pw.print(mPendingStopReason); + pw.print("/"); + pw.print(mPendingLegacyStopReason); + pw.print("/"); + pw.print(mPendingDebugStopReason); + } + pw.println(); pw.decreaseIndent(); } } diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java index 659cfa7153521..4768ca3417c07 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java @@ -325,6 +325,12 @@ public final class JobStatus { /** The evaluated priority of the job when it started running. */ public int lastEvaluatedPriority; + /** + * Whether or not this particular JobStatus instance was treated as an EJ when it started + * running. This isn't copied over when a job is rescheduled. + */ + public boolean startedAsExpeditedJob = false; + // If non-null, this is work that has been enqueued for the job. public ArrayList pendingWork; @@ -1112,18 +1118,19 @@ public final class JobStatus { */ public boolean canRunInDoze() { return (getFlags() & JobInfo.FLAG_WILL_BE_FOREGROUND) != 0 - || (shouldTreatAsExpeditedJob() + || ((shouldTreatAsExpeditedJob() || startedAsExpeditedJob) && (mDynamicConstraints & CONSTRAINT_DEVICE_NOT_DOZING) == 0); } boolean canRunInBatterySaver() { return (getInternalFlags() & INTERNAL_FLAG_HAS_FOREGROUND_EXEMPTION) != 0 - || (shouldTreatAsExpeditedJob() + || ((shouldTreatAsExpeditedJob() || startedAsExpeditedJob) && (mDynamicConstraints & CONSTRAINT_BACKGROUND_NOT_RESTRICTED) == 0); } boolean shouldIgnoreNetworkBlocking() { - return (getFlags() & JobInfo.FLAG_WILL_BE_FOREGROUND) != 0 || shouldTreatAsExpeditedJob(); + return (getFlags() & JobInfo.FLAG_WILL_BE_FOREGROUND) != 0 + || (shouldTreatAsExpeditedJob() || startedAsExpeditedJob); } /** @return true if the constraint was changed, false otherwise. */ @@ -2022,7 +2029,10 @@ public final class JobStatus { pw.println(serviceInfo != null); if ((getFlags() & JobInfo.FLAG_EXPEDITED) != 0) { pw.print("readyWithinExpeditedQuota: "); - pw.println(mReadyWithinExpeditedQuota); + pw.print(mReadyWithinExpeditedQuota); + pw.print(" (started as EJ: "); + pw.print(startedAsExpeditedJob); + pw.println(")"); } pw.decreaseIndent(); diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java index 2b79969f43782..91189e4b6e741 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java @@ -849,10 +849,9 @@ public final class QuotaController extends StateController { return true; } // A job is within quota if one of the following is true: - // 1. it's already running (already executing expedited jobs should be allowed to finish) - // 2. the app is currently in the foreground - // 3. the app overall is within its quota - // 4. It's on the temp allowlist (or within the grace period) + // 1. the app is currently in the foreground + // 2. the app overall is within its quota + // 3. It's on the temp allowlist (or within the grace period) if (isTopStartedJobLocked(jobStatus) || isUidInForeground(jobStatus.getSourceUid())) { return true; } @@ -873,13 +872,6 @@ public final class QuotaController extends StateController { return true; } - Timer ejTimer = mEJPkgTimers.get(jobStatus.getSourceUserId(), - jobStatus.getSourcePackageName()); - // Any already executing expedited jobs should be allowed to finish. - if (ejTimer != null && ejTimer.isRunning(jobStatus)) { - return true; - } - return 0 < getRemainingEJExecutionTimeLocked( jobStatus.getSourceUserId(), jobStatus.getSourcePackageName()); } @@ -4153,6 +4145,8 @@ public final class QuotaController extends StateController { pw.print(", "); if (js.shouldTreatAsExpeditedJob()) { pw.print("within EJ quota"); + } else if (js.startedAsExpeditedJob) { + pw.print("out of EJ quota"); } else if (js.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_QUOTA)) { pw.print("within regular quota"); } else { @@ -4163,6 +4157,8 @@ public final class QuotaController extends StateController { pw.print(getRemainingEJExecutionTimeLocked( js.getSourceUserId(), js.getSourcePackageName())); pw.print("ms remaining in EJ quota"); + } else if (js.startedAsExpeditedJob) { + pw.print("should be stopped after min execution time"); } else { pw.print(getRemainingExecutionTimeLocked(js)); pw.print("ms remaining in quota"); diff --git a/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java index ee1a4f4b35788..4bab8e51874c7 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java @@ -5416,9 +5416,8 @@ public class QuotaControllerTest { inOrder.verify(mJobSchedulerService, timeout(remainingTimeMs / 2 + 2 * SECOND_IN_MILLIS).times(1)) .onControllerStateChanged(); - // Top and bg EJs should still be allowed to run since they started before the app ran - // out of quota. - assertTrue(jobBg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + // Top should still be "in quota" since it started before the app ran on top out of quota. + assertFalse(jobBg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); assertTrue(jobTop.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); assertFalse( jobUnstarted.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); @@ -5467,7 +5466,7 @@ public class QuotaControllerTest { // wasn't started. Top should remain in quota since it started when the app was in TOP. assertTrue(jobTop2.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); assertFalse(jobFg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertTrue(jobBg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + assertFalse(jobBg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); trackJobs(jobBg2); assertFalse(jobBg2.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); assertFalse(