- change the SyncManager to retry MANUAL syncs that encounter a soft error
- make the sync dump handle the case where there are no accounts
- fix a bug that caused the SyncManager to burn up CPU in the system process
The following was implemented:
scheduler offers:
- settings to disable sync
- retries of certain errors
- backoffs
want a way to control these when scheduling a sync
- "ignore_settings"
- "ignore initial backoff"
- "manual" : ignore settings, ignore initial backoff
- "do not retry"
- need to change the default behavior of not retrying manual syncs to retry regardless
diff --git a/api/current.xml b/api/current.xml
index 77298ed..23ec303 100644
--- a/api/current.xml
+++ b/api/current.xml
@@ -31740,6 +31740,17 @@
visibility="public"
>
</field>
+<field name="SYNC_EXTRAS_DO_NOT_RETRY"
+ type="java.lang.String"
+ transient="false"
+ volatile="false"
+ value=""do_not_retry""
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
<field name="SYNC_EXTRAS_EXPEDITED"
type="java.lang.String"
transient="false"
@@ -31762,6 +31773,28 @@
visibility="public"
>
</field>
+<field name="SYNC_EXTRAS_IGNORE_BACKOFF"
+ type="java.lang.String"
+ transient="false"
+ volatile="false"
+ value=""ignore_backoff""
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
+<field name="SYNC_EXTRAS_IGNORE_SETTINGS"
+ type="java.lang.String"
+ transient="false"
+ volatile="false"
+ value=""ignore_settings""
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
<field name="SYNC_EXTRAS_INITIALIZE"
type="java.lang.String"
transient="false"
diff --git a/core/java/android/accounts/AccountManager.java b/core/java/android/accounts/AccountManager.java
index 19e741a..652f405 100644
--- a/core/java/android/accounts/AccountManager.java
+++ b/core/java/android/accounts/AccountManager.java
@@ -837,14 +837,11 @@
private void ensureNotOnMainThread() {
final Looper looper = Looper.myLooper();
if (looper != null && looper == mContext.getMainLooper()) {
- // We really want to throw an exception here, but GTalkService exercises this
- // path quite a bit and needs some serious rewrite in order to work properly.
- //noinspection ThrowableInstanceNeverThrow
-// Log.e(TAG, "calling this from your main thread can lead to deadlock and/or ANRs",
-// new Exception());
- // TODO remove the log and throw this exception when the callers are fixed
-// throw new IllegalStateException(
-// "calling this from your main thread can lead to deadlock");
+ final IllegalStateException exception = new IllegalStateException(
+ "calling this from your main thread can lead to deadlock");
+ Log.e(TAG, "calling this from your main thread can lead to deadlock and/or ANRs",
+ exception);
+ throw exception;
}
}
@@ -909,7 +906,9 @@
private Bundle internalGetResult(Long timeout, TimeUnit unit)
throws OperationCanceledException, IOException, AuthenticatorException {
- ensureNotOnMainThread();
+ if (!isDone()) {
+ ensureNotOnMainThread();
+ }
try {
if (timeout == null) {
return get();
@@ -1075,7 +1074,9 @@
private T internalGetResult(Long timeout, TimeUnit unit)
throws OperationCanceledException, IOException, AuthenticatorException {
- ensureNotOnMainThread();
+ if (!isDone()) {
+ ensureNotOnMainThread();
+ }
try {
if (timeout == null) {
return get();
diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java
index b5587ed..b33be23 100644
--- a/core/java/android/content/ContentResolver.java
+++ b/core/java/android/content/ContentResolver.java
@@ -42,7 +42,6 @@
import java.io.OutputStream;
import java.util.List;
import java.util.ArrayList;
-import java.util.Collection;
/**
@@ -62,7 +61,31 @@
*/
@Deprecated
public static final String SYNC_EXTRAS_FORCE = "force";
+
+ /**
+ * If this extra is set to true then the sync settings (like getSyncAutomatically())
+ * are ignored by the sync scheduler.
+ */
+ public static final String SYNC_EXTRAS_IGNORE_SETTINGS = "ignore_settings";
+
+ /**
+ * If this extra is set to true then any backoffs for the initial attempt (e.g. due to retries)
+ * are ignored by the sync scheduler. If this request fails and gets rescheduled then the
+ * retries will still honor the backoff.
+ */
+ public static final String SYNC_EXTRAS_IGNORE_BACKOFF = "ignore_backoff";
+
+ /**
+ * If this extra is set to true then the request will not be retried if it fails.
+ */
+ public static final String SYNC_EXTRAS_DO_NOT_RETRY = "do_not_retry";
+
+ /**
+ * Setting this extra is the equivalent of setting both {@link #SYNC_EXTRAS_IGNORE_SETTINGS}
+ * and {@link #SYNC_EXTRAS_IGNORE_BACKOFF}
+ */
public static final String SYNC_EXTRAS_MANUAL = "force";
+
public static final String SYNC_EXTRAS_UPLOAD = "upload";
public static final String SYNC_EXTRAS_OVERRIDE_TOO_MANY_DELETIONS = "deletions_override";
public static final String SYNC_EXTRAS_DISCARD_LOCAL_DELETIONS = "discard_deletions";
@@ -976,15 +999,38 @@
* Although these sync are scheduled at the specified frequency, it may take longer for it to
* actually be started if other syncs are ahead of it in the sync operation queue. This means
* that the actual start time may drift.
+ * <p>
+ * Periodic syncs are not allowed to have any of {@link #SYNC_EXTRAS_DO_NOT_RETRY},
+ * {@link #SYNC_EXTRAS_IGNORE_BACKOFF}, {@link #SYNC_EXTRAS_IGNORE_SETTINGS},
+ * {@link #SYNC_EXTRAS_INITIALIZE}, {@link #SYNC_EXTRAS_FORCE},
+ * {@link #SYNC_EXTRAS_EXPEDITED}, {@link #SYNC_EXTRAS_MANUAL} set to true.
+ * If any are supplied then an {@link IllegalArgumentException} will be thrown.
*
* @param account the account to specify in the sync
* @param authority the provider to specify in the sync request
* @param extras extra parameters to go along with the sync request
* @param pollFrequency how frequently the sync should be performed, in seconds.
+ * @throws IllegalArgumentException if an illegal extra was set or if any of the parameters
+ * are null.
*/
public static void addPeriodicSync(Account account, String authority, Bundle extras,
long pollFrequency) {
validateSyncExtrasBundle(extras);
+ if (account == null) {
+ throw new IllegalArgumentException("account must not be null");
+ }
+ if (authority == null) {
+ throw new IllegalArgumentException("authority must not be null");
+ }
+ if (extras.getBoolean(SYNC_EXTRAS_MANUAL, false)
+ || extras.getBoolean(SYNC_EXTRAS_DO_NOT_RETRY, false)
+ || extras.getBoolean(SYNC_EXTRAS_IGNORE_BACKOFF, false)
+ || extras.getBoolean(SYNC_EXTRAS_IGNORE_SETTINGS, false)
+ || extras.getBoolean(SYNC_EXTRAS_INITIALIZE, false)
+ || extras.getBoolean(SYNC_EXTRAS_FORCE, false)
+ || extras.getBoolean(SYNC_EXTRAS_EXPEDITED, false)) {
+ throw new IllegalArgumentException("illegal extras were set");
+ }
try {
getContentService().addPeriodicSync(account, authority, extras, pollFrequency);
} catch (RemoteException e) {
@@ -1003,6 +1049,12 @@
*/
public static void removePeriodicSync(Account account, String authority, Bundle extras) {
validateSyncExtrasBundle(extras);
+ if (account == null) {
+ throw new IllegalArgumentException("account must not be null");
+ }
+ if (authority == null) {
+ throw new IllegalArgumentException("authority must not be null");
+ }
try {
getContentService().removePeriodicSync(account, authority, extras);
} catch (RemoteException e) {
@@ -1018,6 +1070,12 @@
* @return a list of PeriodicSync objects. This list may be empty but will never be null.
*/
public static List<PeriodicSync> getPeriodicSyncs(Account account, String authority) {
+ if (account == null) {
+ throw new IllegalArgumentException("account must not be null");
+ }
+ if (authority == null) {
+ throw new IllegalArgumentException("authority must not be null");
+ }
try {
return getContentService().getPeriodicSyncs(account, authority);
} catch (RemoteException e) {
diff --git a/core/java/android/content/SyncManager.java b/core/java/android/content/SyncManager.java
index 619c7d5..ebb95e87 100644
--- a/core/java/android/content/SyncManager.java
+++ b/core/java/android/content/SyncManager.java
@@ -124,7 +124,7 @@
private Context mContext;
- private volatile Account[] mAccounts = null;
+ private volatile Account[] mAccounts = INITIAL_ACCOUNTS_ARRAY;
volatile private PowerManager.WakeLock mSyncWakeLock;
volatile private PowerManager.WakeLock mHandleAlarmWakeLock;
@@ -186,9 +186,11 @@
}
};
+ private static final Account[] INITIAL_ACCOUNTS_ARRAY = new Account[0];
+
public void onAccountsUpdated(Account[] accounts) {
// remember if this was the first time this was called after an update
- final boolean justBootedUp = mAccounts == null;
+ final boolean justBootedUp = mAccounts == INITIAL_ACCOUNTS_ARRAY;
mAccounts = accounts;
// if a sync is in progress yet it is no longer in the accounts list,
@@ -486,11 +488,6 @@
Bundle extras, long delay, boolean onlyThoseWithUnkownSyncableState) {
boolean isLoggable = Log.isLoggable(TAG, Log.VERBOSE);
- if (mAccounts == null) {
- Log.e(TAG, "scheduleSync: the accounts aren't known yet, this should never happen");
- return;
- }
-
if (!isSyncEnabled()) {
if (isLoggable) {
Log.v(TAG, "not syncing because sync is disabled");
@@ -515,13 +512,6 @@
// if the accounts aren't configured yet then we can't support an account-less
// sync request
accounts = mAccounts;
- if (accounts == null) {
- // not ready yet
- if (isLoggable) {
- Log.v(TAG, "scheduleSync: no accounts yet, dropping");
- }
- return;
- }
if (accounts.length == 0) {
if (isLoggable) {
Log.v(TAG, "scheduleSync: no accounts configured, dropping");
@@ -532,6 +522,12 @@
final boolean uploadOnly = extras.getBoolean(ContentResolver.SYNC_EXTRAS_UPLOAD, false);
final boolean manualSync = extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false);
+ if (manualSync) {
+ extras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, true);
+ extras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, true);
+ }
+ final boolean ignoreSettings =
+ extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, false);
int source;
if (uploadOnly) {
@@ -590,7 +586,7 @@
final boolean syncAutomatically = masterSyncAutomatically
&& mSyncStorageEngine.getSyncAutomatically(account, authority);
boolean syncAllowed =
- manualSync || (backgroundDataUsageAllowed && syncAutomatically);
+ ignoreSettings || (backgroundDataUsageAllowed && syncAutomatically);
if (!syncAllowed) {
if (isLoggable) {
Log.d(TAG, "scheduleSync: sync of " + account + ", " + authority
@@ -797,21 +793,29 @@
Log.d(TAG, "encountered error(s) during the sync: " + syncResult + ", " + operation);
}
+ operation = new SyncOperation(operation);
+
+ // The SYNC_EXTRAS_IGNORE_BACKOFF only applies to the first attempt to sync a given
+ // request. Retries of the request will always honor the backoff, so clear the
+ // flag in case we retry this request.
+ if (operation.extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, false)) {
+ operation.extras.remove(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF);
+ }
+
// 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.
// If the operation succeeded to some extent then retry immediately.
// If this was a two-way sync then retry soft errors with an exponential backoff.
// If this was an upward sync then schedule a two-way sync immediately.
// Otherwise do not reschedule.
- if (operation.extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false)) {
- Log.d(TAG, "not retrying sync operation because it is a manual sync: "
+ if (operation.extras.getBoolean(ContentResolver.SYNC_EXTRAS_DO_NOT_RETRY, false)) {
+ Log.d(TAG, "not retrying sync operation because SYNC_EXTRAS_DO_NOT_RETRY was specified "
+ operation);
} else if (operation.extras.getBoolean(ContentResolver.SYNC_EXTRAS_UPLOAD, false)) {
- final SyncOperation newSyncOperation = new SyncOperation(operation);
- newSyncOperation.extras.remove(ContentResolver.SYNC_EXTRAS_UPLOAD);
+ operation.extras.remove(ContentResolver.SYNC_EXTRAS_UPLOAD);
Log.d(TAG, "retrying sync operation as a two-way sync because an upload-only sync "
+ "encountered an error: " + operation);
- scheduleSyncOperation(newSyncOperation);
+ scheduleSyncOperation(operation);
} else if (syncResult.tooManyRetries) {
Log.d(TAG, "not retrying sync operation because it retried too many times: "
+ operation);
@@ -820,13 +824,13 @@
Log.d(TAG, "retrying sync operation because even though it had an error "
+ "it achieved some success");
}
- scheduleSyncOperation(new SyncOperation(operation));
+ scheduleSyncOperation(operation);
} else if (syncResult.hasSoftError()) {
if (isLoggable) {
Log.d(TAG, "retrying sync operation because it encountered a soft error: "
+ operation);
}
- scheduleSyncOperation(new SyncOperation(operation));
+ scheduleSyncOperation(operation);
} else {
Log.d(TAG, "not retrying sync operation because the error is a hard error: "
+ operation);
@@ -942,10 +946,10 @@
final Account[] accounts = mAccounts;
pw.print("accounts: ");
- if (accounts != null) {
+ if (accounts != INITIAL_ACCOUNTS_ARRAY) {
pw.println(accounts.length);
} else {
- pw.println("none");
+ pw.println("not known yet");
}
final long now = SystemClock.elapsedRealtime();
pw.print("now: "); pw.print(now);
@@ -1448,7 +1452,7 @@
}
}
- private boolean isSyncAllowed(Account account, String authority, boolean manualSync,
+ private boolean isSyncAllowed(Account account, String authority, boolean ignoreSettings,
boolean backgroundDataUsageAllowed) {
Account[] accounts = mAccounts;
@@ -1458,7 +1462,7 @@
}
// skip the sync if the account of this operation no longer exists
- if (accounts == null || !ArrayUtils.contains(accounts, account)) {
+ if (!ArrayUtils.contains(accounts, account)) {
return false;
}
@@ -1466,7 +1470,7 @@
final boolean syncAutomatically =
mSyncStorageEngine.getSyncAutomatically(account, authority)
&& mSyncStorageEngine.getMasterSyncAutomatically();
- if (!(manualSync || (backgroundDataUsageAllowed && syncAutomatically))) {
+ if (!(ignoreSettings || (backgroundDataUsageAllowed && syncAutomatically))) {
return false;
}
@@ -1525,7 +1529,7 @@
// If the accounts aren't known yet then we aren't ready to run. We will be kicked
// when the account lookup request does complete.
Account[] accounts = mAccounts;
- if (accounts == null) {
+ if (accounts == INITIAL_ACCOUNTS_ARRAY) {
if (isLoggable) {
Log.v(TAG, "runStateIdle: accounts not known, skipping");
}
@@ -1553,8 +1557,9 @@
// from the queue now
mSyncQueue.remove(op);
- if (!isSyncAllowed(op.account, op.authority,
- op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false),
+ final boolean ignoreSettings = op.extras
+ .getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, false);
+ if (!isSyncAllowed(op.account, op.authority, ignoreSettings,
backgroundDataUsageAllowed)) {
continue;
}
@@ -1609,7 +1614,7 @@
Bundle bestExtras = null;
ArrayList<SyncStorageEngine.AuthorityInfo> infos = mSyncStorageEngine.getAuthorities();
for (SyncStorageEngine.AuthorityInfo info : infos) {
- if (!isSyncAllowed(info.account, info.authority, false /* manualSync */,
+ if (!isSyncAllowed(info.account, info.authority, false /* ignoreSettings */,
backgroundDataUsageAllowed)) {
continue;
}
@@ -1881,7 +1886,6 @@
// in each of these cases the sync loop will be kicked, which will cause this
// method to be called again
if (!mDataConnectionIsConnected) return;
- if (mAccounts == null) return;
if (mStorageIsLow) return;
final long now = SystemClock.elapsedRealtime();
@@ -1895,7 +1899,7 @@
if (activeSyncContext == null) {
synchronized (mSyncQueue) {
Pair<SyncOperation, Long> candidate = bestSyncOperationCandidate();
- alarmTime = candidate != null ? candidate.second : 0;
+ alarmTime = candidate != null ? candidate.second : null;
}
} else {
final long notificationTime =
@@ -2040,18 +2044,4 @@
resultMessage, downstreamActivity, upstreamActivity);
}
}
-
- public static long runTimeWithBackoffs(SyncStorageEngine syncStorageEngine,
- Account account, String authority, boolean isManualSync, long runTime) {
- // if this is a manual sync, the run time is unchanged
- // otherwise, the run time is the max of the backoffs and the run time.
- if (isManualSync) {
- return runTime;
- }
-
- Pair<Long, Long> backoff = syncStorageEngine.getBackoff(account, authority);
- long delayUntilTime = syncStorageEngine.getDelayUntilTime(account, authority);
-
- return Math.max(Math.max(runTime, delayUntilTime), backoff != null ? backoff.first : 0);
- }
}
diff --git a/core/java/android/content/SyncOperation.java b/core/java/android/content/SyncOperation.java
index 2d6e833..4599165 100644
--- a/core/java/android/content/SyncOperation.java
+++ b/core/java/android/content/SyncOperation.java
@@ -26,6 +26,9 @@
this.extras = new Bundle(extras);
removeFalseExtra(ContentResolver.SYNC_EXTRAS_UPLOAD);
removeFalseExtra(ContentResolver.SYNC_EXTRAS_MANUAL);
+ removeFalseExtra(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS);
+ removeFalseExtra(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF);
+ removeFalseExtra(ContentResolver.SYNC_EXTRAS_DO_NOT_RETRY);
removeFalseExtra(ContentResolver.SYNC_EXTRAS_DISCARD_LOCAL_DELETIONS);
removeFalseExtra(ContentResolver.SYNC_EXTRAS_EXPEDITED);
removeFalseExtra(ContentResolver.SYNC_EXTRAS_OVERRIDE_TOO_MANY_DELETIONS);
diff --git a/core/java/android/content/SyncQueue.java b/core/java/android/content/SyncQueue.java
index 2eead3a..bb21488 100644
--- a/core/java/android/content/SyncQueue.java
+++ b/core/java/android/content/SyncQueue.java
@@ -113,10 +113,14 @@
SyncOperation best = null;
long bestRunTime = 0;
for (SyncOperation op : mOperationsMap.values()) {
- long opRunTime = SyncManager.runTimeWithBackoffs(mSyncStorageEngine, op.account,
- op.authority,
- op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false),
- op.earliestRunTime);
+ long opRunTime = op.earliestRunTime;
+ if (!op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, false)) {
+ Pair<Long, Long> backoff = mSyncStorageEngine.getBackoff(op.account, op.authority);
+ long delayUntil = mSyncStorageEngine.getDelayUntilTime(op.account, op.authority);
+ opRunTime = Math.max(
+ Math.max(opRunTime, delayUntil),
+ backoff != null ? backoff.first : 0);
+ }
// if the expedited state of both ops are the same then compare their runtime.
// Otherwise the candidate is only better than the current best if the candidate
// is expedited.
diff --git a/test-runner/android/test/SyncBaseInstrumentation.java b/test-runner/android/test/SyncBaseInstrumentation.java
index e8d72d9..7d418f0 100644
--- a/test-runner/android/test/SyncBaseInstrumentation.java
+++ b/test-runner/android/test/SyncBaseInstrumentation.java
@@ -46,7 +46,7 @@
*/
protected void syncProvider(Uri uri, String accountName, String authority) throws Exception {
Bundle extras = new Bundle();
- extras.putBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, true);
+ extras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, true);
Account account = new Account(accountName, "com.google");
ContentResolver.requestSync(account, authority, extras);