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

Commit f093b530 authored by Varun Shah's avatar Varun Shah
Browse files

Add locking around SyncManager accounts handling. 

The running accounts managed within SyncManager were not protected
behind a lock and caused race conditions when adding new accounts.
This CL adds a lock around the accounts array to avoid such race
conditions.

Bug: 165011202
Test: manual (add new account, observe no "account doesn't exist" errors)
Test: SyncManagerTest
Test: CtsSyncManagerTest
Change-Id: I8ce2a6f93a913bb3cd55ca7a1e292414d45834f6
parent 3609d63a

services/core/java/com/android/server/content/SyncManager.java

+54 −47
Original line number Diff line number Diff line
@@ -231,7 +231,7 @@ public class SyncManager { 


    private static final AccountAndUser[] INITIAL_ACCOUNTS_ARRAY = new AccountAndUser[0];



    // TODO: add better locking around mRunningAccounts

    private final Object mAccountsLock = new Object();

    private volatile AccountAndUser[] mRunningAccounts = INITIAL_ACCOUNTS_ARRAY;



    volatile private PowerManager.WakeLock mSyncManagerWakeLock;
@@ -933,6 +933,7 @@ public class SyncManager { 
        }



        AccountAndUser[] accounts = null;

        synchronized (mAccountsLock) {

            if (requestedAccount != null) {

                if (userId != UserHandle.USER_ALL) {

                    accounts = new AccountAndUser[]{new AccountAndUser(requestedAccount, userId)};
@@ -947,6 +948,7 @@ public class SyncManager { 
            } else {

                accounts = mRunningAccounts;

            }

        }



        if (ArrayUtils.isEmpty(accounts)) {

            mLogger.log("scheduleSync: no accounts configured, dropping");
@@ -3228,6 +3230,7 @@ public class SyncManager { 
        }



        private void updateRunningAccountsH(EndPoint syncTargets) {

            synchronized (mAccountsLock) {

                AccountAndUser[] oldAccounts = mRunningAccounts;

                mRunningAccounts = AccountManagerService.getSingleton().getRunningAccounts();

                if (Log.isLoggable(TAG, Log.VERBOSE)) {
@@ -3260,11 +3263,13 @@ public class SyncManager { 
                                Log.d(TAG, "Account " + aau.account

                                        + " added, checking sync restore data");

                            }

                        AccountSyncSettingsBackupHelper.accountAdded(mContext, syncTargets.userId);

                            AccountSyncSettingsBackupHelper.accountAdded(mContext,

                                    syncTargets.userId);

                            break;

                        }

                    }

                }

            }



            // Cancel all jobs from non-existent accounts.

            AccountAndUser[] allAccounts = AccountManagerService.getSingleton().getAllAccounts();
@@ -3442,6 +3447,7 @@ public class SyncManager { 
            final EndPoint target = op.target;



            // Drop the sync if the account of this operation no longer exists.

            synchronized (mAccountsLock) {

                AccountAndUser[] accounts = mRunningAccounts;

                if (!containsAccountAndUser(accounts, target.account, target.userId)) {

                    if (isLoggable) {
@@ -3450,6 +3456,7 @@ public class SyncManager { 
                    logAccountError("SYNC_OP_STATE_INVALID: account doesn't exist.");

                    return SYNC_OP_STATE_INVALID_NO_ACCOUNT;

                }

            }

            // Drop this sync request if it isn't syncable.

            state = computeSyncable(target.account, target.userId, target.provider, true);

            if (state == AuthorityInfo.SYNCABLE_NO_ACCOUNT_ACCESS) {