drgreen/android_frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

SyncManager sync blow up fix

Browse Source 
Duplicates for periodic syncs would be
created just after boot and when a
periodic sync was delayed. This CL fixes
the behavior.

Bug: 26954967
Change-Id: I2440961629d443a62f96360563d435af9137857b
This commit is contained in:
Shreyas Basarge
2016-02-08 23:52:27 +00:00
parent b64982aaac
commit 1675400a7c
3 changed files with 161 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -96,6 +96,7 @@ import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Random;
import java.util.List;
import java.util.Set;
import java.util.HashSet;
import java.util.Collection;
import java.util.Collections;
@ -177,18 +178,6 @@ public class SyncManager {
*/
private static final int SYNC_MONITOR_PROGRESS_THRESHOLD_BYTES = 10; // 10 bytes
/**
* How long to delay each queued {@link SyncHandler} message that may have occurred before boot
* or befor the device became provisioned.
*/
private static final long PER_SYNC_BOOT_DELAY_MILLIS = 1000L; // 1 second
/**
* The maximum amount of time we're willing to delay syncs out of boot, after device has been
* provisioned, etc.
*/
private static final long MAX_SYNC_BOOT_DELAY_MILLIS = 60000L; // 1 minute
/**
* If a previously scheduled sync becomes ready and we are low on storage, it gets
* pushed back for this amount of time.
@ -242,12 +231,24 @@ public class SyncManager {
private SparseArray<SyncOperation> mScheduledSyncs = new SparseArray<SyncOperation>(32);
private final Random mRand;
private int getUnusedJobId() {
synchronized (mScheduledSyncs) {
int newJobId = mRand.nextInt(Integer.MAX_VALUE);
while (mScheduledSyncs.indexOfKey(newJobId) >= 0) {
newJobId = mRand.nextInt(Integer.MAX_VALUE);
private boolean isJobIdInUseLockedH(int jobId) {
if (mScheduledSyncs.indexOfKey(jobId) >= 0) {
return true;
}
for (ActiveSyncContext asc: mActiveSyncContexts) {
if (asc.mSyncOperation.jobId == jobId) {
return true;
}
}
return false;
}
private int getUnusedJobIdH() {
synchronized (mScheduledSyncs) {
int newJobId;
do {
newJobId = mRand.nextInt(Integer.MAX_VALUE);
} while (isJobIdInUseLockedH(newJobId));
return newJobId;
}
}
@ -425,6 +426,35 @@ public class SyncManager {
}
}
/**
* Cancel all unnecessary jobs. This function will be run once after every boot.
*/
private void cleanupJobs() {
// O(n^2) in number of jobs, so we run this on the background thread.
mSyncHandler.postAtFrontOfQueue(new Runnable() {
@Override
public void run() {
List<SyncOperation> ops = getAllPendingSyncsFromCache();
Set<String> cleanedKeys = new HashSet<String>();
for (SyncOperation opx: ops) {
if (cleanedKeys.contains(opx.key)) {
continue;
}
cleanedKeys.add(opx.key);
for (SyncOperation opy: ops) {
if (opx == opy) {
continue;
}
if (opx.key.equals(opy.key)) {
removeSyncOperationFromCache(opy.jobId);
mJobScheduler.cancel(opy.jobId);
}
}
}
}
});
}
private synchronized void verifyJobScheduler() {
if (mJobScheduler != null) {
return;
@ -449,6 +479,7 @@ public class SyncManager {
}
}
}
cleanupJobs();
}
private JobScheduler getJobScheduler() {
@ -722,7 +753,6 @@ public class SyncManager {
String requestedAuthority, Bundle extras, long beforeRuntimeMillis,
long runtimeMillis, boolean onlyThoseWithUnkownSyncableState) {
final boolean isLoggable = Log.isLoggable(TAG, Log.VERBOSE);
EndPoint ep = new EndPoint(requestedAccount,requestedAuthority, userId);
if (extras == null) {
extras = new Bundle();
}
@ -1197,7 +1227,18 @@ public class SyncManager {
}
// Check if duplicate syncs are pending. If found, keep one with least expected run time.
if (!syncOperation.isReasonPeriodic()) {
if (!syncOperation.isPeriodic) {
// Check currently running syncs
for (ActiveSyncContext asc: mActiveSyncContexts) {
if (asc.mSyncOperation.key.equals(syncOperation.key)) {
if (isLoggable) {
Log.v(TAG, "Duplicate sync is already running. Not scheduling "
+ syncOperation);
}
return;
}
}
int duplicatesCount = 0;
long now = SystemClock.elapsedRealtime();
syncOperation.expectedRuntime = now + minDelay;
@ -1240,14 +1281,16 @@ public class SyncManager {
}
}
syncOperation.jobId = getUnusedJobId();
// Syncs that are re-scheduled shouldn't get a new job id.
if (syncOperation.jobId == SyncOperation.NO_JOB_ID) {
syncOperation.jobId = getUnusedJobIdH();
}
addSyncOperationToCache(syncOperation);
if (isLoggable) {
Slog.v(TAG, "scheduling sync operation " + syncOperation.target.toString());
Slog.v(TAG, "scheduling sync operation " + syncOperation.toString());
}
// This is done to give preference to syncs that are not pushed back.
int priority = syncOperation.findPriority();
final int networkType = syncOperation.isNotAllowedOnMetered() ?
@ -1343,7 +1386,7 @@ public class SyncManager {
Log.d(TAG, "retrying sync operation as a two-way sync because an upload-only sync "
+ "encountered an error: " + operation);
}
scheduleSyncOperationH(operation, 0 /* immediately */);
scheduleSyncOperationH(operation);
} else if (syncResult.tooManyRetries) {
// If this sync aborted because the internal sync loop retried too many times then
// don't reschedule. Otherwise we risk getting into a retry loop.
@ -1357,7 +1400,7 @@ public class SyncManager {
Log.d(TAG, "retrying sync operation because even though it had an error "
+ "it achieved some success");
}
scheduleSyncOperationH(operation, 0 /* immediately */);
scheduleSyncOperationH(operation);
} else if (syncResult.syncAlreadyInProgress) {
if (isLoggable) {
Log.d(TAG, "retrying sync operation that failed because there was already a "
@ -2207,7 +2250,8 @@ public class SyncManager {
synchronized (this) {
if (!mBootCompleted || !mProvisioned) {
if (msg.what == MESSAGE_START_SYNC) {
deferSyncH((SyncOperation) msg.obj, 60*1000 /* 1 minute */);
SyncOperation op = (SyncOperation) msg.obj;
addSyncOperationToCache(op);
}
// Need to copy the message bc looper will recycle it.
Message m = Message.obtain(msg);
@ -2272,21 +2316,24 @@ public class SyncManager {
case MESSAGE_STOP_SYNC:
op = (SyncOperation) msg.obj;
boolean reschedule = msg.arg1 != 0;
boolean applyBackoff = msg.arg2 != 0;
if (isLoggable) {
Slog.v(TAG, "Stop sync received. Reschedule: " + reschedule
+ "Backoff: " + applyBackoff);
}
if (applyBackoff) {
increaseBackoffSetting(op.target);
}
if (reschedule) {
scheduleSyncOperationH(op);
Slog.v(TAG, "Stop sync received.");
}
ActiveSyncContext asc = findActiveSyncContextH(op.jobId);
if (asc != null) {
runSyncFinishedOrCanceledH(null /* no result */, asc);
boolean reschedule = msg.arg1 != 0;
boolean applyBackoff = msg.arg2 != 0;
if (isLoggable) {
Slog.v(TAG, "Stopping sync. Reschedule: " + reschedule
+ "Backoff: " + applyBackoff);
}
if (applyBackoff) {
increaseBackoffSetting(op.target);
}
if (reschedule) {
deferStoppedSyncH(op, 0);
}
}
break;
@ -2414,7 +2461,8 @@ public class SyncManager {
/**
* Defer the specified SyncOperation by rescheduling it on the JobScheduler with some
* delay.
* delay. This is equivalent to a failure. If this is a periodic sync, a delayed one-off
* sync will be scheduled.
*/
private void deferSyncH(SyncOperation op, long delay) {
mSyncJobService.callJobFinished(op.jobId, false);
@ -2426,14 +2474,22 @@ public class SyncManager {
}
}
/* Same as deferSyncH, but assumes that job is no longer running on JobScheduler. */
private void deferStoppedSyncH(SyncOperation op, long delay) {
if (op.isPeriodic) {
scheduleSyncOperationH(op.createOneTimeSyncOperation(), delay);
} else {
removeSyncOperationFromCache(op.jobId);
scheduleSyncOperationH(op, delay);
}
}
/**
* Cancel an active sync and reschedule it on the JobScheduler with some delay.
*/
private void deferActiveSyncH(ActiveSyncContext asc) {
SyncOperation op = asc.mSyncOperation;
mSyncHandler.obtainMessage(MESSAGE_STOP_SYNC, 0 /* no reschedule */,
0 /* no backoff */, op).sendToTarget();
runSyncFinishedOrCanceledH(null, asc);
deferSyncH(op, SYNC_DELAY_ON_CONFLICT);
}
@ -2467,6 +2523,7 @@ public class SyncManager {
// Check for adapter delays.
if (isAdapterDelayed(op.target)) {
deferSyncH(op, 0 /* No minimum delay */);
return;
}
} else {
// Remove SyncOperation entry from mScheduledSyncs cache for non periodic jobs.
@ -2567,10 +2624,10 @@ public class SyncManager {
Slog.v(TAG, "updating period " + syncOperation + " to " + pollFrequencyMillis
+ " and flex to " + flexMillis);
}
removePeriodicSyncInternalH(syncOperation);
syncOperation.periodMillis = pollFrequencyMillis;
syncOperation.flexMillis = flexMillis;
scheduleSyncOperationH(syncOperation);
SyncOperation newOp = new SyncOperation(syncOperation, pollFrequencyMillis,
flexMillis);
newOp.jobId = syncOperation.jobId;
scheduleSyncOperationH(newOp);
}
}
@ -2614,9 +2671,8 @@ public class SyncManager {
SyncOperation op = new SyncOperation(target, syncAdapterInfo.uid,
syncAdapterInfo.componentName.getPackageName(), SyncOperation.REASON_PERIODIC,
SyncStorageEngine.SOURCE_PERIODIC, extras,
syncAdapterInfo.type.allowParallelSyncs(), true, SyncOperation.NO_JOB_ID);
op.periodMillis = pollFrequencyMillis;
op.flexMillis = flexMillis;
syncAdapterInfo.type.allowParallelSyncs(), true, SyncOperation.NO_JOB_ID,
pollFrequencyMillis, flexMillis);
scheduleSyncOperationH(op);
mSyncStorageEngine.reportChange(ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
}
@ -2816,10 +2872,19 @@ public class SyncManager {
* Should be called when a one-off instance of a periodic sync completes successfully.
*/
private void reschedulePeriodicSyncH(SyncOperation syncOperation) {
removeSyncOperationFromCache(syncOperation.sourcePeriodicId);
getJobScheduler().cancel(syncOperation.sourcePeriodicId);
SyncOperation periodic = syncOperation.createPeriodicSyncOperation();
scheduleSyncOperationH(periodic);
// Ensure that the periodic sync wasn't removed.
SyncOperation periodicSync = null;
List<SyncOperation> ops = getAllPendingSyncsFromCache();
for (SyncOperation op: ops) {
if (op.isPeriodic && syncOperation.matchesPeriodicOperation(op)) {
periodicSync = op;
break;
}
}
if (periodicSync == null) {
return;
}
scheduleSyncOperationH(periodicSync);
}
private void runSyncFinishedOrCanceledH(SyncResult syncResult,

93
services/core/java/com/android/server/content/SyncOperation.java
View File

@ -79,9 +79,9 @@ public class SyncOperation {
public final String key;
/** Poll frequency of periodic sync in milliseconds */
public long periodMillis;
public final long periodMillis;
/** Flex time of periodic sync in milliseconds */
public long flexMillis;
public final long flexMillis;
/** Descriptive string key for this operation */
public String wakeLockName;
/**
@ -90,6 +90,9 @@ public class SyncOperation {
*/
public long expectedRuntime;
/** Stores the number of times this sync operation failed and had to be retried. */
int retries;
/** jobId of the JobScheduler job corresponding to this sync */
public int jobId;
@ -103,23 +106,32 @@ public class SyncOperation {
private SyncOperation(SyncStorageEngine.EndPoint info, int owningUid, String owningPackage,
int reason, int source, Bundle extras, boolean allowParallelSyncs) {
this(info, owningUid, owningPackage, reason, source, extras, allowParallelSyncs, false,
NO_JOB_ID);
NO_JOB_ID, 0, 0);
}
public SyncOperation(SyncOperation op, long periodMillis, long flexMillis) {
this(op.target, op.owningUid, op.owningPackage, op.reason, op.syncSource,
new Bundle(op.extras), op.allowParallelSyncs, op.isPeriodic, op.sourcePeriodicId,
periodMillis, flexMillis);
}
public SyncOperation(SyncStorageEngine.EndPoint info, int owningUid, String owningPackage,
int reason, int source, Bundle extras, boolean allowParallelSyncs,
boolean isPeriodic, int sourcePeriodicId) {
boolean isPeriodic, int sourcePeriodicId, long periodMillis,
long flexMillis) {
this.target = info;
this.owningUid = owningUid;
this.owningPackage = owningPackage;
this.reason = reason;
this.syncSource = source;
this.extras = new Bundle(extras);
cleanBundle(this.extras);
this.allowParallelSyncs = allowParallelSyncs;
this.isPeriodic = isPeriodic;
this.sourcePeriodicId = sourcePeriodicId;
this.key = toKey(target, extras);
this.periodMillis = periodMillis;
this.flexMillis = flexMillis;
this.jobId = NO_JOB_ID;
this.key = toKey();
}
/* Get a one off sync operation instance from a periodic sync. */
@ -128,18 +140,8 @@ public class SyncOperation {
return null;
}
SyncOperation op = new SyncOperation(target, owningUid, owningPackage, reason, syncSource,
new Bundle(extras), allowParallelSyncs, false, jobId /* sourcePeriodicId */);
// Copied to help us recreate the periodic sync from this one off sync.
op.periodMillis = periodMillis;
op.flexMillis = flexMillis;
return op;
}
public SyncOperation createPeriodicSyncOperation() {
SyncOperation op = new SyncOperation(target, owningUid, owningPackage, reason, syncSource,
new Bundle(extras), allowParallelSyncs, true, NO_JOB_ID);
op.periodMillis = periodMillis;
op.flexMillis = flexMillis;
new Bundle(extras), allowParallelSyncs, false, jobId /* sourcePeriodicId */,
periodMillis, flexMillis);
return op;
}
@ -224,6 +226,7 @@ public class SyncOperation {
jobInfoExtras.putLong("periodMillis", periodMillis);
jobInfoExtras.putLong("flexMillis", flexMillis);
jobInfoExtras.putLong("expectedRuntime", expectedRuntime);
jobInfoExtras.putInt("retries", retries);
return jobInfoExtras;
}
@ -240,6 +243,7 @@ public class SyncOperation {
int initiatedBy;
Bundle extras;
boolean allowParallelSyncs, isPeriodic;
long periodMillis, flexMillis;
if (!jobExtras.getBoolean("SyncManagerJob", false)) {
return null;
@ -256,6 +260,8 @@ public class SyncOperation {
allowParallelSyncs = jobExtras.getBoolean("allowParallelSyncs", false);
isPeriodic = jobExtras.getBoolean("isPeriodic", false);
initiatedBy = jobExtras.getInt("sourcePeriodicId", NO_JOB_ID);
periodMillis = jobExtras.getLong("periodMillis");
flexMillis = jobExtras.getLong("flexMillis");
extras = new Bundle();
PersistableBundle syncExtras = jobExtras.getPersistableBundle("syncExtras");
@ -277,37 +283,13 @@ public class SyncOperation {
SyncStorageEngine.EndPoint target =
new SyncStorageEngine.EndPoint(account, provider, userId);
SyncOperation op = new SyncOperation(target, owningUid, owningPackage, reason, source,
extras, allowParallelSyncs, isPeriodic, initiatedBy);
extras, allowParallelSyncs, isPeriodic, initiatedBy, periodMillis, flexMillis);
op.jobId = jobExtras.getInt("jobId");
op.periodMillis = jobExtras.getLong("periodMillis");
op.flexMillis = jobExtras.getLong("flexMillis");
op.expectedRuntime = jobExtras.getLong("expectedRuntime");
op.retries = jobExtras.getInt("retries");
return op;
}
/**
* Make sure the bundle attached to this SyncOperation doesn't have unnecessary
* flags set.
* @param bundle to clean.
*/
private void cleanBundle(Bundle bundle) {
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_UPLOAD);
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_MANUAL);
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS);
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF);
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_DO_NOT_RETRY);
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_DISCARD_LOCAL_DELETIONS);
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_EXPEDITED);
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_OVERRIDE_TOO_MANY_DELETIONS);
removeFalseExtra(bundle, ContentResolver.SYNC_EXTRAS_DISALLOW_METERED);
}
private void removeFalseExtra(Bundle bundle, String extraName) {
if (!bundle.getBoolean(extraName, false)) {
bundle.remove(extraName);
}
}
/**
* Determine whether if this sync operation is running, the provided operation would conflict
* with it.
@ -326,6 +308,12 @@ public class SyncOperation {
return reason == REASON_PERIODIC;
}
boolean matchesPeriodicOperation(SyncOperation other) {
return target.matchesSpec(other.target)
&& SyncManager.syncExtrasEquals(extras, other.extras, true)
&& periodMillis == other.periodMillis && flexMillis == other.flexMillis;
}
boolean isDerivedFromFailedPeriodicSync() {
return sourcePeriodicId != NO_JOB_ID;
}
@ -339,15 +327,18 @@ public class SyncOperation {
return 0;
}
static String toKey(SyncStorageEngine.EndPoint info, Bundle extras) {
private String toKey() {
StringBuilder sb = new StringBuilder();
sb.append("provider: ").append(info.provider);
sb.append(" account {name=" + info.account.name
sb.append("provider: ").append(target.provider);
sb.append(" account {name=" + target.account.name
+ ", user="
+ info.userId
+ target.userId
+ ", type="
+ info.account.type
+ target.account.type
+ "}");
sb.append(" isPeriodic: ").append(isPeriodic);
sb.append(" period: ").append(periodMillis);
sb.append(" flex: ").append(flexMillis);
sb.append(" extras: ");
extrasToStringBuilder(extras, sb);
return sb.toString();
@ -360,7 +351,9 @@ public class SyncOperation {
String dump(PackageManager pm, boolean useOneLine) {
StringBuilder sb = new StringBuilder();
sb.append(target.account.name)
sb.append("JobId: ").append(jobId)
.append(", ")
.append(target.account.name)
.append(" u")
.append(target.userId).append(" (")
.append(target.account.type)

10
services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java
View File

@ -145,14 +145,10 @@ public class SyncOperationTest extends AndroidTestCase {
"provider", 0);
Bundle extras = new Bundle();
SyncOperation periodic = new SyncOperation(ep, 0, "package", 0, 0, extras, false, true,
SyncOperation.NO_JOB_ID);
periodic.periodMillis = 60000;
periodic.flexMillis = 10000;
SyncOperation.NO_JOB_ID, 60000, 10000);
SyncOperation oneoff = periodic.createOneTimeSyncOperation();
assertFalse("Conversion to oneoff sync failed.", oneoff.isPeriodic);
SyncOperation recreated = oneoff.createPeriodicSyncOperation();
assertTrue("Conversion to periodic sync failed.", oneoff.isPeriodic);
assertEquals("Period not restored", periodic.periodMillis, recreated.periodMillis);
assertEquals("Flex not restored", periodic.flexMillis, recreated.flexMillis);
assertEquals("Period not restored", periodic.periodMillis, oneoff.periodMillis);
assertEquals("Flex not restored", periodic.flexMillis, oneoff.flexMillis);
}
}