Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 85b1df18 authored by Matthew Williams's avatar Matthew Williams Committed by Android (Google) Code Review
Browse files

Merge "Fix JobScheduler race condition" into lmp-dev

parents 63c4c06b 01ac45b6
Loading
Loading
Loading
Loading
+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);
}
+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) {
+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.");
+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());
    }

    /**