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 b15fa9ee4fa45..8375d47a61fe6 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -429,6 +429,7 @@ public class JobSchedulerService extends com.android.server.SystemService public void onPropertiesChanged(DeviceConfig.Properties properties) { boolean apiQuotaScheduleUpdated = false; boolean concurrencyUpdated = false; + boolean persistenceUpdated = false; boolean runtimeUpdated = false; for (int controller = 0; controller < mControllers.size(); controller++) { final StateController sc = mControllers.get(controller); @@ -488,9 +489,13 @@ public class JobSchedulerService extends com.android.server.SystemService runtimeUpdated = true; } break; + case Constants.KEY_MAX_NUM_PERSISTED_JOB_WORK_ITEMS: case Constants.KEY_PERSIST_IN_SPLIT_FILES: - mConstants.updatePersistingConstantsLocked(); - mJobs.setUseSplitFiles(mConstants.PERSIST_IN_SPLIT_FILES); + if (!persistenceUpdated) { + mConstants.updatePersistingConstantsLocked(); + mJobs.setUseSplitFiles(mConstants.PERSIST_IN_SPLIT_FILES); + persistenceUpdated = true; + } break; default: if (name.startsWith(JobConcurrencyManager.CONFIG_KEY_PREFIX_CONCURRENCY) @@ -583,6 +588,9 @@ public class JobSchedulerService extends com.android.server.SystemService private static final String KEY_PERSIST_IN_SPLIT_FILES = "persist_in_split_files"; + private static final String KEY_MAX_NUM_PERSISTED_JOB_WORK_ITEMS = + "max_num_persisted_job_work_items"; + private static final int DEFAULT_MIN_READY_NON_ACTIVE_JOBS_COUNT = 5; private static final long DEFAULT_MAX_NON_ACTIVE_JOB_BATCH_DELAY_MS = 31 * MINUTE_IN_MILLIS; private static final float DEFAULT_HEAVY_USE_FACTOR = .9f; @@ -618,6 +626,7 @@ public class JobSchedulerService extends com.android.server.SystemService public static final long DEFAULT_RUNTIME_UI_DATA_TRANSFER_LIMIT_MS = Math.min(Long.MAX_VALUE, DEFAULT_RUNTIME_UI_LIMIT_MS); static final boolean DEFAULT_PERSIST_IN_SPLIT_FILES = true; + static final int DEFAULT_MAX_NUM_PERSISTED_JOB_WORK_ITEMS = 100_000; /** * Minimum # of non-ACTIVE jobs for which the JMS will be happy running some work early. @@ -760,6 +769,11 @@ public class JobSchedulerService extends com.android.server.SystemService */ public boolean PERSIST_IN_SPLIT_FILES = DEFAULT_PERSIST_IN_SPLIT_FILES; + /** + * The maximum number of {@link JobWorkItem JobWorkItems} that can be persisted per job. + */ + public int MAX_NUM_PERSISTED_JOB_WORK_ITEMS = DEFAULT_MAX_NUM_PERSISTED_JOB_WORK_ITEMS; + /** * If true, use TARE policy for job limiting. If false, use quotas. */ @@ -822,6 +836,10 @@ public class JobSchedulerService extends com.android.server.SystemService private void updatePersistingConstantsLocked() { PERSIST_IN_SPLIT_FILES = DeviceConfig.getBoolean(DeviceConfig.NAMESPACE_JOB_SCHEDULER, KEY_PERSIST_IN_SPLIT_FILES, DEFAULT_PERSIST_IN_SPLIT_FILES); + MAX_NUM_PERSISTED_JOB_WORK_ITEMS = DeviceConfig.getInt( + DeviceConfig.NAMESPACE_JOB_SCHEDULER, + KEY_MAX_NUM_PERSISTED_JOB_WORK_ITEMS, + DEFAULT_MAX_NUM_PERSISTED_JOB_WORK_ITEMS); } private void updatePrefetchConstantsLocked() { @@ -961,6 +979,8 @@ public class JobSchedulerService extends com.android.server.SystemService RUNTIME_UI_DATA_TRANSFER_LIMIT_MS).println(); pw.print(KEY_PERSIST_IN_SPLIT_FILES, PERSIST_IN_SPLIT_FILES).println(); + pw.print(KEY_MAX_NUM_PERSISTED_JOB_WORK_ITEMS, MAX_NUM_PERSISTED_JOB_WORK_ITEMS) + .println(); pw.print(Settings.Global.ENABLE_TARE, USE_TARE_POLICY).println(); @@ -1344,6 +1364,25 @@ public class JobSchedulerService extends com.android.server.SystemService // Fast path: we are adding work to an existing job, and the JobInfo is not // changing. We can just directly enqueue this work in to the job. if (toCancel.getJob().equals(job)) { + // On T and below, JobWorkItem count was unlimited but they could not be + // persisted. Now in U and above, we allow persisting them. In both cases, + // there is a danger of apps adding too many JobWorkItems and causing the + // system to OOM since we keep everything in memory. The persisting danger + // is greater because it could technically lead to a boot loop if the system + // keeps trying to load all the JobWorkItems that led to the initial OOM. + // Therefore, for now (partly for app compatibility), we tackle the latter + // and limit the number of JobWorkItems that can be persisted. + // Moving forward, we should look into two things: + // 1. Limiting the number of unpersisted JobWorkItems + // 2. Offloading some state to disk so we don't keep everything in memory + // TODO(273758274): improve JobScheduler's resilience and memory management + if (toCancel.getWorkCount() >= mConstants.MAX_NUM_PERSISTED_JOB_WORK_ITEMS + && toCancel.isPersisted()) { + Slog.w(TAG, "Too many JWIs for uid " + uId); + throw new IllegalStateException("Apps may not persist more than " + + mConstants.MAX_NUM_PERSISTED_JOB_WORK_ITEMS + + " JobWorkItems per job"); + } toCancel.enqueueWorkLocked(work); mJobs.touchJob(toCancel); @@ -1388,6 +1427,26 @@ public class JobSchedulerService extends com.android.server.SystemService jobStatus.prepareLocked(); if (toCancel != null) { + // On T and below, JobWorkItem count was unlimited but they could not be + // persisted. Now in U and above, we allow persisting them. In both cases, + // there is a danger of apps adding too many JobWorkItems and causing the + // system to OOM since we keep everything in memory. The persisting danger + // is greater because it could technically lead to a boot loop if the system + // keeps trying to load all the JobWorkItems that led to the initial OOM. + // Therefore, for now (partly for app compatibility), we tackle the latter + // and limit the number of JobWorkItems that can be persisted. + // Moving forward, we should look into two things: + // 1. Limiting the number of unpersisted JobWorkItems + // 2. Offloading some state to disk so we don't keep everything in memory + // TODO(273758274): improve JobScheduler's resilience and memory management + if (work != null && toCancel.isPersisted() + && toCancel.getWorkCount() >= mConstants.MAX_NUM_PERSISTED_JOB_WORK_ITEMS) { + Slog.w(TAG, "Too many JWIs for uid " + uId); + throw new IllegalStateException("Apps may not persist more than " + + mConstants.MAX_NUM_PERSISTED_JOB_WORK_ITEMS + + " JobWorkItems per job"); + } + // Implicitly replaces the existing job record with the new instance cancelJobImplLocked(toCancel, jobStatus, JobParameters.STOP_REASON_CANCELLED_BY_APP, JobParameters.INTERNAL_STOP_REASON_CANCELED, "job rescheduled by app"); 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 537a67039a826..0cc775870f880 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 @@ -814,6 +814,13 @@ public final class JobStatus { return null; } + /** Returns the number of {@link JobWorkItem JobWorkItems} attached to this job. */ + public int getWorkCount() { + final int pendingCount = pendingWork == null ? 0 : pendingWork.size(); + final int executingCount = executingWork == null ? 0 : executingWork.size(); + return pendingCount + executingCount; + } + public boolean hasWorkLocked() { return (pendingWork != null && pendingWork.size() > 0) || hasExecutingWorkLocked(); } 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 7b55edb31df34..e056417811c70 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java @@ -50,6 +50,7 @@ import android.app.UiModeManager; import android.app.job.JobInfo; import android.app.job.JobParameters; import android.app.job.JobScheduler; +import android.app.job.JobWorkItem; import android.app.usage.UsageStatsManagerInternal; import android.content.ComponentName; import android.content.Context; @@ -91,6 +92,7 @@ import java.time.ZoneOffset; public class JobSchedulerServiceTest { private static final String TAG = JobSchedulerServiceTest.class.getSimpleName(); + private static final int TEST_UID = 10123; private JobSchedulerService mService; @@ -177,6 +179,9 @@ public class JobSchedulerServiceTest { if (mMockingSession != null) { mMockingSession.finishMocking(); } + mService.cancelJobsForUid(TEST_UID, true, + JobParameters.STOP_REASON_UNDEFINED, JobParameters.INTERNAL_STOP_REASON_UNKNOWN, + "test cleanup"); } private Clock getAdvancedClock(Clock clock, long incrementMs) { @@ -1170,7 +1175,7 @@ public class JobSchedulerServiceTest { i < 300 ? JobScheduler.RESULT_SUCCESS : JobScheduler.RESULT_FAILURE; assertEquals("Got unexpected result for schedule #" + (i + 1), expected, - mService.scheduleAsPackage(job, null, 10123, null, 0, "JSSTest", "")); + mService.scheduleAsPackage(job, null, TEST_UID, null, 0, "JSSTest", "")); } } @@ -1191,7 +1196,7 @@ public class JobSchedulerServiceTest { for (int i = 0; i < 500; ++i) { assertEquals("Got unexpected result for schedule #" + (i + 1), JobScheduler.RESULT_SUCCESS, - mService.scheduleAsPackage(job, null, 10123, null, 0, "JSSTest", "")); + mService.scheduleAsPackage(job, null, TEST_UID, null, 0, "JSSTest", "")); } } @@ -1212,7 +1217,7 @@ public class JobSchedulerServiceTest { for (int i = 0; i < 500; ++i) { assertEquals("Got unexpected result for schedule #" + (i + 1), JobScheduler.RESULT_SUCCESS, - mService.scheduleAsPackage(job, null, 10123, "proxied.package", 0, "JSSTest", + mService.scheduleAsPackage(job, null, TEST_UID, "proxied.package", 0, "JSSTest", "")); } } @@ -1236,11 +1241,63 @@ public class JobSchedulerServiceTest { i < 300 ? JobScheduler.RESULT_SUCCESS : JobScheduler.RESULT_FAILURE; assertEquals("Got unexpected result for schedule #" + (i + 1), expected, - mService.scheduleAsPackage(job, null, 10123, job.getService().getPackageName(), + mService.scheduleAsPackage(job, null, TEST_UID, + job.getService().getPackageName(), 0, "JSSTest", "")); } } + /** + * Tests that the number of persisted JobWorkItems is capped. + */ + @Test + public void testScheduleLimiting_JobWorkItems_Nonpersisted() { + mService.mConstants.MAX_NUM_PERSISTED_JOB_WORK_ITEMS = 500; + mService.mConstants.ENABLE_API_QUOTAS = false; + mService.mConstants.API_QUOTA_SCHEDULE_THROW_EXCEPTION = false; + mService.mConstants.API_QUOTA_SCHEDULE_RETURN_FAILURE_RESULT = false; + mService.updateQuotaTracker(); + + final JobInfo job = createJobInfo().setPersisted(false).build(); + final JobWorkItem item = new JobWorkItem.Builder().build(); + for (int i = 0; i < 1000; ++i) { + assertEquals("Got unexpected result for schedule #" + (i + 1), + JobScheduler.RESULT_SUCCESS, + mService.scheduleAsPackage(job, item, TEST_UID, + job.getService().getPackageName(), + 0, "JSSTest", "")); + } + } + + /** + * Tests that the number of persisted JobWorkItems is capped. + */ + @Test + public void testScheduleLimiting_JobWorkItems_Persisted() { + mService.mConstants.MAX_NUM_PERSISTED_JOB_WORK_ITEMS = 500; + mService.mConstants.ENABLE_API_QUOTAS = false; + mService.mConstants.API_QUOTA_SCHEDULE_THROW_EXCEPTION = false; + mService.mConstants.API_QUOTA_SCHEDULE_RETURN_FAILURE_RESULT = false; + mService.updateQuotaTracker(); + + final JobInfo job = createJobInfo().setPersisted(true).build(); + final JobWorkItem item = new JobWorkItem.Builder().build(); + for (int i = 0; i < 500; ++i) { + assertEquals("Got unexpected result for schedule #" + (i + 1), + JobScheduler.RESULT_SUCCESS, + mService.scheduleAsPackage(job, item, TEST_UID, + job.getService().getPackageName(), + 0, "JSSTest", "")); + } + try { + mService.scheduleAsPackage(job, item, TEST_UID, job.getService().getPackageName(), + 0, "JSSTest", ""); + fail("Added more items than allowed"); + } catch (IllegalStateException expected) { + // Success + } + } + /** Tests that jobs are removed from the pending list if the user stops the app. */ @Test public void testUserStopRemovesPending() {