am 07b554fc: Merge "Fix lock ordering in JobScheduler" into lmp-dev

* commit '07b554fcce08611872ea053ea9c97fa2671fc6db':
  Fix lock ordering in JobScheduler
This commit is contained in:
Matthew Williams
2014-09-24 01:12:47 +00:00
committed by Android Git Automerger
3 changed files with 117 additions and 115 deletions

View File

@@ -195,12 +195,13 @@ public class JobSchedulerService extends com.android.server.SystemService
}
private void cancelJobsForUser(int userHandle) {
List<JobStatus> jobsForUser;
synchronized (mJobs) {
List<JobStatus> jobsForUser = mJobs.getJobsByUser(userHandle);
for (int i=0; i<jobsForUser.size(); i++) {
JobStatus toRemove = jobsForUser.get(i);
cancelJobLocked(toRemove);
}
jobsForUser = mJobs.getJobsByUser(userHandle);
}
for (int i=0; i<jobsForUser.size(); i++) {
JobStatus toRemove = jobsForUser.get(i);
cancelJobImpl(toRemove);
}
}
@@ -208,16 +209,16 @@ public class JobSchedulerService extends com.android.server.SystemService
* Entry point from client to cancel all jobs originating 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 To check against for removal of a job.
* @param uid Uid to check against for removal of a job.
*/
public void cancelJobsForUid(int uid) {
// Remove from master list.
List<JobStatus> jobsForUid;
synchronized (mJobs) {
List<JobStatus> jobsForUid = mJobs.getJobsByUid(uid);
for (int i=0; i<jobsForUid.size(); i++) {
JobStatus toRemove = jobsForUid.get(i);
cancelJobLocked(toRemove);
}
jobsForUid = mJobs.getJobsByUid(uid);
}
for (int i=0; i<jobsForUid.size(); i++) {
JobStatus toRemove = jobsForUid.get(i);
cancelJobImpl(toRemove);
}
}
@@ -232,22 +233,23 @@ public class JobSchedulerService extends com.android.server.SystemService
JobStatus toCancel;
synchronized (mJobs) {
toCancel = mJobs.getJobByUidAndJobId(uid, jobId);
if (toCancel != null) {
cancelJobLocked(toCancel);
}
}
if (toCancel != null) {
cancelJobImpl(toCancel);
}
}
private void cancelJobLocked(JobStatus cancelled) {
private void cancelJobImpl(JobStatus cancelled) {
if (DEBUG) {
Slog.d(TAG, "Cancelling: " + cancelled);
}
// Remove from store.
stopTrackingJob(cancelled);
// Remove from pending queue.
mPendingJobs.remove(cancelled);
// Cancel if running.
stopJobOnServiceContextLocked(cancelled);
synchronized (mJobs) {
// Remove from pending queue.
mPendingJobs.remove(cancelled);
// Cancel if running.
stopJobOnServiceContextLocked(cancelled);
}
}
/**
@@ -482,21 +484,13 @@ public class JobSchedulerService extends com.android.server.SystemService
// StateChangedListener implementations.
/**
* Off-board work to our handler thread as quickly as possible, b/c this call is probably being
* made on the main thread.
* For now this takes the job and if it's ready to run it will run it. In future we might not
* provide the job, so that the StateChangedListener has to run through its list of jobs to
* see which are ready. This will further decouple the controllers from the execution logic.
* Posts a message to the {@link com.android.server.job.JobSchedulerService.JobHandler} that
* some controller's state has changed, so as to run through the list of jobs and start/stop
* any that are eligible.
*/
@Override
public void onControllerStateChanged() {
synchronized (mJobs) {
if (mReadyToRock) {
// Post a message to to run through the list of jobs and start/stop any that
// are eligible.
mHandler.obtainMessage(MSG_CHECK_JOB).sendToTarget();
}
}
mHandler.obtainMessage(MSG_CHECK_JOB).sendToTarget();
}
@Override
@@ -512,21 +506,29 @@ public class JobSchedulerService extends com.android.server.SystemService
@Override
public void handleMessage(Message message) {
synchronized (mJobs) {
if (!mReadyToRock) {
return;
}
}
switch (message.what) {
case MSG_JOB_EXPIRED:
synchronized (mJobs) {
JobStatus runNow = (JobStatus) message.obj;
// runNow can be null, which is a controller's way of indicating that its
// state is such that all ready jobs should be run immediately.
if (runNow != null && !mPendingJobs.contains(runNow)) {
if (runNow != null && !mPendingJobs.contains(runNow)
&& mJobs.containsJob(runNow)) {
mPendingJobs.add(runNow);
}
queueReadyJobsForExecutionLockedH();
}
queueReadyJobsForExecutionH();
break;
case MSG_CHECK_JOB:
// Check the list of jobs and run some of them if we feel inclined.
maybeQueueReadyJobsForExecutionH();
synchronized (mJobs) {
// Check the list of jobs and run some of them if we feel inclined.
maybeQueueReadyJobsForExecutionLockedH();
}
break;
}
maybeRunPendingJobsH();
@@ -538,30 +540,28 @@ public class JobSchedulerService extends com.android.server.SystemService
* Run through list of jobs and execute all possible - at least one is expired so we do
* as many as we can.
*/
private void queueReadyJobsForExecutionH() {
synchronized (mJobs) {
ArraySet<JobStatus> jobs = mJobs.getJobs();
if (DEBUG) {
Slog.d(TAG, "queuing all ready jobs for execution:");
}
for (int i=0; i<jobs.size(); i++) {
JobStatus job = jobs.valueAt(i);
if (isReadyToBeExecutedLocked(job)) {
if (DEBUG) {
Slog.d(TAG, " queued " + job.toShortString());
}
mPendingJobs.add(job);
} else if (isReadyToBeCancelledLocked(job)) {
stopJobOnServiceContextLocked(job);
private void queueReadyJobsForExecutionLockedH() {
ArraySet<JobStatus> jobs = mJobs.getJobs();
if (DEBUG) {
Slog.d(TAG, "queuing all ready jobs for execution:");
}
for (int i=0; i<jobs.size(); i++) {
JobStatus job = jobs.valueAt(i);
if (isReadyToBeExecutedLocked(job)) {
if (DEBUG) {
Slog.d(TAG, " queued " + job.toShortString());
}
mPendingJobs.add(job);
} else if (isReadyToBeCancelledLocked(job)) {
stopJobOnServiceContextLocked(job);
}
if (DEBUG) {
final int queuedJobs = mPendingJobs.size();
if (queuedJobs == 0) {
Slog.d(TAG, "No jobs pending.");
} else {
Slog.d(TAG, queuedJobs + " jobs queued.");
}
}
if (DEBUG) {
final int queuedJobs = mPendingJobs.size();
if (queuedJobs == 0) {
Slog.d(TAG, "No jobs pending.");
} else {
Slog.d(TAG, queuedJobs + " jobs queued.");
}
}
}
@@ -575,55 +575,53 @@ public class JobSchedulerService extends com.android.server.SystemService
* If more than 4 jobs total are ready we send them all off.
* TODO: It would be nice to consolidate these sort of high-level policies somewhere.
*/
private void maybeQueueReadyJobsForExecutionH() {
synchronized (mJobs) {
int chargingCount = 0;
int idleCount = 0;
int backoffCount = 0;
int connectivityCount = 0;
List<JobStatus> runnableJobs = new ArrayList<JobStatus>();
ArraySet<JobStatus> jobs = mJobs.getJobs();
for (int i=0; i<jobs.size(); i++) {
JobStatus job = jobs.valueAt(i);
if (isReadyToBeExecutedLocked(job)) {
if (job.getNumFailures() > 0) {
backoffCount++;
}
if (job.hasIdleConstraint()) {
idleCount++;
}
if (job.hasConnectivityConstraint() || job.hasUnmeteredConstraint()) {
connectivityCount++;
}
if (job.hasChargingConstraint()) {
chargingCount++;
}
runnableJobs.add(job);
} else if (isReadyToBeCancelledLocked(job)) {
stopJobOnServiceContextLocked(job);
}
}
if (backoffCount > 0 ||
idleCount >= MIN_IDLE_COUNT ||
connectivityCount >= MIN_CONNECTIVITY_COUNT ||
chargingCount >= MIN_CHARGING_COUNT ||
runnableJobs.size() >= MIN_READY_JOBS_COUNT) {
if (DEBUG) {
Slog.d(TAG, "maybeQueueReadyJobsForExecutionH: Running jobs.");
}
for (int i=0; i<runnableJobs.size(); i++) {
mPendingJobs.add(runnableJobs.get(i));
}
} else {
if (DEBUG) {
Slog.d(TAG, "maybeQueueReadyJobsForExecutionH: Not running anything.");
private void maybeQueueReadyJobsForExecutionLockedH() {
int chargingCount = 0;
int idleCount = 0;
int backoffCount = 0;
int connectivityCount = 0;
List<JobStatus> runnableJobs = new ArrayList<JobStatus>();
ArraySet<JobStatus> jobs = mJobs.getJobs();
for (int i=0; i<jobs.size(); i++) {
JobStatus job = jobs.valueAt(i);
if (isReadyToBeExecutedLocked(job)) {
if (job.getNumFailures() > 0) {
backoffCount++;
}
if (job.hasIdleConstraint()) {
idleCount++;
}
if (job.hasConnectivityConstraint() || job.hasUnmeteredConstraint()) {
connectivityCount++;
}
if (job.hasChargingConstraint()) {
chargingCount++;
}
runnableJobs.add(job);
} else if (isReadyToBeCancelledLocked(job)) {
stopJobOnServiceContextLocked(job);
}
}
if (backoffCount > 0 ||
idleCount >= MIN_IDLE_COUNT ||
connectivityCount >= MIN_CONNECTIVITY_COUNT ||
chargingCount >= MIN_CHARGING_COUNT ||
runnableJobs.size() >= MIN_READY_JOBS_COUNT) {
if (DEBUG) {
Slog.d(TAG, "idle=" + idleCount + " connectivity=" +
connectivityCount + " charging=" + chargingCount + " tot=" +
runnableJobs.size());
Slog.d(TAG, "maybeQueueReadyJobsForExecutionLockedH: Running jobs.");
}
for (int i=0; i<runnableJobs.size(); i++) {
mPendingJobs.add(runnableJobs.get(i));
}
} else {
if (DEBUG) {
Slog.d(TAG, "maybeQueueReadyJobsForExecutionLockedH: Not running anything.");
}
}
if (DEBUG) {
Slog.d(TAG, "idle=" + idleCount + " connectivity=" +
connectivityCount + " charging=" + chargingCount + " tot=" +
runnableJobs.size());
}
}

View File

@@ -50,15 +50,9 @@ import org.xmlpull.v1.XmlPullParserException;
import org.xmlpull.v1.XmlSerializer;
/**
* Maintain a list of classes, and accessor methods/logic for these jobs.
* This class offers the following functionality:
* - When a job is added, it will determine if the job requirements have changed (update) and
* whether the controllers need to be updated.
* - Persists JobInfos, figures out when to to rewrite the JobInfo to disk.
* - Handles rescheduling of jobs.
* - When a periodic job is executed and must be re-added.
* - When a job fails and the client requests that it be retried with backoff.
* - This class <strong>is not</strong> thread-safe.
* Maintains the master list of jobs that the job scheduler is tracking. These jobs are compared by
* reference, so none of the functions in this class should make a copy.
* Also handles read/write of persisted jobs.
*
* Note on locking:
* All callers to this class must <strong>lock on the class object they are calling</strong>.
@@ -152,6 +146,10 @@ public class JobStore {
return false;
}
boolean containsJob(JobStatus jobStatus) {
return mJobSet.contains(jobStatus);
}
public int size() {
return mJobSet.size();
}
@@ -180,6 +178,10 @@ public class JobStore {
maybeWriteStatusToDiskAsync();
}
/**
* @param userHandle User for whom we are querying the list of jobs.
* @return A list of all the jobs scheduled by the provided user. Never null.
*/
public List<JobStatus> getJobsByUser(int userHandle) {
List<JobStatus> matchingJobs = new ArrayList<JobStatus>();
Iterator<JobStatus> it = mJobSet.iterator();
@@ -194,7 +196,7 @@ public class JobStore {
/**
* @param uid Uid of the requesting app.
* @return All JobStatus objects for a given uid from the master list.
* @return All JobStatus objects for a given uid from the master list. Never null.
*/
public List<JobStatus> getJobsByUid(int uid) {
List<JobStatus> matchingJobs = new ArrayList<JobStatus>();

View File

@@ -67,6 +67,7 @@ public class JobStoreTest extends AndroidTestCase {
assertEquals("Didn't get expected number of persisted tasks.", 1, jobStatusSet.size());
final JobStatus loadedTaskStatus = jobStatusSet.iterator().next();
assertTasksEqual(task, loadedTaskStatus.getJob());
assertTrue("JobStore#contains invalid.", mTaskStoreUnderTest.containsJob(ts));
assertEquals("Different uids.", SOME_UID, loadedTaskStatus.getUid());
compareTimestampsSubjectToIoLatency("Early run-times not the same after read.",
ts.getEarliestRunTime(), loadedTaskStatus.getEarliestRunTime());
@@ -103,7 +104,8 @@ public class JobStoreTest extends AndroidTestCase {
JobStatus loaded2 = it.next();
assertTasksEqual(task1, loaded1.getJob());
assertTasksEqual(task2, loaded2.getJob());
assertTrue("JobStore#contains invalid.", mTaskStoreUnderTest.containsJob(taskStatus1));
assertTrue("JobStore#contains invalid.", mTaskStoreUnderTest.containsJob(taskStatus2));
// Check that the loaded task has the correct runtimes.
compareTimestampsSubjectToIoLatency("Early run-times not the same after read.",
taskStatus1.getEarliestRunTime(), loaded1.getEarliestRunTime());