Merge "Limit the number of persistable JobWorkItems." into udc-dev

This commit is contained in:
TreeHugger Robot
2023-03-15 22:34:07 +00:00
committed by Android (Google) Code Review
3 changed files with 129 additions and 6 deletions

View File

@@ -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");

View File

@@ -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();
}

View File

@@ -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() {