Merge "Fix lock ordering in JobScheduler" into lmp-dev (8cfd9e6c) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/job/JobSchedulerService.java

+99 −101

Original line number	Diff line number	Diff line
		@@ -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);
		jobsForUser = mJobs.getJobsByUser(userHandle);
		}
		for (int i=0; i<jobsForUser.size(); i++) {
		JobStatus toRemove = jobsForUser.get(i);
		cancelJobLocked(toRemove);
		}
		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);
		jobsForUid = mJobs.getJobsByUid(uid);
		}
		for (int i=0; i<jobsForUid.size(); i++) {
		JobStatus toRemove = jobsForUid.get(i);
		cancelJobLocked(toRemove);
		}
		cancelJobImpl(toRemove);
		}
		}

		@@ -232,23 +233,24 @@ 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);
		synchronized (mJobs) {
		// Remove from pending queue.
		mPendingJobs.remove(cancelled);
		// Cancel if running.
		stopJobOnServiceContextLocked(cancelled);
		}
		}

		/**
		* Initializes the system service.
		@@ -482,22 +484,14 @@ 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();
		}
		}
		}

		@Override
		public void onRunJobNow(JobStatus jobStatus) {
		@@ -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:
		synchronized (mJobs) {
		// Check the list of jobs and run some of them if we feel inclined.
		maybeQueueReadyJobsForExecutionH();
		maybeQueueReadyJobsForExecutionLockedH();
		}
		break;
		}
		maybeRunPendingJobsH();
		@@ -538,8 +540,7 @@ 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) {
		private void queueReadyJobsForExecutionLockedH() {
		ArraySet<JobStatus> jobs = mJobs.getJobs();
		if (DEBUG) {
		Slog.d(TAG, "queuing all ready jobs for execution:");
		@@ -564,7 +565,6 @@ public class JobSchedulerService extends com.android.server.SystemService
		}
		}
		}
		}

		/**
		* The state of at least one job has changed. Here is where we could enforce various
		@@ -575,8 +575,7 @@ 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) {
		private void maybeQueueReadyJobsForExecutionLockedH() {
		int chargingCount = 0;
		int idleCount = 0;
		int backoffCount = 0;
		@@ -609,14 +608,14 @@ public class JobSchedulerService extends com.android.server.SystemService
		chargingCount >= MIN_CHARGING_COUNT \|\|
		runnableJobs.size() >= MIN_READY_JOBS_COUNT) {
		if (DEBUG) {
		Slog.d(TAG, "maybeQueueReadyJobsForExecutionH: Running jobs.");
		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, "maybeQueueReadyJobsForExecutionH: Not running anything.");
		Slog.d(TAG, "maybeQueueReadyJobsForExecutionLockedH: Not running anything.");
		}
		}
		if (DEBUG) {
		@@ -625,7 +624,6 @@ public class JobSchedulerService extends com.android.server.SystemService
		runnableJobs.size());
		}
		}
		}

		/**
		* Criteria for moving a job into the pending queue:

services/core/java/com/android/server/job/JobStore.java

+12 −10

Original line number	Diff line number	Diff line
		@@ -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>();

services/tests/servicestests/src/com/android/server/job/JobStoreTest.java

+3 −1

Original line number	Diff line number	Diff line
		@@ -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());