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

Commit 33d31c5b authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Simplify job scheduler service locking. 

Unify all locks to just one lock protecting the entire service.
There is really no need for more complicated locking -- there is
nothing in the code that can take a long time to complete.  And
having a single lock will allow various parts of the code to be
much simpler and easier to maintain.

This is just the first step of the change, switching all of the
locking to use one lock.  With this done, we can now start
simplifying the code.  For example, JobStatus no longer needs
to do any locking (or have atomic variables and such), it can
just rely on its callers holding the global service lock.

Change-Id: I502916ed7f2994b601750c67a59a96b1a4e95c6d
parent 263e21f1

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

+25 −19
Original line number Diff line number Diff line
@@ -78,13 +78,15 @@ import com.android.server.job.controllers.TimeController; 
 * Any function with the suffix 'Locked' also needs to lock on {@link #mJobs}.

 * @hide

 */

public class JobSchedulerService extends com.android.server.SystemService

public final class JobSchedulerService extends com.android.server.SystemService

        implements StateChangedListener, JobCompletedListener {

    public static final boolean DEBUG = false;

    /** The number of concurrent jobs we run at one time. */

    private static final int MAX_JOB_CONTEXTS_COUNT

            = ActivityManager.isLowRamDeviceStatic() ? 3 : 6;

    static final String TAG = "JobSchedulerService";

    /** Global local for all job scheduler state. */

    final Object mLock = new Object();

    /** Master list of jobs. */

    final JobStore mJobs;
@@ -207,6 +209,10 @@ public class JobSchedulerService extends com.android.server.SystemService 
        }

    };



    public Object getLock() {

        return mLock;

    }



    @Override

    public void onStartUser(int userHandle) {

        mStartedUsers.add(userHandle);
@@ -231,7 +237,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
    }



    public int scheduleAsPackage(JobInfo job, int uId, String packageName, int userId) {

        JobStatus jobStatus = new JobStatus(job, uId, packageName, userId);

        JobStatus jobStatus = new JobStatus(getLock(), job, uId, packageName, userId);

        try {

            if (ActivityManagerNative.getDefault().getAppStartMode(uId,

                    job.getService().getPackageName()) == ActivityManager.APP_START_MODE_DISABLED) {
@@ -243,7 +249,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
        }

        if (DEBUG) Slog.d(TAG, "SCHEDULE: " + jobStatus.toShortString());

        JobStatus toCancel;

        synchronized (mJobs) {

        synchronized (mLock) {

            toCancel = mJobs.getJobByUidAndJobId(uId, job.getId());

        }

        startTrackingJob(jobStatus, toCancel);
@@ -256,7 +262,7 @@ public class JobSchedulerService extends com.android.server.SystemService 


    public List<JobInfo> getPendingJobs(int uid) {

        ArrayList<JobInfo> outList = new ArrayList<JobInfo>();

        synchronized (mJobs) {

        synchronized (mLock) {

            ArraySet<JobStatus> jobs = mJobs.getJobs();

            for (int i=0; i<jobs.size(); i++) {

                JobStatus job = jobs.valueAt(i);
@@ -270,7 +276,7 @@ public class JobSchedulerService extends com.android.server.SystemService 


    void cancelJobsForUser(int userHandle) {

        List<JobStatus> jobsForUser;

        synchronized (mJobs) {

        synchronized (mLock) {

            jobsForUser = mJobs.getJobsByUser(userHandle);

        }

        for (int i=0; i<jobsForUser.size(); i++) {
@@ -289,7 +295,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
     */

    public void cancelJobsForUid(int uid, boolean forceAll) {

        List<JobStatus> jobsForUid;

        synchronized (mJobs) {

        synchronized (mLock) {

            jobsForUid = mJobs.getJobsByUid(uid);

        }

        for (int i=0; i<jobsForUid.size(); i++) {
@@ -317,7 +323,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
     */

    public void cancelJob(int uid, int jobId) {

        JobStatus toCancel;

        synchronized (mJobs) {

        synchronized (mLock) {

            toCancel = mJobs.getJobByUidAndJobId(uid, jobId);

        }

        if (toCancel != null) {
@@ -328,7 +334,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
    private void cancelJobImpl(JobStatus cancelled) {

        if (DEBUG) Slog.d(TAG, "CANCEL: " + cancelled.toShortString());

        stopTrackingJob(cancelled, true /* writeBack */);

        synchronized (mJobs) {

        synchronized (mLock) {

            // Remove from pending queue.

            mPendingJobs.remove(cancelled);

            // Cancel if running.
@@ -340,7 +346,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
    void updateIdleMode(boolean enabled) {

        boolean changed = false;

        boolean rocking;

        synchronized (mJobs) {

        synchronized (mLock) {

            if (mDeviceIdleMode != enabled) {

                changed = true;

            }
@@ -352,7 +358,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
                    mControllers.get(i).deviceIdleModeChanged(enabled);

                }

            }

            synchronized (mJobs) {

            synchronized (mLock) {

                mDeviceIdleMode = enabled;

                if (enabled) {

                    // When becoming idle, make sure no jobs are actively running.
@@ -451,7 +457,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
                // ignored; both services live in system_server

            }

        } else if (phase == PHASE_THIRD_PARTY_APPS_CAN_START) {

            synchronized (mJobs) {

            synchronized (mLock) {

                // Let's go!

                mReadyToRock = true;

                mBatteryStats = IBatteryStats.Stub.asInterface(ServiceManager.getService(
@@ -487,7 +493,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
    private void startTrackingJob(JobStatus jobStatus, JobStatus lastJob) {

        boolean update;

        boolean rocking;

        synchronized (mJobs) {

        synchronized (mLock) {

            update = mJobs.add(jobStatus);

            rocking = mReadyToRock;

        }
@@ -509,7 +515,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
    private boolean stopTrackingJob(JobStatus jobStatus, boolean writeBack) {

        boolean removed;

        boolean rocking;

        synchronized (mJobs) {

        synchronized (mLock) {

            // Remove from store as well as controllers.

            removed = mJobs.remove(jobStatus, writeBack);

            rocking = mReadyToRock;
@@ -693,14 +699,14 @@ public class JobSchedulerService extends com.android.server.SystemService 


        @Override

        public void handleMessage(Message message) {

            synchronized (mJobs) {

            synchronized (mLock) {

                if (!mReadyToRock) {

                    return;

                }

            }

            switch (message.what) {

                case MSG_JOB_EXPIRED:

                    synchronized (mJobs) {

                    synchronized (mLock) {

                        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.
@@ -712,7 +718,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
                    }

                    break;

                case MSG_CHECK_JOB:

                    synchronized (mJobs) {

                    synchronized (mLock) {

                        if (mReportedActive) {

                            // if jobs are currently being run, queue all ready jobs for execution.

                            queueReadyJobsForExecutionLockedH();
@@ -723,7 +729,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
                    }

                    break;

                case MSG_CHECK_JOB_GREEDY:

                    synchronized (mJobs) {

                    synchronized (mLock) {

                        queueReadyJobsForExecutionLockedH();

                    }

                    break;
@@ -879,7 +885,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
         * here is where we decide whether to actually execute it.

         */

        private void maybeRunPendingJobsH() {

            synchronized (mJobs) {

            synchronized (mLock) {

                if (mDeviceIdleMode) {

                    // If device is idle, we will not schedule jobs to run.

                    return;
@@ -1185,7 +1191,7 @@ public class JobSchedulerService extends com.android.server.SystemService 


    void dumpInternal(PrintWriter pw) {

        final long now = SystemClock.elapsedRealtime();

        synchronized (mJobs) {

        synchronized (mLock) {

            pw.print("Started users: ");

            for (int i=0; i<mStartedUsers.size(); i++) {

                pw.print("u" + mStartedUsers.get(i) + " ");

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

+4 −3
Original line number Diff line number Diff line
@@ -103,6 +103,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne 
    private final JobCompletedListener mCompletedListener;

    /** Used for service binding, etc. */

    private final Context mContext;

    private final Object mLock;

    private final IBatteryStats mBatteryStats;

    private PowerManager.WakeLock mWakeLock;
@@ -124,7 +125,6 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne 
    private int mPreferredUid;

    IJobService service;



    private final Object mLock = new Object();

    /**

     * Whether this context is free. This is set to false at the start of execution, and reset to

     * true when execution is complete.
@@ -137,13 +137,14 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne 
    private long mTimeoutElapsed;



    JobServiceContext(JobSchedulerService service, IBatteryStats batteryStats, Looper looper) {

        this(service.getContext(), batteryStats, service, looper);

        this(service.getContext(), service.getLock(), batteryStats, service, looper);

    }



    @VisibleForTesting

    JobServiceContext(Context context, IBatteryStats batteryStats,

    JobServiceContext(Context context, Object lock, IBatteryStats batteryStats,

                      JobCompletedListener completedListener, Looper looper) {

        mContext = context;

        mLock = lock;

        mBatteryStats = batteryStats;

        mCallbackHandler = new JobServiceHandler(looper);

        mCompletedListener = completedListener;

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

+10 −8
Original line number Diff line number Diff line
@@ -70,6 +70,7 @@ public class JobStore { 
    /** Threshold to adjust how often we want to write to the db. */

    private static final int MAX_OPS_BEFORE_WRITE = 1;

    final ArraySet<JobStatus> mJobSet;

    final Object mLock;

    final Context mContext;



    private int mDirtyOperations;
@@ -85,7 +86,7 @@ public class JobStore { 
        synchronized (sSingletonLock) {

            if (sSingleton == null) {

                sSingleton = new JobStore(jobManagerService.getContext(),

                        Environment.getDataDirectory());

                        jobManagerService.getLock(), Environment.getDataDirectory());

            }

            return sSingleton;

        }
@@ -96,7 +97,7 @@ public class JobStore { 
     */

    @VisibleForTesting

    public static JobStore initAndGetForTesting(Context context, File dataDir) {

        JobStore jobStoreUnderTest = new JobStore(context, dataDir);

        JobStore jobStoreUnderTest = new JobStore(context, new Object(), dataDir);

        jobStoreUnderTest.clear();

        return jobStoreUnderTest;

    }
@@ -104,7 +105,8 @@ public class JobStore { 
    /**

     * Construct the instance of the job store. This results in a blocking read from disk.

     */

    private JobStore(Context context, File dataDir) {

    private JobStore(Context context, Object lock, File dataDir) {

        mLock = lock;

        mContext = context;

        mDirtyOperations = 0;
@@ -266,14 +268,14 @@ public class JobStore { 


    /**

     * Runnable that writes {@link #mJobSet} out to xml.

     * NOTE: This Runnable locks on JobStore.this

     * NOTE: This Runnable locks on mLock

     */

    private class WriteJobsMapToDiskRunnable implements Runnable {

        @Override

        public void run() {

            final long startElapsed = SystemClock.elapsedRealtime();

            List<JobStatus> mStoreCopy = new ArrayList<JobStatus>();

            synchronized (JobStore.this) {

            synchronized (mLock) {

                // Copy over the jobs so we can release the lock before writing.

                for (int i=0; i<mJobSet.size(); i++) {

                    JobStatus jobStatus = mJobSet.valueAt(i);
@@ -454,7 +456,7 @@ public class JobStore { 
            try {

                List<JobStatus> jobs;

                FileInputStream fis = mJobsFile.openRead();

                synchronized (JobStore.this) {

                synchronized (mLock) {

                    jobs = readJobMapImpl(fis);

                    if (jobs != null) {

                        for (int i=0; i<jobs.size(); i++) {
@@ -678,8 +680,8 @@ public class JobStore { 
            parser.nextTag(); // Consume </extras>



            JobStatus js = new JobStatus(

                    jobBuilder.build(), uid, sourcePackageName, sourceUserId, elapsedRuntimes.first,

                    elapsedRuntimes.second);

                    mLock, jobBuilder.build(), uid, sourcePackageName, sourceUserId,

                    elapsedRuntimes.first, elapsedRuntimes.second);

            return js;

        }

services/core/java/com/android/server/job/controllers/AppIdleController.java

+10 −8
Original line number Diff line number Diff line
@@ -48,14 +48,16 @@ public class AppIdleController extends StateController { 
    public static AppIdleController get(JobSchedulerService service) {

        synchronized (sCreationLock) {

            if (sController == null) {

                sController = new AppIdleController(service, service.getContext());

                sController = new AppIdleController(service, service.getContext(),

                        service.getLock());

            }

            return sController;

        }

    }



    private AppIdleController(StateChangedListener stateChangedListener, Context context) {

        super(stateChangedListener, context);

    private AppIdleController(StateChangedListener stateChangedListener, Context context,

            Object lock) {

        super(stateChangedListener, context, lock);

        mUsageStatsInternal = LocalServices.getService(UsageStatsManagerInternal.class);

        mAppIdleParoleOn = mUsageStatsInternal.isAppIdleParoleOn();

        mUsageStatsInternal.addAppIdleStateChangeListener(new AppIdleStateChangeListener());
@@ -63,7 +65,7 @@ public class AppIdleController extends StateController { 


    @Override

    public void maybeStartTrackingJob(JobStatus jobStatus, JobStatus lastJob) {

        synchronized (mTrackedTasks) {

        synchronized (mLock) {

            mTrackedTasks.add(jobStatus);

            String packageName = jobStatus.getSourcePackageName();

            final boolean appIdle = !mAppIdleParoleOn && mUsageStatsInternal.isAppIdle(packageName,
@@ -78,7 +80,7 @@ public class AppIdleController extends StateController { 


    @Override

    public void maybeStopTrackingJob(JobStatus jobStatus, boolean forUpdate) {

        synchronized (mTrackedTasks) {

        synchronized (mLock) {

            mTrackedTasks.remove(jobStatus);

        }

    }
@@ -87,7 +89,7 @@ public class AppIdleController extends StateController { 
    public void dumpControllerState(PrintWriter pw) {

        pw.println("AppIdle");

        pw.println("Parole On: " + mAppIdleParoleOn);

        synchronized (mTrackedTasks) {

        synchronized (mLock) {

            for (JobStatus task : mTrackedTasks) {

                pw.print(task.getSourcePackageName());

                pw.print(":idle=" + !task.appNotIdleConstraintSatisfied.get());
@@ -100,7 +102,7 @@ public class AppIdleController extends StateController { 
    void setAppIdleParoleOn(boolean isAppIdleParoleOn) {

        // Flag if any app's idle state has changed

        boolean changed = false;

        synchronized (mTrackedTasks) {

        synchronized (mLock) {

            if (mAppIdleParoleOn == isAppIdleParoleOn) {

                return;

            }
@@ -128,7 +130,7 @@ public class AppIdleController extends StateController { 
        @Override

        public void onAppIdleStateChanged(String packageName, int userId, boolean idle) {

            boolean changed = false;

            synchronized (mTrackedTasks) {

            synchronized (mLock) {

                if (mAppIdleParoleOn) {

                    return;

                }

services/core/java/com/android/server/job/controllers/BatteryController.java

+9 −8
Original line number Diff line number Diff line
@@ -53,7 +53,7 @@ public class BatteryController extends StateController { 
        synchronized (sCreationLock) {

            if (sController == null) {

                sController = new BatteryController(taskManagerService,

                        taskManagerService.getContext());

                        taskManagerService.getContext(), taskManagerService.getLock());

            }

        }

        return sController;
@@ -67,11 +67,12 @@ public class BatteryController extends StateController { 
    @VisibleForTesting

    public static BatteryController getForTesting(StateChangedListener stateChangedListener,

                                           Context context) {

        return new BatteryController(stateChangedListener, context);

        return new BatteryController(stateChangedListener, context, new Object());

    }



    private BatteryController(StateChangedListener stateChangedListener, Context context) {

        super(stateChangedListener, context);

    private BatteryController(StateChangedListener stateChangedListener, Context context,

            Object lock) {

        super(stateChangedListener, context, lock);

        mChargeTracker = new ChargingTracker();

        mChargeTracker.startTracking();

    }
@@ -80,7 +81,7 @@ public class BatteryController extends StateController { 
    public void maybeStartTrackingJob(JobStatus taskStatus, JobStatus lastJob) {

        final boolean isOnStablePower = mChargeTracker.isOnStablePower();

        if (taskStatus.hasChargingConstraint()) {

            synchronized (mTrackedTasks) {

            synchronized (mLock) {

                mTrackedTasks.add(taskStatus);

                taskStatus.chargingConstraintSatisfied.set(isOnStablePower);

            }
@@ -90,7 +91,7 @@ public class BatteryController extends StateController { 
    @Override

    public void maybeStopTrackingJob(JobStatus taskStatus, boolean forUpdate) {

        if (taskStatus.hasChargingConstraint()) {

            synchronized (mTrackedTasks) {

            synchronized (mLock) {

                mTrackedTasks.remove(taskStatus);

            }

        }
@@ -102,7 +103,7 @@ public class BatteryController extends StateController { 
            Slog.d(TAG, "maybeReportNewChargingState: " + stablePower);

        }

        boolean reportChange = false;

        synchronized (mTrackedTasks) {

        synchronized (mLock) {

            for (JobStatus ts : mTrackedTasks) {

                boolean previous = ts.chargingConstraintSatisfied.getAndSet(stablePower);

                if (previous != stablePower) {
@@ -200,7 +201,7 @@ public class BatteryController extends StateController { 
    public void dumpControllerState(PrintWriter pw) {

        pw.println("Batt.");

        pw.println("Stable power: " + mChargeTracker.isOnStablePower());

        synchronized (mTrackedTasks) {

        synchronized (mLock) {

            Iterator<JobStatus> it = mTrackedTasks.iterator();

            if (it.hasNext()) {

                pw.print(String.valueOf(it.next().hashCode()));