diff --git a/services/core/java/com/android/server/job/JobSchedulerService.java b/services/core/java/com/android/server/job/JobSchedulerService.java index 57cede8e7d092..25b54acf6b786 100644 --- a/services/core/java/com/android/server/job/JobSchedulerService.java +++ b/services/core/java/com/android/server/job/JobSchedulerService.java @@ -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 getPendingJobs(int uid) { ArrayList outList = new ArrayList(); - synchronized (mJobs) { + synchronized (mLock) { ArraySet jobs = mJobs.getJobs(); for (int i=0; i jobsForUser; - synchronized (mJobs) { + synchronized (mLock) { jobsForUser = mJobs.getJobsByUser(userHandle); } for (int i=0; i jobsForUid; - synchronized (mJobs) { + synchronized (mLock) { jobsForUid = mJobs.getJobsByUid(uid); } for (int i=0; i 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 mStoreCopy = new ArrayList(); - synchronized (JobStore.this) { + synchronized (mLock) { // Copy over the jobs so we can release the lock before writing. for (int i=0; i jobs; FileInputStream fis = mJobsFile.openRead(); - synchronized (JobStore.this) { + synchronized (mLock) { jobs = readJobMapImpl(fis); if (jobs != null) { for (int i=0; i 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; } diff --git a/services/core/java/com/android/server/job/controllers/AppIdleController.java b/services/core/java/com/android/server/job/controllers/AppIdleController.java index 5f3da7577a5d1..f7f34aec60386 100644 --- a/services/core/java/com/android/server/job/controllers/AppIdleController.java +++ b/services/core/java/com/android/server/job/controllers/AppIdleController.java @@ -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; } diff --git a/services/core/java/com/android/server/job/controllers/BatteryController.java b/services/core/java/com/android/server/job/controllers/BatteryController.java index b322a3e96c149..b101c82649837 100644 --- a/services/core/java/com/android/server/job/controllers/BatteryController.java +++ b/services/core/java/com/android/server/job/controllers/BatteryController.java @@ -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 it = mTrackedTasks.iterator(); if (it.hasNext()) { pw.print(String.valueOf(it.next().hashCode())); diff --git a/services/core/java/com/android/server/job/controllers/ConnectivityController.java b/services/core/java/com/android/server/job/controllers/ConnectivityController.java index b84658a7db96d..29b54c2eb7166 100644 --- a/services/core/java/com/android/server/job/controllers/ConnectivityController.java +++ b/services/core/java/com/android/server/job/controllers/ConnectivityController.java @@ -58,14 +58,15 @@ public class ConnectivityController extends StateController implements public static ConnectivityController get(JobSchedulerService jms) { synchronized (sCreationLock) { if (mSingleton == null) { - mSingleton = new ConnectivityController(jms, jms.getContext()); + mSingleton = new ConnectivityController(jms, jms.getContext(), jms.getLock()); } return mSingleton; } } - private ConnectivityController(StateChangedListener stateChangedListener, Context context) { - super(stateChangedListener, context); + private ConnectivityController(StateChangedListener stateChangedListener, Context context, + Object lock) { + super(stateChangedListener, context, lock); // Register connectivity changed BR. IntentFilter intentFilter = new IntentFilter(); intentFilter.addAction(ConnectivityManager.CONNECTIVITY_ACTION); @@ -84,7 +85,7 @@ public class ConnectivityController extends StateController implements @Override public void maybeStartTrackingJob(JobStatus jobStatus, JobStatus lastJob) { if (jobStatus.hasConnectivityConstraint() || jobStatus.hasUnmeteredConstraint()) { - synchronized (mTrackedJobs) { + synchronized (mLock) { jobStatus.connectivityConstraintSatisfied.set(mNetworkConnected); jobStatus.unmeteredConstraintSatisfied.set(mNetworkUnmetered); mTrackedJobs.add(jobStatus); @@ -95,7 +96,7 @@ public class ConnectivityController extends StateController implements @Override public void maybeStopTrackingJob(JobStatus jobStatus, boolean forUpdate) { if (jobStatus.hasConnectivityConstraint() || jobStatus.hasUnmeteredConstraint()) { - synchronized (mTrackedJobs) { + synchronized (mLock) { mTrackedJobs.remove(jobStatus); } } @@ -105,7 +106,7 @@ public class ConnectivityController extends StateController implements * @param userId Id of the user for whom we are updating the connectivity state. */ private void updateTrackedJobs(int userId) { - synchronized (mTrackedJobs) { + synchronized (mLock) { boolean changed = false; for (JobStatus js : mTrackedJobs) { if (js.getUserId() != userId) { @@ -128,7 +129,7 @@ public class ConnectivityController extends StateController implements * We know the network has just come up. We want to run any jobs that are ready. */ public synchronized void onNetworkActive() { - synchronized (mTrackedJobs) { + synchronized (mLock) { for (JobStatus js : mTrackedJobs) { if (js.isReady()) { if (DEBUG) { diff --git a/services/core/java/com/android/server/job/controllers/ContentObserverController.java b/services/core/java/com/android/server/job/controllers/ContentObserverController.java index 212cc949364d1..af994f037af76 100644 --- a/services/core/java/com/android/server/job/controllers/ContentObserverController.java +++ b/services/core/java/com/android/server/job/controllers/ContentObserverController.java @@ -57,7 +57,7 @@ public class ContentObserverController extends StateController { synchronized (sCreationLock) { if (sController == null) { sController = new ContentObserverController(taskManagerService, - taskManagerService.getContext()); + taskManagerService.getContext(), taskManagerService.getLock()); } } return sController; @@ -66,17 +66,18 @@ public class ContentObserverController extends StateController { @VisibleForTesting public static ContentObserverController getForTesting(StateChangedListener stateChangedListener, Context context) { - return new ContentObserverController(stateChangedListener, context); + return new ContentObserverController(stateChangedListener, context, new Object()); } - private ContentObserverController(StateChangedListener stateChangedListener, Context context) { - super(stateChangedListener, context); + private ContentObserverController(StateChangedListener stateChangedListener, Context context, + Object lock) { + super(stateChangedListener, context, lock); } @Override public void maybeStartTrackingJob(JobStatus taskStatus, JobStatus lastJob) { if (taskStatus.hasContentTriggerConstraint()) { - synchronized (mTrackedTasks) { + synchronized (mLock) { if (taskStatus.contentObserverJobInstance == null) { taskStatus.contentObserverJobInstance = new JobInstance(taskStatus); } @@ -128,7 +129,7 @@ public class ContentObserverController extends StateController { @Override public void prepareForExecution(JobStatus taskStatus) { if (taskStatus.hasContentTriggerConstraint()) { - synchronized (mTrackedTasks) { + synchronized (mLock) { if (taskStatus.contentObserverJobInstance != null) { taskStatus.changedUris = taskStatus.contentObserverJobInstance.mChangedUris; taskStatus.changedAuthorities @@ -143,7 +144,7 @@ public class ContentObserverController extends StateController { @Override public void maybeStopTrackingJob(JobStatus taskStatus, boolean forUpdate) { if (taskStatus.hasContentTriggerConstraint()) { - synchronized (mTrackedTasks) { + synchronized (mLock) { if (!forUpdate) { // We won't do this reset if being called for an update, because // we know it will be immediately followed by maybeStartTrackingJob... @@ -162,7 +163,7 @@ public class ContentObserverController extends StateController { public void rescheduleForFailure(JobStatus newJob, JobStatus failureToReschedule) { if (failureToReschedule.hasContentTriggerConstraint() && newJob.hasContentTriggerConstraint()) { - synchronized (mTrackedTasks) { + synchronized (mLock) { // Our job has failed, and we are scheduling a new job for it. // Copy the last reported content changes in to the new job, so when // we schedule the new one we will pick them up and report them again. @@ -184,7 +185,7 @@ public class ContentObserverController extends StateController { @Override public void onChange(boolean selfChange, Uri uri) { boolean reportChange = false; - synchronized (mTrackedTasks) { + synchronized (mLock) { final int N = mJobs.size(); for (int i=0; i it = mTrackedTasks.iterator(); if (it.hasNext()) { pw.print(String.valueOf(it.next().hashCode())); diff --git a/services/core/java/com/android/server/job/controllers/IdleController.java b/services/core/java/com/android/server/job/controllers/IdleController.java index 9f4cdef0e5f76..6b85f222663cb 100644 --- a/services/core/java/com/android/server/job/controllers/IdleController.java +++ b/services/core/java/com/android/server/job/controllers/IdleController.java @@ -51,14 +51,15 @@ public class IdleController extends StateController { public static IdleController get(JobSchedulerService service) { synchronized (sCreationLock) { if (sController == null) { - sController = new IdleController(service, service.getContext()); + sController = new IdleController(service, service.getContext(), service.getLock()); } return sController; } } - private IdleController(StateChangedListener stateChangedListener, Context context) { - super(stateChangedListener, context); + private IdleController(StateChangedListener stateChangedListener, Context context, + Object lock) { + super(stateChangedListener, context, lock); initIdleStateTracking(); } @@ -68,7 +69,7 @@ public class IdleController extends StateController { @Override public void maybeStartTrackingJob(JobStatus taskStatus, JobStatus lastJob) { if (taskStatus.hasIdleConstraint()) { - synchronized (mTrackedTasks) { + synchronized (mLock) { mTrackedTasks.add(taskStatus); taskStatus.idleConstraintSatisfied.set(mIdleTracker.isIdle()); } @@ -77,7 +78,7 @@ public class IdleController extends StateController { @Override public void maybeStopTrackingJob(JobStatus taskStatus, boolean forUpdate) { - synchronized (mTrackedTasks) { + synchronized (mLock) { mTrackedTasks.remove(taskStatus); } } @@ -86,7 +87,7 @@ public class IdleController extends StateController { * Interaction with the task manager service */ void reportNewIdleState(boolean isIdle) { - synchronized (mTrackedTasks) { + synchronized (mLock) { for (JobStatus task : mTrackedTasks) { task.idleConstraintSatisfied.set(isIdle); } @@ -194,7 +195,7 @@ public class IdleController extends StateController { @Override public void dumpControllerState(PrintWriter pw) { - synchronized (mTrackedTasks) { + synchronized (mLock) { pw.print("Idle: "); pw.println(mIdleTracker.isIdle() ? "true" : "false"); pw.println(mTrackedTasks.size()); diff --git a/services/core/java/com/android/server/job/controllers/JobStatus.java b/services/core/java/com/android/server/job/controllers/JobStatus.java index c4d564c4e577e..3cf105480b5d4 100644 --- a/services/core/java/com/android/server/job/controllers/JobStatus.java +++ b/services/core/java/com/android/server/job/controllers/JobStatus.java @@ -46,6 +46,11 @@ public class JobStatus { public static final long NO_LATEST_RUNTIME = Long.MAX_VALUE; public static final long NO_EARLIEST_RUNTIME = 0L; + /** + * Service global lock. NOTE: this should be removed, having this class just rely on + * its callers doing the appropriate locking. + */ + final Object lock; final JobInfo job; /** Uid of the package requesting this job. */ final int callingUid; @@ -94,8 +99,9 @@ public class JobStatus { return callingUid; } - private JobStatus(JobInfo job, int callingUid, String sourcePackageName, int sourceUserId, - int numFailures) { + private JobStatus(Object lock, JobInfo job, int callingUid, String sourcePackageName, + int sourceUserId, int numFailures) { + this.lock = lock; this.job = job; this.callingUid = callingUid; this.name = job.getService().flattenToShortString(); @@ -124,8 +130,9 @@ public class JobStatus { /** Copy constructor. */ public JobStatus(JobStatus jobStatus) { - this(jobStatus.getJob(), jobStatus.getUid(), jobStatus.getSourcePackageName(), - jobStatus.getSourceUserId(), jobStatus.getNumFailures()); + this(jobStatus.lock, jobStatus.getJob(), jobStatus.getUid(), + jobStatus.getSourcePackageName(), jobStatus.getSourceUserId(), + jobStatus.getNumFailures()); this.earliestRunTimeElapsedMillis = jobStatus.getEarliestRunTime(); this.latestRunTimeElapsedMillis = jobStatus.getLatestRunTimeElapsed(); } @@ -138,8 +145,9 @@ public class JobStatus { * @param sourceUserId User id for whom this job is scheduled. -1 indicates this is same as the * calling userId. */ - public JobStatus(JobInfo job, int callingUid, String sourcePackageName, int sourceUserId) { - this(job, callingUid, sourcePackageName, sourceUserId, 0); + public JobStatus(Object lock, JobInfo job, int callingUid, String sourcePackageName, + int sourceUserId) { + this(lock, job, callingUid, sourcePackageName, sourceUserId, 0); final long elapsedNow = SystemClock.elapsedRealtime(); @@ -161,9 +169,9 @@ public class JobStatus { * wallclock runtime rather than resetting it on every boot. * We consider a freshly loaded job to no longer be in back-off. */ - public JobStatus(JobInfo job, int callingUid, String sourcePackageName, int sourceUserId, - long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis) { - this(job, callingUid, sourcePackageName, sourceUserId, 0); + public JobStatus(Object lock, JobInfo job, int callingUid, String sourcePackageName, + int sourceUserId, long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis) { + this(lock, job, callingUid, sourcePackageName, sourceUserId, 0); this.earliestRunTimeElapsedMillis = earliestRunTimeElapsedMillis; this.latestRunTimeElapsedMillis = latestRunTimeElapsedMillis; @@ -172,7 +180,8 @@ public class JobStatus { /** Create a new job to be rescheduled with the provided parameters. */ public JobStatus(JobStatus rescheduling, long newEarliestRuntimeElapsedMillis, long newLatestRuntimeElapsedMillis, int backoffAttempt) { - this(rescheduling.job, rescheduling.getUid(), rescheduling.getSourcePackageName(), + this(rescheduling.lock, rescheduling.job, rescheduling.getUid(), + rescheduling.getSourcePackageName(), rescheduling.getSourceUserId(), backoffAttempt); earliestRunTimeElapsedMillis = newEarliestRuntimeElapsedMillis; @@ -275,27 +284,31 @@ public class JobStatus { * @return Whether or not this job is ready to run, based on its requirements. This is true if * the constraints are satisfied or the deadline on the job has expired. */ - public synchronized boolean isReady() { - // Deadline constraint trumps other constraints (except for periodic jobs where deadline - // (is an implementation detail. A periodic job should only run if it's constraints are - // satisfied). - // AppNotIdle implicit constraint trumps all! - return (isConstraintsSatisfied() + public boolean isReady() { + synchronized (lock) { + // Deadline constraint trumps other constraints (except for periodic jobs where deadline + // (is an implementation detail. A periodic job should only run if it's constraints are + // satisfied). + // AppNotIdle implicit constraint trumps all! + return (isConstraintsSatisfied() || (!job.isPeriodic() - && hasDeadlineConstraint() && deadlineConstraintSatisfied.get())) - && appNotIdleConstraintSatisfied.get(); + && hasDeadlineConstraint() && deadlineConstraintSatisfied.get())) + && appNotIdleConstraintSatisfied.get(); + } } /** * @return Whether the constraints set on this job are satisfied. */ - public synchronized boolean isConstraintsSatisfied() { - return (!hasChargingConstraint() || chargingConstraintSatisfied.get()) - && (!hasTimingDelayConstraint() || timeDelayConstraintSatisfied.get()) - && (!hasConnectivityConstraint() || connectivityConstraintSatisfied.get()) - && (!hasUnmeteredConstraint() || unmeteredConstraintSatisfied.get()) - && (!hasIdleConstraint() || idleConstraintSatisfied.get()) - && (!hasContentTriggerConstraint() || contentTriggerConstraintSatisfied.get()); + public boolean isConstraintsSatisfied() { + synchronized (lock) { + return (!hasChargingConstraint() || chargingConstraintSatisfied.get()) + && (!hasTimingDelayConstraint() || timeDelayConstraintSatisfied.get()) + && (!hasConnectivityConstraint() || connectivityConstraintSatisfied.get()) + && (!hasUnmeteredConstraint() || unmeteredConstraintSatisfied.get()) + && (!hasIdleConstraint() || idleConstraintSatisfied.get()) + && (!hasContentTriggerConstraint() || contentTriggerConstraintSatisfied.get()); + } } public boolean matches(int uid, int jobId) { diff --git a/services/core/java/com/android/server/job/controllers/StateController.java b/services/core/java/com/android/server/job/controllers/StateController.java index b619ea8cdd5dd..8b8611a14d69e 100644 --- a/services/core/java/com/android/server/job/controllers/StateController.java +++ b/services/core/java/com/android/server/job/controllers/StateController.java @@ -30,13 +30,16 @@ import java.io.PrintWriter; */ public abstract class StateController { protected static final boolean DEBUG = JobSchedulerService.DEBUG; - protected Context mContext; - protected StateChangedListener mStateChangedListener; + protected final Context mContext; + protected final Object mLock; + protected final StateChangedListener mStateChangedListener; protected boolean mDeviceIdleMode; - public StateController(StateChangedListener stateChangedListener, Context context) { + public StateController(StateChangedListener stateChangedListener, Context context, + Object lock) { mStateChangedListener = stateChangedListener; mContext = context; + mLock = lock; } public void deviceIdleModeChanged(boolean enabled) { diff --git a/services/core/java/com/android/server/job/controllers/TimeController.java b/services/core/java/com/android/server/job/controllers/TimeController.java index a68c3adabb087..bf17827d49f8d 100644 --- a/services/core/java/com/android/server/job/controllers/TimeController.java +++ b/services/core/java/com/android/server/job/controllers/TimeController.java @@ -54,13 +54,14 @@ public class TimeController extends StateController { public static synchronized TimeController get(JobSchedulerService jms) { if (mSingleton == null) { - mSingleton = new TimeController(jms, jms.getContext()); + mSingleton = new TimeController(jms, jms.getContext(), jms.getLock()); } return mSingleton; } - private TimeController(StateChangedListener stateChangedListener, Context context) { - super(stateChangedListener, context); + private TimeController(StateChangedListener stateChangedListener, Context context, + Object lock) { + super(stateChangedListener, context, lock); mNextJobExpiredElapsedMillis = Long.MAX_VALUE; mNextDelayExpiredElapsedMillis = Long.MAX_VALUE; @@ -71,27 +72,28 @@ public class TimeController extends StateController { * list. */ @Override - public synchronized void maybeStartTrackingJob(JobStatus job, JobStatus lastJob) { - if (job.hasTimingDelayConstraint() || job.hasDeadlineConstraint()) { - maybeStopTrackingJob(job, false); - boolean isInsert = false; - ListIterator it = mTrackedJobs.listIterator(mTrackedJobs.size()); - while (it.hasPrevious()) { - JobStatus ts = it.previous(); - if (ts.getLatestRunTimeElapsed() < job.getLatestRunTimeElapsed()) { - // Insert - isInsert = true; - break; + public void maybeStartTrackingJob(JobStatus job, JobStatus lastJob) { + synchronized (mLock) { + if (job.hasTimingDelayConstraint() || job.hasDeadlineConstraint()) { + maybeStopTrackingJob(job, false); + boolean isInsert = false; + ListIterator it = mTrackedJobs.listIterator(mTrackedJobs.size()); + while (it.hasPrevious()) { + JobStatus ts = it.previous(); + if (ts.getLatestRunTimeElapsed() < job.getLatestRunTimeElapsed()) { + // Insert + isInsert = true; + break; + } } + if (isInsert) { + it.next(); + } + it.add(job); + maybeUpdateAlarms( + job.hasTimingDelayConstraint() ? job.getEarliestRunTime() : Long.MAX_VALUE, + job.hasDeadlineConstraint() ? job.getLatestRunTimeElapsed() : Long.MAX_VALUE); } - if(isInsert) - { - it.next(); - } - it.add(job); - maybeUpdateAlarms( - job.hasTimingDelayConstraint() ? job.getEarliestRunTime() : Long.MAX_VALUE, - job.hasDeadlineConstraint() ? job.getLatestRunTimeElapsed() : Long.MAX_VALUE); } } @@ -101,10 +103,12 @@ public class TimeController extends StateController { * Really an == comparison should be enough, but why play with fate? We'll do <=. */ @Override - public synchronized void maybeStopTrackingJob(JobStatus job, boolean forUpdate) { - if (mTrackedJobs.remove(job)) { - checkExpiredDelaysAndResetAlarm(); - checkExpiredDeadlinesAndResetAlarm(); + public void maybeStopTrackingJob(JobStatus job, boolean forUpdate) { + synchronized (mLock) { + if (mTrackedJobs.remove(job)) { + checkExpiredDelaysAndResetAlarm(); + checkExpiredDeadlinesAndResetAlarm(); + } } } @@ -131,63 +135,67 @@ public class TimeController extends StateController { * Checks list of jobs for ones that have an expired deadline, sending them to the JobScheduler * if so, removing them from this list, and updating the alarm for the next expiry time. */ - private synchronized void checkExpiredDeadlinesAndResetAlarm() { - long nextExpiryTime = Long.MAX_VALUE; - final long nowElapsedMillis = SystemClock.elapsedRealtime(); + private void checkExpiredDeadlinesAndResetAlarm() { + synchronized (mLock) { + long nextExpiryTime = Long.MAX_VALUE; + final long nowElapsedMillis = SystemClock.elapsedRealtime(); - Iterator it = mTrackedJobs.iterator(); - while (it.hasNext()) { - JobStatus job = it.next(); - if (!job.hasDeadlineConstraint()) { - continue; - } - final long jobDeadline = job.getLatestRunTimeElapsed(); + Iterator it = mTrackedJobs.iterator(); + while (it.hasNext()) { + JobStatus job = it.next(); + if (!job.hasDeadlineConstraint()) { + continue; + } + final long jobDeadline = job.getLatestRunTimeElapsed(); - if (jobDeadline <= nowElapsedMillis) { - job.deadlineConstraintSatisfied.set(true); - mStateChangedListener.onRunJobNow(job); - it.remove(); - } else { // Sorted by expiry time, so take the next one and stop. - nextExpiryTime = jobDeadline; - break; + if (jobDeadline <= nowElapsedMillis) { + job.deadlineConstraintSatisfied.set(true); + mStateChangedListener.onRunJobNow(job); + it.remove(); + } else { // Sorted by expiry time, so take the next one and stop. + nextExpiryTime = jobDeadline; + break; + } } + setDeadlineExpiredAlarm(nextExpiryTime); } - setDeadlineExpiredAlarm(nextExpiryTime); } /** * Handles alarm that notifies us that a job's delay has expired. Iterates through the list of * tracked jobs and marks them as ready as appropriate. */ - private synchronized void checkExpiredDelaysAndResetAlarm() { - final long nowElapsedMillis = SystemClock.elapsedRealtime(); - long nextDelayTime = Long.MAX_VALUE; - boolean ready = false; - Iterator it = mTrackedJobs.iterator(); - while (it.hasNext()) { - final JobStatus job = it.next(); - if (!job.hasTimingDelayConstraint()) { - continue; - } - final long jobDelayTime = job.getEarliestRunTime(); - if (jobDelayTime <= nowElapsedMillis) { - job.timeDelayConstraintSatisfied.set(true); - if (canStopTrackingJob(job)) { - it.remove(); + private void checkExpiredDelaysAndResetAlarm() { + synchronized (mLock) { + final long nowElapsedMillis = SystemClock.elapsedRealtime(); + long nextDelayTime = Long.MAX_VALUE; + boolean ready = false; + Iterator it = mTrackedJobs.iterator(); + while (it.hasNext()) { + final JobStatus job = it.next(); + if (!job.hasTimingDelayConstraint()) { + continue; } - if (job.isReady()) { - ready = true; - } - } else { // Keep going through list to get next delay time. - if (nextDelayTime > jobDelayTime) { - nextDelayTime = jobDelayTime; + final long jobDelayTime = job.getEarliestRunTime(); + if (jobDelayTime <= nowElapsedMillis) { + job.timeDelayConstraintSatisfied.set(true); + if (canStopTrackingJob(job)) { + it.remove(); + } + if (job.isReady()) { + ready = true; + } + } else { // Keep going through list to get next delay time. + if (nextDelayTime > jobDelayTime) { + nextDelayTime = jobDelayTime; + } } } + if (ready) { + mStateChangedListener.onControllerStateChanged(); + } + setDelayExpiredAlarm(nextDelayTime); } - if (ready) { - mStateChangedListener.onControllerStateChanged(); - } - setDelayExpiredAlarm(nextDelayTime); } private void maybeUpdateAlarms(long delayExpiredElapsed, long deadlineExpiredElapsed) { diff --git a/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java b/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java index 577c3a1af1957..fbcd156c99759 100644 --- a/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java +++ b/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java @@ -58,7 +58,7 @@ public class JobStoreTest extends AndroidTestCase { .setMinimumLatency(runFromMillis) .setPersisted(true) .build(); - final JobStatus ts = new JobStatus(task, SOME_UID, null, -1); + final JobStatus ts = new JobStatus(this, task, SOME_UID, null, -1); mTaskStoreUnderTest.add(ts); Thread.sleep(IO_WAIT); // Manually load tasks from xml file. @@ -91,8 +91,8 @@ public class JobStoreTest extends AndroidTestCase { .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED) .setPersisted(true) .build(); - final JobStatus taskStatus1 = new JobStatus(task1, SOME_UID, null, -1); - final JobStatus taskStatus2 = new JobStatus(task2, SOME_UID, null, -1); + final JobStatus taskStatus1 = new JobStatus(this, task1, SOME_UID, null, -1); + final JobStatus taskStatus2 = new JobStatus(this, task2, SOME_UID, null, -1); mTaskStoreUnderTest.add(taskStatus1); mTaskStoreUnderTest.add(taskStatus2); Thread.sleep(IO_WAIT); @@ -140,7 +140,7 @@ public class JobStoreTest extends AndroidTestCase { extras.putInt("into", 3); b.setExtras(extras); final JobInfo task = b.build(); - JobStatus taskStatus = new JobStatus(task, SOME_UID, null, -1); + JobStatus taskStatus = new JobStatus(this, task, SOME_UID, null, -1); mTaskStoreUnderTest.add(taskStatus); Thread.sleep(IO_WAIT); @@ -157,7 +157,8 @@ public class JobStoreTest extends AndroidTestCase { .setPeriodic(10000L) .setRequiresCharging(true) .setPersisted(true); - JobStatus taskStatus = new JobStatus(b.build(), SOME_UID, "com.google.android.gms", 0); + JobStatus taskStatus = new JobStatus(this, b.build(), SOME_UID, + "com.google.android.gms", 0); mTaskStoreUnderTest.add(taskStatus); Thread.sleep(IO_WAIT); @@ -178,7 +179,7 @@ public class JobStoreTest extends AndroidTestCase { .setPeriodic(5*60*60*1000, 1*60*60*1000) .setRequiresCharging(true) .setPersisted(true); - JobStatus taskStatus = new JobStatus(b.build(), SOME_UID, null, -1); + JobStatus taskStatus = new JobStatus(this, b.build(), SOME_UID, null, -1); mTaskStoreUnderTest.add(taskStatus); Thread.sleep(IO_WAIT); @@ -203,7 +204,8 @@ public class JobStoreTest extends AndroidTestCase { SystemClock.elapsedRealtime() + (TWO_HOURS * ONE_HOUR) + TWO_HOURS; // > period+flex final long invalidEarlyRuntimeElapsedMillis = invalidLateRuntimeElapsedMillis - TWO_HOURS; // Early is (late - period). - final JobStatus js = new JobStatus(b.build(), SOME_UID, "somePackage", 0 /* sourceUserId */, + final JobStatus js = new JobStatus(this, b.build(), SOME_UID, "somePackage", + 0 /* sourceUserId */, invalidEarlyRuntimeElapsedMillis, invalidLateRuntimeElapsedMillis); mTaskStoreUnderTest.add(js); @@ -229,7 +231,7 @@ public class JobStoreTest extends AndroidTestCase { .setOverrideDeadline(5000) .setPriority(42) .setPersisted(true); - final JobStatus js = new JobStatus(b.build(), SOME_UID, null, -1); + final JobStatus js = new JobStatus(this, b.build(), SOME_UID, null, -1); mTaskStoreUnderTest.add(js); Thread.sleep(IO_WAIT); final ArraySet jobStatusSet = new ArraySet(); @@ -245,12 +247,12 @@ public class JobStoreTest extends AndroidTestCase { JobInfo.Builder b = new Builder(42, mComponent) .setOverrideDeadline(10000) .setPersisted(false); - JobStatus jsNonPersisted = new JobStatus(b.build(), SOME_UID, null, -1); + JobStatus jsNonPersisted = new JobStatus(this, b.build(), SOME_UID, null, -1); mTaskStoreUnderTest.add(jsNonPersisted); b = new Builder(43, mComponent) .setOverrideDeadline(10000) .setPersisted(true); - JobStatus jsPersisted = new JobStatus(b.build(), SOME_UID, null, -1); + JobStatus jsPersisted = new JobStatus(this, b.build(), SOME_UID, null, -1); mTaskStoreUnderTest.add(jsPersisted); Thread.sleep(IO_WAIT); final ArraySet jobStatusSet = new ArraySet();