From 966e72b82216ce866d1d4203c3e121a656ceee87 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Thu, 30 May 2019 14:45:39 -0700 Subject: [PATCH] Adding checks for bad periodic job scheduling. 1. There are cases where a periodic job is scheduled in a past time period. Without any other constraints, the job could be run much too frequently and spam the system. Adding these checks makes sure the jobs are scheduled with sane values. 2. Adding an upper limit on the job period to avoid overflow cases. 3. Changing int to long to avoid overflow. Bug: 133654009 Test: atest com.android.server.job.JobSchedulerServiceTest Test: atest com.android.server.job.controllers.QuotaControllerTest Test: atest com.android.server.job.controllers.TimeControllerTest Test: atest CtsJobSchedulerTestCases Change-Id: I97616efb6c37b7201eb926d16c0d9be450bf2ba4 --- .../server/job/JobSchedulerService.java | 31 +++++++++-- .../server/job/controllers/JobStatus.java | 12 ++++- .../server/job/JobSchedulerServiceTest.java | 51 +++++++++++++++++++ 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/job/JobSchedulerService.java b/services/core/java/com/android/server/job/JobSchedulerService.java index 26307cbf79089..ab4ae9d521bd1 100644 --- a/services/core/java/com/android/server/job/JobSchedulerService.java +++ b/services/core/java/com/android/server/job/JobSchedulerService.java @@ -1635,6 +1635,9 @@ public class JobSchedulerService extends com.android.server.SystemService */ private static final long PERIODIC_JOB_WINDOW_BUFFER = 30 * MINUTE_IN_MILLIS; + /** The maximum period a periodic job can have. Anything higher will be clamped down to this. */ + public static final long MAX_ALLOWED_PERIOD_MS = 365 * 24 * 60 * 60 * 1000L; + /** * Called after a periodic has executed so we can reschedule it. We take the last execution * time of the job to be the time of completion (i.e. the time at which this function is @@ -1652,11 +1655,21 @@ public class JobSchedulerService extends com.android.server.SystemService JobStatus getRescheduleJobForPeriodic(JobStatus periodicToReschedule) { final long elapsedNow = sElapsedRealtimeClock.millis(); final long newLatestRuntimeElapsed; - final long period = periodicToReschedule.getJob().getIntervalMillis(); - final long latestRunTimeElapsed = periodicToReschedule.getOriginalLatestRunTimeElapsed(); - final long flex = periodicToReschedule.getJob().getFlexMillis(); + // Make sure period is in the interval [min_possible_period, max_possible_period]. + final long period = Math.max(JobInfo.getMinPeriodMillis(), + Math.min(MAX_ALLOWED_PERIOD_MS, periodicToReschedule.getJob().getIntervalMillis())); + // Make sure flex is in the interval [min_possible_flex, period]. + final long flex = Math.max(JobInfo.getMinFlexMillis(), + Math.min(period, periodicToReschedule.getJob().getFlexMillis())); long rescheduleBuffer = 0; + long olrte = periodicToReschedule.getOriginalLatestRunTimeElapsed(); + if (olrte < 0 || olrte == JobStatus.NO_LATEST_RUNTIME) { + Slog.wtf(TAG, "Invalid periodic job original latest run time: " + olrte); + olrte = elapsedNow; + } + final long latestRunTimeElapsed = olrte; + final long diffMs = Math.abs(elapsedNow - latestRunTimeElapsed); if (elapsedNow > latestRunTimeElapsed) { // The job ran past its expected run window. Have it count towards the current window @@ -1664,7 +1677,7 @@ public class JobSchedulerService extends com.android.server.SystemService if (DEBUG) { Slog.i(TAG, "Periodic job ran after its intended window."); } - int numSkippedWindows = (int) (diffMs / period) + 1; // +1 to include original window + long numSkippedWindows = (diffMs / period) + 1; // +1 to include original window if (period != flex && diffMs > Math.min(PERIODIC_JOB_WINDOW_BUFFER, (period - flex) / 2)) { if (DEBUG) { @@ -1684,6 +1697,16 @@ public class JobSchedulerService extends com.android.server.SystemService } } + if (newLatestRuntimeElapsed < elapsedNow) { + Slog.wtf(TAG, "Rescheduling calculated latest runtime in the past: " + + newLatestRuntimeElapsed); + return new JobStatus(periodicToReschedule, getCurrentHeartbeat(), + elapsedNow + period - flex, elapsedNow + period, + 0 /* backoffAttempt */, + sSystemClock.millis() /* lastSuccessfulRunTime */, + periodicToReschedule.getLastFailedRunTime()); + } + final long newEarliestRunTimeElapsed = newLatestRuntimeElapsed - Math.min(flex, period - rescheduleBuffer); diff --git a/services/core/java/com/android/server/job/controllers/JobStatus.java b/services/core/java/com/android/server/job/controllers/JobStatus.java index fd20e116d82aa..eb5d472f7fbc3 100644 --- a/services/core/java/com/android/server/job/controllers/JobStatus.java +++ b/services/core/java/com/android/server/job/controllers/JobStatus.java @@ -511,8 +511,13 @@ public final class JobStatus { final long elapsedNow = sElapsedRealtimeClock.millis(); final long earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis; if (job.isPeriodic()) { - latestRunTimeElapsedMillis = elapsedNow + job.getIntervalMillis(); - earliestRunTimeElapsedMillis = latestRunTimeElapsedMillis - job.getFlexMillis(); + // Make sure period is in the interval [min_possible_period, max_possible_period]. + final long period = Math.max(JobInfo.getMinPeriodMillis(), + Math.min(JobSchedulerService.MAX_ALLOWED_PERIOD_MS, job.getIntervalMillis())); + latestRunTimeElapsedMillis = elapsedNow + period; + earliestRunTimeElapsedMillis = latestRunTimeElapsedMillis + // Make sure flex is in the interval [min_possible_flex, period]. + - Math.max(JobInfo.getMinFlexMillis(), Math.min(period, job.getFlexMillis())); } else { earliestRunTimeElapsedMillis = job.hasEarlyConstraint() ? elapsedNow + job.getMinLatencyMillis() : NO_EARLIEST_RUNTIME; @@ -1631,6 +1636,9 @@ public final class JobStatus { formatRunTime(pw, earliestRunTimeElapsedMillis, NO_EARLIEST_RUNTIME, elapsedRealtimeMillis); pw.print(", latest="); formatRunTime(pw, latestRunTimeElapsedMillis, NO_LATEST_RUNTIME, elapsedRealtimeMillis); + pw.print(", original latest="); + formatRunTime(pw, mOriginalLatestRunTimeElapsedMillis, + NO_LATEST_RUNTIME, elapsedRealtimeMillis); pw.println(); if (numFailures != 0) { pw.print(prefix); pw.print("Num failures: "); pw.println(numFailures); diff --git a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java index 18c524ad7a945..748c23a24f6d9 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java @@ -16,6 +16,7 @@ package com.android.server.job; +import static android.text.format.DateUtils.DAY_IN_MILLIS; import static android.text.format.DateUtils.HOUR_IN_MILLIS; import static android.text.format.DateUtils.MINUTE_IN_MILLIS; @@ -160,6 +161,56 @@ public class JobSchedulerServiceTest { jobInfoBuilder.build(), 1234, "com.android.test", 0, testTag); } + /** + * Confirm that {@link JobSchedulerService#getRescheduleJobForPeriodic(JobStatus)} returns a job + * with the correct delay and deadline constraints if the periodic job is scheduled with the + * minimum possible period. + */ + @Test + public void testGetRescheduleJobForPeriodic_minPeriod() { + final long now = sElapsedRealtimeClock.millis(); + JobStatus job = createJobStatus("testGetRescheduleJobForPeriodic_insideWindow", + createJobInfo().setPeriodic(15 * MINUTE_IN_MILLIS)); + final long nextWindowStartTime = now + 15 * MINUTE_IN_MILLIS; + final long nextWindowEndTime = now + 30 * MINUTE_IN_MILLIS; + + for (int i = 0; i < 25; i++) { + JobStatus rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + advanceElapsedClock(30_000); // 30 seconds + } + + for (int i = 0; i < 5; i++) { + // Window buffering in last 1/6 of window. + JobStatus rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime + i * 30_000, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + advanceElapsedClock(30_000); // 30 seconds + } + } + + /** + * Confirm that {@link JobSchedulerService#getRescheduleJobForPeriodic(JobStatus)} returns a job + * with the correct delay and deadline constraints if the periodic job is scheduled with a + * period that's too large. + */ + @Test + public void testGetRescheduleJobForPeriodic_largePeriod() { + final long now = sElapsedRealtimeClock.millis(); + JobStatus job = createJobStatus("testGetRescheduleJobForPeriodic_insideWindow", + createJobInfo().setPeriodic(2 * 365 * DAY_IN_MILLIS)); + assertEquals(now, job.getEarliestRunTime()); + // Periods are capped at 365 days (1 year). + assertEquals(now + 365 * DAY_IN_MILLIS, job.getLatestRunTimeElapsed()); + final long nextWindowStartTime = now + 365 * DAY_IN_MILLIS; + final long nextWindowEndTime = nextWindowStartTime + 365 * DAY_IN_MILLIS; + + JobStatus rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + } + /** * Confirm that {@link JobSchedulerService#getRescheduleJobForPeriodic(JobStatus)} returns a job * with the correct delay and deadline constraints if the periodic job is completed and