Fix JobScheduler race condition (01ac45b6) · Commits · e / os / android_frameworks_base

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

deleted100644 → 0

+0 −34

Original line number	Diff line number	Diff line
		/*
		* Copyright (C) 2014 The Android Open Source Project
		*
		* Licensed under the Apache License, Version 2.0 (the "License");
		* you may not use this file except in compliance with the License.
		* You may obtain a copy of the License at
		*
		* http://www.apache.org/licenses/LICENSE-2.0
		*
		* Unless required by applicable law or agreed to in writing, software
		* distributed under the License is distributed on an "AS IS" BASIS,
		* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
		* See the License for the specific language governing permissions and
		* limitations under the License
		*/

		package com.android.server.job;

		import java.util.List;

		import com.android.server.job.controllers.JobStatus;

		/**
		* Callback definition for I/O thread to let the JobManagerService know when
		* I/O read has completed. Done this way so we don't stall the main thread on
		* boot.
		*/
		public interface JobMapReadFinishedListener {

		/**
		* Called by the {@link JobStore} at boot, when the disk read is finished.
		*/
		public void onJobMapReadFinished(List<JobStatus> jobs);
		}

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

+1 −23

Original line number	Diff line number	Diff line
		@@ -69,7 +69,7 @@ import com.android.server.job.controllers.TimeController;
		* @hide
		*/
		public class JobSchedulerService extends com.android.server.SystemService
		implements StateChangedListener, JobCompletedListener, JobMapReadFinishedListener {
		implements StateChangedListener, JobCompletedListener {
		// TODO: Switch this off for final version.
		static final boolean DEBUG = true;
		/** The number of concurrent jobs we run at one time. */
		@@ -487,28 +487,6 @@ public class JobSchedulerService extends com.android.server.SystemService
		mHandler.obtainMessage(MSG_JOB_EXPIRED, jobStatus).sendToTarget();
		}

		/**
		* Disk I/O is finished, take the list of jobs we read from disk and add them to our
		* {@link JobStore}.
		* This is run on the {@link com.android.server.IoThread} instance, which is a separate thread,
		* and is called once at boot.
		*/
		@Override
		public void onJobMapReadFinished(List<JobStatus> jobs) {
		synchronized (mJobs) {
		for (int i=0; i<jobs.size(); i++) {
		JobStatus js = jobs.get(i);
		if (mJobs.containsJobIdForUid(js.getJobId(), js.getUid())) {
		// An app with BOOT_COMPLETED might have decided to reschedule their job, in
		// the same amount of time it took us to read it from disk. If this is the case
		// we leave it be.
		continue;
		}
		startTrackingJob(js);
		}
		}
		}

		private class JobHandler extends Handler {

		public JobHandler(Looper looper) {

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

+32 −21

Original line number	Diff line number	Diff line
		@@ -88,19 +88,26 @@ public class JobStore {
		synchronized (sSingletonLock) {
		if (sSingleton == null) {
		sSingleton = new JobStore(jobManagerService.getContext(),
		Environment.getDataDirectory(), jobManagerService);
		Environment.getDataDirectory());
		}
		return sSingleton;
		}
		}

		/**
		* @return A freshly initialized job store object, with no loaded jobs.
		*/
		@VisibleForTesting
		public static JobStore initAndGetForTesting(Context context, File dataDir,
		JobMapReadFinishedListener callback) {
		return new JobStore(context, dataDir, callback);
		public static JobStore initAndGetForTesting(Context context, File dataDir) {
		JobStore jobStoreUnderTest = new JobStore(context, dataDir);
		jobStoreUnderTest.clear();
		return jobStoreUnderTest;
		}

		private JobStore(Context context, File dataDir, JobMapReadFinishedListener callback) {
		/**
		* Construct the instance of the job store. This results in a blocking read from disk.
		*/
		private JobStore(Context context, File dataDir) {
		mContext = context;
		mDirtyOperations = 0;

		@@ -111,7 +118,7 @@ public class JobStore {

		mJobSet = new ArraySet<JobStatus>();

		readJobMapFromDiskAsync(callback);
		readJobMapFromDisk(mJobSet);
		}

		/**
		@@ -249,12 +256,9 @@ public class JobStore {
		}
		}

		private void readJobMapFromDiskAsync(JobMapReadFinishedListener callback) {
		mIoHandler.post(new ReadJobMapFromDiskRunnable(callback));
		}

		public void readJobMapFromDisk(JobMapReadFinishedListener callback) {
		new ReadJobMapFromDiskRunnable(callback).run();
		@VisibleForTesting
		public void readJobMapFromDisk(ArraySet<JobStatus> jobSet) {
		new ReadJobMapFromDiskRunnable(jobSet).run();
		}

		/**
		@@ -398,13 +402,18 @@ public class JobStore {
		}

		/**
		* Runnable that reads list of persisted job from xml.
		* NOTE: This Runnable locks on JobStore.this
		* Runnable that reads list of persisted job from xml. This is run once at start up, so doesn't
		* need to go through {@link JobStore#add(com.android.server.job.controllers.JobStatus)}.
		*/
		private class ReadJobMapFromDiskRunnable implements Runnable {
		private JobMapReadFinishedListener mCallback;
		public ReadJobMapFromDiskRunnable(JobMapReadFinishedListener callback) {
		mCallback = callback;
		private final ArraySet<JobStatus> jobSet;

		/**
		* @param jobSet Reference to the (empty) set of JobStatus objects that back the JobStore,
		* so that after disk read we can populate it directly.
		*/
		ReadJobMapFromDiskRunnable(ArraySet<JobStatus> jobSet) {
		this.jobSet = jobSet;
		}

		@Override
		@@ -414,11 +423,13 @@ public class JobStore {
		FileInputStream fis = mJobsFile.openRead();
		synchronized (JobStore.this) {
		jobs = readJobMapImpl(fis);
		}
		fis.close();
		if (jobs != null) {
		mCallback.onJobMapReadFinished(jobs);
		for (int i=0; i<jobs.size(); i++) {
		this.jobSet.add(jobs.get(i));
		}
		}
		}
		fis.close();
		} catch (FileNotFoundException e) {
		if (JobSchedulerService.DEBUG) {
		Slog.d(TAG, "Could not find jobs file, probably there was nothing to load.");

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

+44 −58

Original line number	Diff line number	Diff line
		package com.android.server.task;
		package com.android.server.job;


		import android.content.ComponentName;
		@@ -9,40 +9,32 @@ import android.os.PersistableBundle;
		import android.test.AndroidTestCase;
		import android.test.RenamingDelegatingContext;
		import android.util.Log;
		import android.util.ArraySet;

		import com.android.server.job.JobMapReadFinishedListener;
		import com.android.server.job.JobStore;
		import com.android.server.job.controllers.JobStatus;

		import java.util.List;
		import java.util.Iterator;

		/**
		* Test reading and writing correctly from file.
		*/
		public class TaskStoreTest extends AndroidTestCase {
		public class JobStoreTest extends AndroidTestCase {
		private static final String TAG = "TaskStoreTest";
		private static final String TEST_PREFIX = "_test_";
		// private static final int USER_NON_0 = 3;

		private static final int SOME_UID = 34234;
		private ComponentName mComponent;
		private static final long IO_WAIT = 600L;
		private static final long IO_WAIT = 1000L;

		JobStore mTaskStoreUnderTest;
		Context mTestContext;
		JobMapReadFinishedListener mTaskMapReadFinishedListenerStub =
		new JobMapReadFinishedListener() {
		@Override
		public void onJobMapReadFinished(List<JobStatus> tasks) {
		// do nothing.
		}
		};

		@Override
		public void setUp() throws Exception {
		mTestContext = new RenamingDelegatingContext(getContext(), TEST_PREFIX);
		Log.d(TAG, "Saving tasks to '" + mTestContext.getFilesDir() + "'");
		mTaskStoreUnderTest = JobStore.initAndGetForTesting(mTestContext,
		mTestContext.getFilesDir(), mTaskMapReadFinishedListenerStub);
		mTaskStoreUnderTest =
		JobStore.initAndGetForTesting(mTestContext, mTestContext.getFilesDir());
		mComponent = new ComponentName(getContext().getPackageName(), StubClass.class.getName());
		}

		@@ -69,19 +61,17 @@ public class TaskStoreTest extends AndroidTestCase {
		mTaskStoreUnderTest.add(ts);
		Thread.sleep(IO_WAIT);
		// Manually load tasks from xml file.
		mTaskStoreUnderTest.readJobMapFromDisk(new JobMapReadFinishedListener() {
		@Override
		public void onJobMapReadFinished(List<JobStatus> tasks) {
		assertEquals("Didn't get expected number of persisted tasks.", 1, tasks.size());
		JobStatus loadedTaskStatus = tasks.get(0);
		final ArraySet<JobStatus> jobStatusSet = new ArraySet<JobStatus>();
		mTaskStoreUnderTest.readJobMapFromDisk(jobStatusSet);

		assertEquals("Didn't get expected number of persisted tasks.", 1, jobStatusSet.size());
		final JobStatus loadedTaskStatus = jobStatusSet.iterator().next();
		assertTasksEqual(task, loadedTaskStatus.getJob());
		assertEquals("Different uids.", SOME_UID, tasks.get(0).getUid());
		assertEquals("Different uids.", SOME_UID, loadedTaskStatus.getUid());
		compareTimestampsSubjectToIoLatency("Early run-times not the same after read.",
		ts.getEarliestRunTime(), loadedTaskStatus.getEarliestRunTime());
		compareTimestampsSubjectToIoLatency("Late run-times not the same after read.",
		ts.getLatestRunTimeElapsed(), loadedTaskStatus.getLatestRunTimeElapsed());
		}
		});

		}

		@@ -104,12 +94,13 @@ public class TaskStoreTest extends AndroidTestCase {
		mTaskStoreUnderTest.add(taskStatus1);
		mTaskStoreUnderTest.add(taskStatus2);
		Thread.sleep(IO_WAIT);
		mTaskStoreUnderTest.readJobMapFromDisk(new JobMapReadFinishedListener() {
		@Override
		public void onJobMapReadFinished(List<JobStatus> tasks) {
		assertEquals("Incorrect # of persisted tasks.", 2, tasks.size());
		JobStatus loaded1 = tasks.get(0);
		JobStatus loaded2 = tasks.get(1);

		final ArraySet<JobStatus> jobStatusSet = new ArraySet<JobStatus>();
		mTaskStoreUnderTest.readJobMapFromDisk(jobStatusSet);
		assertEquals("Incorrect # of persisted tasks.", 2, jobStatusSet.size());
		Iterator<JobStatus> it = jobStatusSet.iterator();
		JobStatus loaded1 = it.next();
		JobStatus loaded2 = it.next();
		assertTasksEqual(task1, loaded1.getJob());
		assertTasksEqual(task2, loaded2.getJob());

		@@ -122,8 +113,6 @@ public class TaskStoreTest extends AndroidTestCase {
		taskStatus2.getEarliestRunTime(), loaded2.getEarliestRunTime());
		compareTimestampsSubjectToIoLatency("Late run-times not the same after read.",
		taskStatus2.getLatestRunTimeElapsed(), loaded2.getLatestRunTimeElapsed());
		}
		});

		}

		@@ -144,15 +133,12 @@ public class TaskStoreTest extends AndroidTestCase {

		mTaskStoreUnderTest.add(taskStatus);
		Thread.sleep(IO_WAIT);
		mTaskStoreUnderTest.readJobMapFromDisk(new JobMapReadFinishedListener() {
		@Override
		public void onJobMapReadFinished(List<JobStatus> tasks) {
		assertEquals("Incorrect # of persisted tasks.", 1, tasks.size());
		JobStatus loaded = tasks.get(0);
		assertTasksEqual(task, loaded.getJob());
		}
		});

		final ArraySet<JobStatus> jobStatusSet = new ArraySet<JobStatus>();
		mTaskStoreUnderTest.readJobMapFromDisk(jobStatusSet);
		assertEquals("Incorrect # of persisted tasks.", 1, jobStatusSet.size());
		JobStatus loaded = jobStatusSet.iterator().next();
		assertTasksEqual(task, loaded.getJob());
		}

		/**