From a90582494645ba70fe9f538dd9fa74117a367f11 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Thu, 29 Sep 2022 19:16:57 +0000 Subject: [PATCH] Cancel all jobs associated with a UID when appropriate. Make sure we cancel all jobs scheduled by and for a UID when an ephemeral, or otherwise restricted app, goes to the background or when the user clears an app's data. Also fix bug in FlexibilityController that looked at the scheduling app's jobs instead of the source app's jobs. Bug: 247808104 Test: atest CtsJobSchedulerTestCase Change-Id: Iba2c371e0da642c5e59070fe25dd89c09db3b974 --- .../server/job/JobSchedulerInternal.java | 6 ++- .../server/job/JobSchedulerService.java | 50 ++++++++++++------- .../java/com/android/server/job/JobStore.java | 20 ++++---- .../controllers/FlexibilityController.java | 5 +- .../server/am/ActivityManagerService.java | 3 +- 5 files changed, 51 insertions(+), 33 deletions(-) diff --git a/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java b/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java index 2682dd75ca738..442c13009d8bb 100644 --- a/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java +++ b/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java @@ -36,9 +36,11 @@ public interface JobSchedulerInternal { /** * Cancel the jobs for a given uid (e.g. when app data is cleared) + * + * @param includeProxiedJobs Include jobs scheduled for this UID by other apps */ - void cancelJobsForUid(int uid, @JobParameters.StopReason int reason, int debugReasonCode, - String debugReason); + void cancelJobsForUid(int uid, boolean includeProxiedJobs, + @JobParameters.StopReason int reason, int debugReasonCode, String debugReason); /** * These are for activity manager to communicate to use what is currently performing backups. 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 1775d908e21b6..597365a8fc6a5 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -967,12 +967,12 @@ public class JobSchedulerService extends com.android.server.SystemService // Has this package scheduled any jobs, such that we will take action // if it were to be force-stopped? if (pkgUid != -1) { - List jobsForUid; + ArraySet jobsForUid; synchronized (mLock) { jobsForUid = mJobs.getJobsByUid(pkgUid); } for (int i = jobsForUid.size() - 1; i >= 0; i--) { - if (jobsForUid.get(i).getSourcePackageName().equals(pkgName)) { + if (jobsForUid.valueAt(i).getSourcePackageName().equals(pkgName)) { if (DEBUG) { Slog.d(TAG, "Restart query: package " + pkgName + " at uid " + pkgUid + " has jobs"); @@ -1289,10 +1289,11 @@ public class JobSchedulerService extends com.android.server.SystemService public List getPendingJobs(int uid) { synchronized (mLock) { - List jobs = mJobs.getJobsByUid(uid); + ArraySet jobs = mJobs.getJobsByUid(uid); ArrayList outList = new ArrayList(jobs.size()); + // Write out for loop to avoid addAll() creating an Iterator. for (int i = jobs.size() - 1; i >= 0; i--) { - JobStatus job = jobs.get(i); + final JobStatus job = jobs.valueAt(i); outList.add(job.getJob()); } return outList; @@ -1301,9 +1302,9 @@ public class JobSchedulerService extends com.android.server.SystemService public JobInfo getPendingJob(int uid, int jobId) { synchronized (mLock) { - List jobs = mJobs.getJobsByUid(uid); + ArraySet jobs = mJobs.getJobsByUid(uid); for (int i = jobs.size() - 1; i >= 0; i--) { - JobStatus job = jobs.get(i); + JobStatus job = jobs.valueAt(i); if (job.getJobId() == jobId) { return job.getJob(); } @@ -1345,7 +1346,7 @@ public class JobSchedulerService extends com.android.server.SystemService Slog.wtfStack(TAG, "Can't cancel all jobs for system package"); return; } - final List jobsForUid = new ArrayList<>(); + final ArraySet jobsForUid = new ArraySet<>(); if (includeSchedulingApp) { mJobs.getJobsByUid(uid, jobsForUid); } @@ -1353,7 +1354,7 @@ public class JobSchedulerService extends com.android.server.SystemService mJobs.getJobsBySourceUid(uid, jobsForUid); } for (int i = jobsForUid.size() - 1; i >= 0; i--) { - final JobStatus job = jobsForUid.get(i); + final JobStatus job = jobsForUid.valueAt(i); final boolean shouldCancel = (includeSchedulingApp && job.getServiceComponent().getPackageName().equals(pkgName)) @@ -1365,14 +1366,16 @@ public class JobSchedulerService extends com.android.server.SystemService } /** - * Entry point from client to cancel all jobs originating from their uid. + * Entry point from client to cancel all jobs scheduled for or from their uid. * This will remove the job from the master list, and cancel the job if it was staged for * execution or being executed. * * @param uid Uid to check against for removal of a job. + * @param includeSourceApp Whether to include jobs scheduled for this UID by another UID. + * If false, only jobs scheduled by this UID will be cancelled. */ - public boolean cancelJobsForUid(int uid, @JobParameters.StopReason int reason, - int internalReasonCode, String debugReason) { + public boolean cancelJobsForUid(int uid, boolean includeSourceApp, + @JobParameters.StopReason int reason, int internalReasonCode, String debugReason) { if (uid == Process.SYSTEM_UID) { Slog.wtfStack(TAG, "Can't cancel all jobs for system uid"); return false; @@ -1380,9 +1383,15 @@ public class JobSchedulerService extends com.android.server.SystemService boolean jobsCanceled = false; synchronized (mLock) { - final List jobsForUid = mJobs.getJobsByUid(uid); + final ArraySet jobsForUid = new ArraySet<>(); + // Get jobs scheduled by the app. + mJobs.getJobsByUid(uid, jobsForUid); + if (includeSourceApp) { + // Get jobs scheduled for the app by someone else. + mJobs.getJobsBySourceUid(uid, jobsForUid); + } for (int i = 0; i < jobsForUid.size(); i++) { - JobStatus toRemove = jobsForUid.get(i); + JobStatus toRemove = jobsForUid.valueAt(i); cancelJobImplLocked(toRemove, null, reason, internalReasonCode, debugReason); jobsCanceled = true; } @@ -2217,6 +2226,7 @@ public class JobSchedulerService extends com.android.server.SystemService updateUidState(uid, ActivityManager.PROCESS_STATE_CACHED_EMPTY); if (disabled) { cancelJobsForUid(uid, + /* includeSourceApp */ true, JobParameters.STOP_REASON_BACKGROUND_RESTRICTION, JobParameters.INTERNAL_STOP_REASON_CONSTRAINTS_NOT_SATISFIED, "uid gone"); @@ -2238,6 +2248,7 @@ public class JobSchedulerService extends com.android.server.SystemService final boolean disabled = message.arg2 != 0; if (disabled) { cancelJobsForUid(uid, + /* includeSourceApp */ true, JobParameters.STOP_REASON_BACKGROUND_RESTRICTION, JobParameters.INTERNAL_STOP_REASON_CONSTRAINTS_NOT_SATISFIED, "app uid idle"); @@ -2896,9 +2907,10 @@ public class JobSchedulerService extends com.android.server.SystemService } @Override - public void cancelJobsForUid(int uid, @JobParameters.StopReason int reason, - int internalReasonCode, String debugReason) { - JobSchedulerService.this.cancelJobsForUid(uid, reason, internalReasonCode, debugReason); + public void cancelJobsForUid(int uid, boolean includeProxiedJobs, + @JobParameters.StopReason int reason, int internalReasonCode, String debugReason) { + JobSchedulerService.this.cancelJobsForUid(uid, + includeProxiedJobs, reason, internalReasonCode, debugReason); } @Override @@ -3270,6 +3282,8 @@ public class JobSchedulerService extends com.android.server.SystemService final long ident = Binder.clearCallingIdentity(); try { JobSchedulerService.this.cancelJobsForUid(uid, + // Documentation says only jobs scheduled BY the app will be cancelled + /* includeSourceApp */ false, JobParameters.STOP_REASON_CANCELLED_BY_APP, JobParameters.INTERNAL_STOP_REASON_CANCELED, "cancelAll() called by app, callingUid=" + uid); @@ -3481,7 +3495,9 @@ public class JobSchedulerService extends com.android.server.SystemService if (!hasJobId) { pw.println("Canceling all jobs for " + pkgName + " in user " + userId); - if (!cancelJobsForUid(pkgUid, JobParameters.STOP_REASON_USER, + if (!cancelJobsForUid(pkgUid, + /* includeSourceApp */ false, + JobParameters.STOP_REASON_USER, JobParameters.INTERNAL_STOP_REASON_CANCELED, "cancel shell command for package")) { pw.println("No matching jobs found."); 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 78ab06c9e3328..9f69530ffb45e 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobStore.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobStore.java @@ -291,11 +291,11 @@ public final class JobStore { * @return A list of all the jobs scheduled for the source app. Never null. */ @NonNull - public List getJobsBySourceUid(int sourceUid) { + public ArraySet getJobsBySourceUid(int sourceUid) { return mJobSet.getJobsBySourceUid(sourceUid); } - public void getJobsBySourceUid(int sourceUid, @NonNull List insertInto) { + public void getJobsBySourceUid(int sourceUid, @NonNull Set insertInto) { mJobSet.getJobsBySourceUid(sourceUid, insertInto); } @@ -304,11 +304,11 @@ public final class JobStore { * @return All JobStatus objects for a given uid from the master list. Never null. */ @NonNull - public List getJobsByUid(int uid) { + public ArraySet getJobsByUid(int uid) { return mJobSet.getJobsByUid(uid); } - public void getJobsByUid(int uid, @NonNull List insertInto) { + public void getJobsByUid(int uid, @NonNull Set insertInto) { mJobSet.getJobsByUid(uid, insertInto); } @@ -1229,13 +1229,13 @@ public final class JobStore { mJobsPerSourceUid = new SparseArray<>(); } - public List getJobsByUid(int uid) { - ArrayList matchingJobs = new ArrayList(); + public ArraySet getJobsByUid(int uid) { + ArraySet matchingJobs = new ArraySet<>(); getJobsByUid(uid, matchingJobs); return matchingJobs; } - public void getJobsByUid(int uid, List insertInto) { + public void getJobsByUid(int uid, Set insertInto) { ArraySet jobs = mJobs.get(uid); if (jobs != null) { insertInto.addAll(jobs); @@ -1243,13 +1243,13 @@ public final class JobStore { } @NonNull - public List getJobsBySourceUid(int sourceUid) { - final ArrayList result = new ArrayList(); + public ArraySet getJobsBySourceUid(int sourceUid) { + final ArraySet result = new ArraySet<>(); getJobsBySourceUid(sourceUid, result); return result; } - public void getJobsBySourceUid(int sourceUid, List insertInto) { + public void getJobsBySourceUid(int sourceUid, Set insertInto) { final ArraySet jobs = mJobsPerSourceUid.get(sourceUid); if (jobs != null) { insertInto.addAll(jobs); diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/FlexibilityController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/FlexibilityController.java index 9c167728cff94..bbd661acca011 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/FlexibilityController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/FlexibilityController.java @@ -50,7 +50,6 @@ import com.android.server.utils.AlarmQueue; import java.util.ArrayList; import java.util.Arrays; -import java.util.List; import java.util.function.Predicate; /** @@ -397,10 +396,10 @@ public final class FlexibilityController extends StateController { return; } final long nowElapsed = sElapsedRealtimeClock.millis(); - List jobsByUid = mService.getJobStore().getJobsByUid(uid); + ArraySet jobsByUid = mService.getJobStore().getJobsBySourceUid(uid); boolean hasPrefetch = false; for (int i = 0; i < jobsByUid.size(); i++) { - JobStatus js = jobsByUid.get(i); + JobStatus js = jobsByUid.valueAt(i); if (js.hasFlexibilityConstraint()) { js.setFlexibilityConstraintSatisfied(nowElapsed, isFlexibilitySatisfiedLocked(js)); hasPrefetch |= js.getJob().isPrefetch(); diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 7d85c1333b1b3..529454884a99f 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -3933,7 +3933,8 @@ public class ActivityManagerService extends IActivityManager.Stub // Clear its scheduled jobs JobSchedulerInternal js = LocalServices.getService(JobSchedulerInternal.class); // Clearing data is a user-initiated action. - js.cancelJobsForUid(appInfo.uid, JobParameters.STOP_REASON_USER, + js.cancelJobsForUid(appInfo.uid, /* includeProxiedJobs */ true, + JobParameters.STOP_REASON_USER, JobParameters.INTERNAL_STOP_REASON_DATA_CLEARED, "clear data"); // Clear its pending alarms