diff --git a/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java b/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java index 442c13009d8bb..217b8b6bae935 100644 --- a/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java +++ b/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java @@ -17,23 +17,15 @@ package com.android.server.job; import android.annotation.Nullable; -import android.app.job.JobInfo; import android.app.job.JobParameters; import android.util.proto.ProtoOutputStream; -import java.util.List; - /** * JobScheduler local system service interface. * {@hide} Only for use within the system server. */ public interface JobSchedulerInternal { - /** - * Returns a list of pending jobs scheduled by the system service. - */ - List getSystemScheduledPendingJobs(); - /** * Cancel the jobs for a given uid (e.g. when app data is cleared) * diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java index ccad436839aab..5e4d000b0d78f 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -3478,23 +3478,6 @@ public class JobSchedulerService extends com.android.server.SystemService final class LocalService implements JobSchedulerInternal { - /** - * Returns a list of all pending jobs. A running job is not considered pending. Periodic - * jobs are always considered pending. - */ - @Override - public List getSystemScheduledPendingJobs() { - synchronized (mLock) { - final List pendingJobs = new ArrayList(); - mJobs.forEachJob(Process.SYSTEM_UID, (job) -> { - if (job.getJob().isPeriodic() || !mConcurrencyManager.isJobRunningLocked(job)) { - pendingJobs.add(job.getJob()); - } - }); - return pendingJobs; - } - } - @Override public void cancelJobsForUid(int uid, boolean includeProxiedJobs, @JobParameters.StopReason int reason, int internalReasonCode, String debugReason) { diff --git a/core/proto/android/server/syncstorageengine.proto b/core/proto/android/server/syncstorageengine.proto index d3137473b8398..2f35a077c59e6 100644 --- a/core/proto/android/server/syncstorageengine.proto +++ b/core/proto/android/server/syncstorageengine.proto @@ -83,4 +83,6 @@ message SyncStatusProto { } repeated StatusInfo status = 1; + + optional bool is_job_namespace_migrated = 2; } diff --git a/services/core/java/com/android/server/content/SyncManager.java b/services/core/java/com/android/server/content/SyncManager.java index 5b696c2b253b5..ec2e2540a15a7 100644 --- a/services/core/java/com/android/server/content/SyncManager.java +++ b/services/core/java/com/android/server/content/SyncManager.java @@ -206,13 +206,6 @@ public class SyncManager { */ private static final long SYNC_DELAY_ON_CONFLICT = 10*1000; // 10 seconds - /** - * Generate job ids in the range [MIN_SYNC_JOB_ID, MAX_SYNC_JOB_ID) to avoid conflicts with - * other jobs scheduled by the system process. - */ - private static final int MIN_SYNC_JOB_ID = 100000; - private static final int MAX_SYNC_JOB_ID = 110000; - private static final String SYNC_WAKE_LOCK_PREFIX = "*sync*/"; private static final String HANDLE_SYNC_ALARM_WAKE_LOCK = "SyncManagerHandleSyncAlarm"; private static final String SYNC_LOOP_WAKE_LOCK = "SyncLoopWakeLock"; @@ -243,12 +236,11 @@ public class SyncManager { volatile private PowerManager.WakeLock mSyncManagerWakeLock; volatile private boolean mDataConnectionIsConnected = false; - private volatile int mNextJobIdOffset = 0; + private volatile int mNextJobId = 0; private final NotificationManager mNotificationMgr; private final IBatteryStats mBatteryStats; private JobScheduler mJobScheduler; - private JobSchedulerInternal mJobSchedulerInternal; private SyncStorageEngine mSyncStorageEngine; @@ -284,24 +276,19 @@ public class SyncManager { } private int getUnusedJobIdH() { - final int maxNumSyncJobIds = MAX_SYNC_JOB_ID - MIN_SYNC_JOB_ID; - final List pendingJobs = mJobSchedulerInternal.getSystemScheduledPendingJobs(); - for (int i = 0; i < maxNumSyncJobIds; ++i) { - int newJobId = MIN_SYNC_JOB_ID + ((mNextJobIdOffset + i) % maxNumSyncJobIds); - if (!isJobIdInUseLockedH(newJobId, pendingJobs)) { - mNextJobIdOffset = (mNextJobIdOffset + i + 1) % maxNumSyncJobIds; - return newJobId; - } + final List pendingJobs = mJobScheduler.getAllPendingJobs(); + while (isJobIdInUseLockedH(mNextJobId, pendingJobs)) { + // SyncManager jobs are placed in their own namespace. Since there's no chance of + // conflicting with other parts of the system, we can just keep incrementing until + // we find an unused ID. + mNextJobId++; } - // We've used all 10,000 intended job IDs.... We're probably in a world of pain right now :/ - Slog.wtf(TAG, "All " + maxNumSyncJobIds + " possible sync job IDs are taken :/"); - mNextJobIdOffset = (mNextJobIdOffset + 1) % maxNumSyncJobIds; - return MIN_SYNC_JOB_ID + mNextJobIdOffset; + return mNextJobId; } private List getAllPendingSyncs() { verifyJobScheduler(); - List pendingJobs = mJobSchedulerInternal.getSystemScheduledPendingJobs(); + List pendingJobs = mJobScheduler.getAllPendingJobs(); final int numJobs = pendingJobs.size(); final List pendingSyncs = new ArrayList<>(numJobs); for (int i = 0; i < numJobs; ++i) { @@ -309,6 +296,8 @@ public class SyncManager { SyncOperation op = SyncOperation.maybeCreateFromJobExtras(job.getExtras()); if (op != null) { pendingSyncs.add(op); + } else { + Slog.wtf(TAG, "Non-sync job inside of SyncManager's namespace"); } } return pendingSyncs; @@ -494,6 +483,38 @@ public class SyncManager { }); } + /** + * Migrate syncs from the default job namespace to SyncManager's namespace if they haven't been + * migrated already. + */ + private void migrateSyncJobNamespaceIfNeeded() { + if (mSyncStorageEngine.isJobNamespaceMigrated()) { + return; + } + final JobScheduler jobSchedulerDefaultNamespace = + mContext.getSystemService(JobScheduler.class); + final List pendingJobs = jobSchedulerDefaultNamespace.getAllPendingJobs(); + // Wait until we've confirmed that all syncs have been migrated to the new namespace + // before we persist successful migration to our status file. This is done to avoid + // internal consistency issues if the devices reboots right after SyncManager has + // done the migration on its side but before JobScheduler has finished persisting + // the updated jobs to disk. If JobScheduler hasn't persisted the update to disk, + // then nothing that happened afterwards should have been persisted either, so there's + // no concern over activity happening after the migration causing issues. + boolean allSyncsMigrated = true; + for (int i = pendingJobs.size() - 1; i >= 0; --i) { + final JobInfo job = pendingJobs.get(i); + final SyncOperation op = SyncOperation.maybeCreateFromJobExtras(job.getExtras()); + if (op != null) { + // This is a sync. Move it over to SyncManager's namespace. + mJobScheduler.schedule(job); + jobSchedulerDefaultNamespace.cancel(job.getId()); + allSyncsMigrated = false; + } + } + mSyncStorageEngine.setJobNamespaceMigrated(allSyncsMigrated); + } + private synchronized void verifyJobScheduler() { if (mJobScheduler != null) { return; @@ -503,10 +524,12 @@ public class SyncManager { if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.d(TAG, "initializing JobScheduler object."); } - mJobScheduler = (JobScheduler) mContext.getSystemService( - Context.JOB_SCHEDULER_SERVICE); - mJobSchedulerInternal = getJobSchedulerInternal(); - // Get all persisted syncs from JobScheduler + // Use a dedicated namespace to avoid conflicts with other jobs + // scheduled by the system process. + mJobScheduler = mContext.getSystemService(JobScheduler.class) + .forNamespace("SyncManager"); + migrateSyncJobNamespaceIfNeeded(); + // Get all persisted syncs from JobScheduler in the SyncManager namespace. List pendingJobs = mJobScheduler.getAllPendingJobs(); int numPersistedPeriodicSyncs = 0; @@ -522,6 +545,8 @@ public class SyncManager { // shown on the settings activity. mSyncStorageEngine.markPending(op.target, true); } + } else { + Slog.wtf(TAG, "Non-sync job inside of SyncManager namespace"); } } final String summary = "Loaded persisted syncs: " @@ -543,11 +568,6 @@ public class SyncManager { } } - @VisibleForTesting - protected JobSchedulerInternal getJobSchedulerInternal() { - return LocalServices.getService(JobSchedulerInternal.class); - } - /** * @return whether the device most likely has some periodic syncs. */ diff --git a/services/core/java/com/android/server/content/SyncStorageEngine.java b/services/core/java/com/android/server/content/SyncStorageEngine.java index 9c1cf38500f30..9f3302deba130 100644 --- a/services/core/java/com/android/server/content/SyncStorageEngine.java +++ b/services/core/java/com/android/server/content/SyncStorageEngine.java @@ -172,6 +172,8 @@ public class SyncStorageEngine { private volatile boolean mIsClockValid; + private volatile boolean mIsJobNamespaceMigrated; + static { sAuthorityRenames = new HashMap(); sAuthorityRenames.put("contacts", "com.android.contacts"); @@ -836,6 +838,20 @@ public class SyncStorageEngine { reportChange(ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS, target); } + void setJobNamespaceMigrated(boolean migrated) { + if (mIsJobNamespaceMigrated == migrated) { + return; + } + mIsJobNamespaceMigrated = migrated; + // This isn't urgent enough to write synchronously. Post it to the handler thread so + // SyncManager can move on with whatever it was doing. + mHandler.sendEmptyMessageDelayed(MSG_WRITE_STATUS, WRITE_STATUS_DELAY); + } + + boolean isJobNamespaceMigrated() { + return mIsJobNamespaceMigrated; + } + public Pair getBackoff(EndPoint info) { synchronized (mAuthorities) { AuthorityInfo authority = getAuthorityLocked(info, "getBackoff"); @@ -1585,7 +1601,6 @@ public class SyncStorageEngine { } } - /** * Remove an authority associated with a provider. Needs to be a standalone function for * backward compatibility. @@ -2101,6 +2116,10 @@ public class SyncStorageEngine { mSyncStatus.put(status.authorityId, status); } break; + case (int) SyncStatusProto.IS_JOB_NAMESPACE_MIGRATED: + mIsJobNamespaceMigrated = + proto.readBoolean(SyncStatusProto.IS_JOB_NAMESPACE_MIGRATED); + break; case ProtoInputStream.NO_MORE_FIELDS: return; } @@ -2368,6 +2387,9 @@ public class SyncStorageEngine { } proto.end(token); } + + proto.write(SyncStatusProto.IS_JOB_NAMESPACE_MIGRATED, mIsJobNamespaceMigrated); + proto.flush(); } diff --git a/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java b/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java index 0dd60b8779af3..034466383bac2 100644 --- a/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java @@ -40,8 +40,6 @@ import android.test.suitebuilder.annotation.SmallTest; import androidx.test.core.app.ApplicationProvider; -import com.android.server.job.JobSchedulerInternal; - import junit.framework.TestCase; import org.jetbrains.annotations.NotNull; @@ -49,8 +47,6 @@ import org.junit.Before; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import java.util.ArrayList; - /** * Tests for SyncManager. * @@ -69,8 +65,6 @@ public class SyncManagerTest extends TestCase { private UserManager mUserManager; @Mock private AccountManagerInternal mAccountManagerInternal; - @Mock - private JobSchedulerInternal mJobSchedulerInternal; private class SyncManagerWithMockedServices extends SyncManager { @@ -79,11 +73,6 @@ public class SyncManagerTest extends TestCase { return mAccountManagerInternal; } - @Override - protected JobSchedulerInternal getJobSchedulerInternal() { - return mJobSchedulerInternal; - } - private SyncManagerWithMockedServices(Context context, boolean factoryTest) { super(context, factoryTest); } @@ -95,7 +84,6 @@ public class SyncManagerTest extends TestCase { mContext = spy(ApplicationProvider.getApplicationContext()); when(mContext.getSystemService(Context.USER_SERVICE)).thenReturn(mUserManager); doNothing().when(mAccountManagerInternal).addOnAppPermissionChangeListener(any()); - when(mJobSchedulerInternal.getSystemScheduledPendingJobs()).thenReturn(new ArrayList<>()); mSyncManager = spy(new SyncManagerWithMockedServices(mContext, true)); }