Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 53bd2522 authored by Fred Quintana's avatar Fred Quintana
Browse files

- 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
parent 9be54d40

api/current.xml

+33 −0
Original line number Diff line number Diff line
@@ -31740,6 +31740,17 @@ 
 visibility="public"

>

</field>

<field name="SYNC_EXTRAS_DO_NOT_RETRY"

 type="java.lang.String"

 transient="false"

 volatile="false"

 value="&quot;do_not_retry&quot;"

 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="&quot;ignore_backoff&quot;"

 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="&quot;ignore_settings&quot;"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

<field name="SYNC_EXTRAS_INITIALIZE"

 type="java.lang.String"

 transient="false"

core/java/android/accounts/AccountManager.java

+11 −10
Original line number Diff line number Diff line
@@ -837,14 +837,11 @@ public class AccountManager { 
    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 @@ public class AccountManager { 


        private Bundle internalGetResult(Long timeout, TimeUnit unit)

                throws OperationCanceledException, IOException, AuthenticatorException {

            if (!isDone()) {

                ensureNotOnMainThread();

            }

            try {

                if (timeout == null) {

                    return get();
@@ -1075,7 +1074,9 @@ public class AccountManager { 


        private T internalGetResult(Long timeout, TimeUnit unit)

                throws OperationCanceledException, IOException, AuthenticatorException {

            if (!isDone()) {

                ensureNotOnMainThread();

            }

            try {

                if (timeout == null) {

                    return get();

core/java/android/content/ContentResolver.java

+59 −1
Original line number Diff line number Diff line
@@ -42,7 +42,6 @@ import java.io.InputStream; 
import java.io.OutputStream;

import java.util.List;

import java.util.ArrayList;

import java.util.Collection;





/**
@@ -62,7 +61,31 @@ public abstract class ContentResolver { 
     */

    @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 @@ public abstract class ContentResolver { 
     * 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 abstract class ContentResolver { 
     */

    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 @@ public abstract class ContentResolver { 
     * @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) {

core/java/android/content/SyncManager.java

+37 −47
Original line number Diff line number Diff line
@@ -124,7 +124,7 @@ public class SyncManager implements OnAccountsUpdateListener { 


    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 @@ public class SyncManager implements OnAccountsUpdateListener { 
        }

    };



    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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            // 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 @@ public class SyncManager implements OnAccountsUpdateListener { 


        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 @@ public class SyncManager implements OnAccountsUpdateListener { 
                    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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            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 @@ public class SyncManager implements OnAccountsUpdateListener { 
                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 @@ public class SyncManager implements OnAccountsUpdateListener { 


        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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            }

        }



        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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            }



            // 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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            final boolean syncAutomatically =

                    mSyncStorageEngine.getSyncAutomatically(account, authority)

                            && mSyncStorageEngine.getMasterSyncAutomatically();

            if (!(manualSync || (backgroundDataUsageAllowed && syncAutomatically))) {

            if (!(ignoreSettings || (backgroundDataUsageAllowed && syncAutomatically))) {

                return false;

            }
@@ -1525,7 +1529,7 @@ public class SyncManager implements OnAccountsUpdateListener { 
            // 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 @@ public class SyncManager implements OnAccountsUpdateListener { 
                    // 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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            // 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 @@ public class SyncManager implements OnAccountsUpdateListener { 
            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 @@ public class SyncManager implements OnAccountsUpdateListener { 
                    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);

    }

}

core/java/android/content/SyncOperation.java

+3 −0
Original line number Diff line number Diff line
@@ -26,6 +26,9 @@ public class SyncOperation implements Comparable { 
        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);