From 62bbd4f6bc2a1fbb700034fa7c9ff1d83b0de1c2 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Wed, 3 Mar 2021 12:31:54 -0800 Subject: [PATCH] Disallow EJs that time out from running in Doze or battery saver. If an EJ has used its minimum runtime guarantee before, then we shouldn't let it run in Doze or battery saver again. The restriction will persist across reschedules. Bug: 171305774 Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job Test: atest CtsJobSchedulerTestCases Change-Id: Id63ea89062b6269e87518123df5d65441f3a7923 --- .../server/job/JobCompletedListener.java | 4 +- .../server/job/JobSchedulerService.java | 10 ++++- .../android/server/job/JobServiceContext.java | 13 +++--- .../java/com/android/server/job/JobStore.java | 2 +- .../controllers/DeviceIdleJobsController.java | 19 ++++---- .../server/job/controllers/JobStatus.java | 43 +++++++++++++++---- .../ConnectivityControllerTest.java | 2 +- .../server/job/controllers/JobStatusTest.java | 2 +- .../com/android/server/job/JobStoreTest.java | 2 +- 9 files changed, 65 insertions(+), 32 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobCompletedListener.java b/apex/jobscheduler/service/java/com/android/server/job/JobCompletedListener.java index 34ba753b3daac..862d8b7cac508 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobCompletedListener.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobCompletedListener.java @@ -25,7 +25,9 @@ import com.android.server.job.controllers.JobStatus; public interface JobCompletedListener { /** * Callback for when a job is completed. + * + * @param stopReason The stop reason provided to JobParameters. * @param needsReschedule Whether the implementing class should reschedule this job. */ - void onJobCompletedLocked(JobStatus jobStatus, boolean needsReschedule); + void onJobCompletedLocked(JobStatus jobStatus, int stopReason, boolean needsReschedule); } 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 8bb03e911528e..515cb747a99e6 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -1745,11 +1745,12 @@ public class JobSchedulerService extends com.android.server.SystemService * A job just finished executing. We fetch the * {@link com.android.server.job.controllers.JobStatus} from the store and depending on * whether we want to reschedule we re-add it to the controllers. - * @param jobStatus Completed job. + * + * @param jobStatus Completed job. * @param needsReschedule Whether the implementing class should reschedule this job. */ @Override - public void onJobCompletedLocked(JobStatus jobStatus, boolean needsReschedule) { + public void onJobCompletedLocked(JobStatus jobStatus, int stopReason, boolean needsReschedule) { if (DEBUG) { Slog.d(TAG, "Completed " + jobStatus + ", reschedule=" + needsReschedule); } @@ -1767,6 +1768,11 @@ public class JobSchedulerService extends com.android.server.SystemService // we stop it. final JobStatus rescheduledJob = needsReschedule ? getRescheduleJobForFailureLocked(jobStatus) : null; + if (rescheduledJob != null + && (stopReason == JobParameters.REASON_TIMEOUT + || stopReason == JobParameters.REASON_PREEMPT)) { + rescheduledJob.disallowRunInBatterySaverAndDoze(); + } // Do not write back immediately if this is a periodic job. The job may get lost if system // shuts down before it is added back. 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 2a23d60d8af6b..c40772fdc0bc6 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java @@ -379,8 +379,8 @@ public final class JobServiceContext implements ServiceConnection { } boolean isWithinExecutionGuaranteeTime() { - return mExecutionStartTimeElapsed + mMinExecutionGuaranteeMillis - < sElapsedRealtimeClock.millis(); + return sElapsedRealtimeClock.millis() + < mExecutionStartTimeElapsed + mMinExecutionGuaranteeMillis; } @GuardedBy("mLock") @@ -848,11 +848,12 @@ public final class JobServiceContext implements ServiceConnection { } applyStoppedReasonLocked(reason); completedJob = mRunningJob; - mJobPackageTracker.noteInactive(completedJob, mParams.getStopReason(), reason); + final int stopReason = mParams.getStopReason(); + mJobPackageTracker.noteInactive(completedJob, stopReason, reason); FrameworkStatsLog.write_non_chained(FrameworkStatsLog.SCHEDULED_JOB_STATE_CHANGED, completedJob.getSourceUid(), null, completedJob.getBatteryName(), FrameworkStatsLog.SCHEDULED_JOB_STATE_CHANGED__STATE__FINISHED, - mParams.getStopReason(), completedJob.getStandbyBucket(), completedJob.getJobId(), + stopReason, completedJob.getStandbyBucket(), completedJob.getJobId(), completedJob.hasChargingConstraint(), completedJob.hasBatteryNotLowConstraint(), completedJob.hasStorageNotLowConstraint(), @@ -863,7 +864,7 @@ public final class JobServiceContext implements ServiceConnection { completedJob.hasContentTriggerConstraint()); try { mBatteryStats.noteJobFinish(mRunningJob.getBatteryName(), mRunningJob.getSourceUid(), - mParams.getStopReason()); + stopReason); } catch (RemoteException e) { // Whatever. } @@ -882,7 +883,7 @@ public final class JobServiceContext implements ServiceConnection { service = null; mAvailable = true; removeOpTimeOutLocked(); - mCompletedListener.onJobCompletedLocked(completedJob, reschedule); + mCompletedListener.onJobCompletedLocked(completedJob, stopReason, reschedule); mJobConcurrencyManager.onJobCompletedLocked(this, completedJob, workType); } diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobStore.java b/apex/jobscheduler/service/java/com/android/server/job/JobStore.java index eaf8f4d963315..aa8d98c018537 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobStore.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobStore.java @@ -954,7 +954,7 @@ public final class JobStore { appBucket, sourceTag, elapsedRuntimes.first, elapsedRuntimes.second, lastSuccessfulRunTime, lastFailedRunTime, - (rtcIsGood) ? null : rtcRuntimes, internalFlags); + (rtcIsGood) ? null : rtcRuntimes, internalFlags, /* dynamicConstraints */ 0); return js; } diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/DeviceIdleJobsController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/DeviceIdleJobsController.java index 192f5e66255d9..79ef321eaf07d 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/DeviceIdleJobsController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/DeviceIdleJobsController.java @@ -64,7 +64,7 @@ public final class DeviceIdleJobsController extends StateController { * when the app is temp whitelisted or in the foreground. */ private final ArraySet mAllowInIdleJobs; - private final SparseBooleanArray mForegroundUids; + private final SparseBooleanArray mForegroundUids = new SparseBooleanArray(); private final DeviceIdleUpdateFunctor mDeviceIdleUpdateFunctor; private final DeviceIdleJobsDelayHandler mHandler; private final PowerManager mPowerManager; @@ -77,7 +77,6 @@ public final class DeviceIdleJobsController extends StateController { private int[] mDeviceIdleWhitelistAppIds; private int[] mPowerSaveTempWhitelistAppIds; - // onReceive private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { @@ -120,6 +119,10 @@ public final class DeviceIdleJobsController extends StateController { } }; + /** Criteria for whether or not we should a job's rush evaluation when the device exits Doze. */ + private final Predicate mShouldRushEvaluation = (jobStatus) -> + jobStatus.isRequestedExpeditedJob() || mForegroundUids.get(jobStatus.getSourceUid()); + public DeviceIdleJobsController(JobSchedulerService service) { super(service); @@ -133,7 +136,6 @@ public final class DeviceIdleJobsController extends StateController { mLocalDeviceIdleController.getPowerSaveTempWhitelistAppIds(); mDeviceIdleUpdateFunctor = new DeviceIdleUpdateFunctor(); mAllowInIdleJobs = new ArraySet<>(); - mForegroundUids = new SparseBooleanArray(); final IntentFilter filter = new IntentFilter(); filter.addAction(PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED); filter.addAction(PowerManager.ACTION_LIGHT_DEVICE_IDLE_MODE_CHANGED); @@ -156,14 +158,9 @@ public final class DeviceIdleJobsController extends StateController { mHandler.removeMessages(PROCESS_BACKGROUND_JOBS); mService.getJobStore().forEachJob(mDeviceIdleUpdateFunctor); } else { - // When coming out of doze, process all foreground uids immediately, while others - // will be processed after a delay of 3 seconds. - for (int i = 0; i < mForegroundUids.size(); i++) { - if (mForegroundUids.valueAt(i)) { - mService.getJobStore().forEachJobForSourceUid( - mForegroundUids.keyAt(i), mDeviceIdleUpdateFunctor); - } - } + // When coming out of doze, process all foreground uids and EJs immediately, + // while others will be processed after a delay of 3 seconds. + mService.getJobStore().forEachJob(mShouldRushEvaluation, mDeviceIdleUpdateFunctor); mHandler.sendEmptyMessageDelayed(PROCESS_BACKGROUND_JOBS, BACKGROUND_JOBS_DELAY); } } 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 5bdeb38a14245..bad8dc1ad1cbc 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 @@ -91,6 +91,12 @@ public final class JobStatus { static final int CONSTRAINT_WITHIN_EXPEDITED_QUOTA = 1 << 23; // Implicit constraint static final int CONSTRAINT_BACKGROUND_NOT_RESTRICTED = 1 << 22; // Implicit constraint + // The following set of dynamic constraints are for specific use cases (as explained in their + // relative naming and comments). Right now, they apply different constraints, which is fine, + // but if in the future, we have overlapping dynamic constraint sets, removing one constraint + // set may accidentally remove a constraint applied by another dynamic set. + // TODO: properly handle overlapping dynamic constraint sets + /** * The additional set of dynamic constraints that must be met if the job's effective bucket is * {@link JobSchedulerService#RESTRICTED_INDEX}. Connectivity can be ignored if the job doesn't @@ -102,6 +108,13 @@ public final class JobStatus { | CONSTRAINT_CONNECTIVITY | CONSTRAINT_IDLE; + /** + * The additional set of dynamic constraints that must be met if this is an expedited job that + * had a long enough run while the device was Dozing or in battery saver. + */ + private static final int DYNAMIC_EXPEDITED_DEFERRAL_CONSTRAINTS = + CONSTRAINT_DEVICE_NOT_DOZING | CONSTRAINT_BACKGROUND_NOT_RESTRICTED; + /** * Standard media URIs that contain the media files that might be important to the user. * @see #mHasMediaBackupExemption @@ -426,7 +439,8 @@ public final class JobStatus { private JobStatus(JobInfo job, int callingUid, String sourcePackageName, int sourceUserId, int standbyBucket, String tag, int numFailures, long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis, - long lastSuccessfulRunTime, long lastFailedRunTime, int internalFlags) { + long lastSuccessfulRunTime, long lastFailedRunTime, int internalFlags, + int dynamicConstraints) { this.job = job; this.callingUid = callingUid; this.standbyBucket = standbyBucket; @@ -487,6 +501,7 @@ public final class JobStatus { } this.requiredConstraints = requiredConstraints; mRequiredConstraintsOfInterest = requiredConstraints & CONSTRAINTS_OF_INTEREST; + addDynamicConstraints(dynamicConstraints); mReadyNotDozing = canRunInDoze(); if (standbyBucket == RESTRICTED_INDEX) { addDynamicConstraints(DYNAMIC_RESTRICTED_CONSTRAINTS); @@ -521,7 +536,7 @@ public final class JobStatus { jobStatus.getSourceTag(), jobStatus.getNumFailures(), jobStatus.getEarliestRunTime(), jobStatus.getLatestRunTimeElapsed(), jobStatus.getLastSuccessfulRunTime(), jobStatus.getLastFailedRunTime(), - jobStatus.getInternalFlags()); + jobStatus.getInternalFlags(), jobStatus.mDynamicConstraints); mPersistedUtcTimes = jobStatus.mPersistedUtcTimes; if (jobStatus.mPersistedUtcTimes != null) { if (DEBUG) { @@ -543,12 +558,12 @@ public final class JobStatus { long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis, long lastSuccessfulRunTime, long lastFailedRunTime, Pair persistedExecutionTimesUTC, - int innerFlags) { + int innerFlags, int dynamicConstraints) { this(job, callingUid, sourcePkgName, sourceUserId, standbyBucket, sourceTag, 0, earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis, - lastSuccessfulRunTime, lastFailedRunTime, innerFlags); + lastSuccessfulRunTime, lastFailedRunTime, innerFlags, dynamicConstraints); // Only during initial inflation do we record the UTC-timebase execution bounds // read from the persistent store. If we ever have to recreate the JobStatus on @@ -572,7 +587,8 @@ public final class JobStatus { rescheduling.getStandbyBucket(), rescheduling.getSourceTag(), backoffAttempt, newEarliestRuntimeElapsedMillis, newLatestRuntimeElapsedMillis, - lastSuccessfulRunTime, lastFailedRunTime, rescheduling.getInternalFlags()); + lastSuccessfulRunTime, lastFailedRunTime, rescheduling.getInternalFlags(), + rescheduling.mDynamicConstraints); } /** @@ -609,7 +625,7 @@ public final class JobStatus { standbyBucket, tag, 0, earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis, 0 /* lastSuccessfulRunTime */, 0 /* lastFailedRunTime */, - /*innerFlags=*/ 0); + /*innerFlags=*/ 0, /* dynamicConstraints */ 0); } public void enqueueWorkLocked(JobWorkItem work) { @@ -1083,12 +1099,15 @@ public final class JobStatus { * in Doze. */ public boolean canRunInDoze() { - return (getFlags() & JobInfo.FLAG_WILL_BE_FOREGROUND) != 0 || shouldTreatAsExpeditedJob(); + return (getFlags() & JobInfo.FLAG_WILL_BE_FOREGROUND) != 0 + || (shouldTreatAsExpeditedJob() + && (mDynamicConstraints & CONSTRAINT_DEVICE_NOT_DOZING) == 0); } boolean canRunInBatterySaver() { return (getInternalFlags() & INTERNAL_FLAG_HAS_FOREGROUND_EXEMPTION) != 0 - || shouldTreatAsExpeditedJob(); + || (shouldTreatAsExpeditedJob() + && (mDynamicConstraints & CONSTRAINT_BACKGROUND_NOT_RESTRICTED) == 0); } boolean shouldIgnoreNetworkBlocking() { @@ -1244,6 +1263,14 @@ public final class JobStatus { trackingControllers |= which; } + /** + * Add additional constraints to prevent this job from running when doze or battery saver are + * active. + */ + public void disallowRunInBatterySaverAndDoze() { + addDynamicConstraints(DYNAMIC_EXPEDITED_DEFERRAL_CONSTRAINTS); + } + /** * Indicates that this job cannot run without the specified constraints. This is evaluated * separately from the job's explicitly requested constraints and MUST be satisfied before diff --git a/services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java index f7f592886473e..3870b02ba37c4 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java @@ -641,6 +641,6 @@ public class ConnectivityControllerTest { private static JobStatus createJobStatus(JobInfo.Builder job, int uid, long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis) { return new JobStatus(job.build(), uid, null, -1, 0, null, - earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis, 0, 0, null, 0); + earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis, 0, 0, null, 0, 0); } } diff --git a/services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java b/services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java index 91b3cb7dbdd9f..7925b69852ba5 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java @@ -685,7 +685,7 @@ public class JobStatusTest { final JobInfo job = new JobInfo.Builder(101, new ComponentName("foo", "bar")) .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY).build(); return new JobStatus(job, 0, null, -1, 0, null, earliestRunTimeElapsedMillis, - latestRunTimeElapsedMillis, 0, 0, null, 0); + latestRunTimeElapsedMillis, 0, 0, null, 0, 0); } private static JobStatus createJobStatus(JobInfo job) { diff --git a/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java b/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java index deaeb46c40742..8b35af80e47fd 100644 --- a/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java +++ b/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java @@ -276,7 +276,7 @@ public class JobStoreTest { 0 /* sourceUserId */, 0, "someTag", invalidEarlyRuntimeElapsedMillis, invalidLateRuntimeElapsedMillis, 0 /* lastSuccessfulRunTime */, 0 /* lastFailedRunTime */, - persistedExecutionTimesUTC, 0 /* innerFlagg */); + persistedExecutionTimesUTC, 0 /* innerFlag */, 0 /* dynamicConstraints */); mTaskStoreUnderTest.add(js); waitForPendingIo();