Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Put syncs in a dedicated job namespace.

Browse Source 
Put all sync jobs in a dedicated SyncManager job namespace so that sync
jobs don't conflict with jobs scheduled by other parts of the system.

Bug: 238764917
Bug: 253649458
Test: atest CtsSyncManagerTestsCases
Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/content
Change-Id: I7190d9d710f83e319bf922d2dfbfdb13b5e77310
This commit is contained in:
Kweku Adams
2022-12-22 20:59:44 +00:00
parent 6cc3ef13dc
commit 5ebdf21a7d
6 changed files with 76 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java
View File

@@ -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<JobInfo> getSystemScheduledPendingJobs();
/**
* Cancel the jobs for a given uid (e.g. when app data is cleared)
*

17
apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
View File

@@ -3474,23 +3474,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<JobInfo> getSystemScheduledPendingJobs() {
synchronized (mLock) {
final List<JobInfo> pendingJobs = new ArrayList<JobInfo>();
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) {

2
core/proto/android/server/syncstorageengine.proto
View File

@@ -83,4 +83,6 @@ message SyncStatusProto {
}
repeated StatusInfo status = 1;
optional bool is_job_namespace_migrated = 2;
}

82
services/core/java/com/android/server/content/SyncManager.java
View File

@@ -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<JobInfo> 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<JobInfo> 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<SyncOperation> getAllPendingSyncs() {
verifyJobScheduler();
List<JobInfo> pendingJobs = mJobSchedulerInternal.getSystemScheduledPendingJobs();
List<JobInfo> pendingJobs = mJobScheduler.getAllPendingJobs();
final int numJobs = pendingJobs.size();
final List<SyncOperation> 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<JobInfo> 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<JobInfo> 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.
*/

24
services/core/java/com/android/server/content/SyncStorageEngine.java
View File

@@ -172,6 +172,8 @@ public class SyncStorageEngine {
private volatile boolean mIsClockValid;
private volatile boolean mIsJobNamespaceMigrated;
static {
sAuthorityRenames = new HashMap<String, String>();
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<Long, Long> 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();
}

12
services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java
View File

@@ -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));
}