Lock SyncQueue when user starting, copy RSC list.

Document SyncQueue locking policy and protect in onUserStarting()
and clearAllBackoffs().  Return copy of ServiceInfo list from
RegisteredServicesCache instead of exposing locking externally.

Bug: 7357776, 7352537
Change-Id: I6a32ca98a355b639d4207a88bde572179beae359

Author: jsharkey

core/java/android/content/SyncManager.java

@@ -21,7 +21,6 @@
 import android.accounts.AccountManager;
 import android.accounts.AccountManagerService;
 import android.app.ActivityManager;
-import android.app.ActivityManagerNative;
 import android.app.AlarmManager;
 import android.app.Notification;
 import android.app.NotificationManager;
@@ -154,7 +153,9 @@ public class SyncManager {
     private AlarmManager mAlarmService = null;
 
     private SyncStorageEngine mSyncStorageEngine;
-    final public SyncQueue mSyncQueue;
+
+    // @GuardedBy("mSyncQueue")
+    private final SyncQueue mSyncQueue;
 
     protected final ArrayList<ActiveSyncContext> mActiveSyncContexts = Lists.newArrayList();
 
@@ -902,7 +903,9 @@ private void onUserStarting(int userId) {
 
         updateRunningAccounts();
 
-        mSyncQueue.addPendingOperations(userId);
+        synchronized (mSyncQueue) {
+            mSyncQueue.addPendingOperations(userId);
+        }
 
         // Schedule sync for any accounts under started user
         final Account[] accounts = AccountManagerService.getSingleton().getAccounts(userId);
@@ -1957,10 +1960,10 @@ private long maybeStartNextSyncLocked() {
             synchronized (mSyncQueue) {
                 if (isLoggable) {
                     Log.v(TAG, "build the operation array, syncQueue size is "
-                        + mSyncQueue.mOperationsMap.size());
+                        + mSyncQueue.getOperations().size());
                 }
-                Iterator<SyncOperation> operationIterator =
-                        mSyncQueue.mOperationsMap.values().iterator();
+                final Iterator<SyncOperation> operationIterator = mSyncQueue.getOperations()
+                        .iterator();
 
                 final ActivityManager activityManager
                         = (ActivityManager) mContext.getSystemService(Context.ACTIVITY_SERVICE);
@@ -2153,7 +2156,7 @@ private long maybeStartNextSyncLocked() {
                     runSyncFinishedOrCanceledLocked(null, toReschedule);
                     scheduleSyncOperation(toReschedule.mSyncOperation);
                 }
-                synchronized (mSyncQueue){
+                synchronized (mSyncQueue) {
                     mSyncQueue.remove(candidate);
                 }
                 dispatchSyncOperation(candidate);

core/java/android/content/SyncQueue.java

@@ -27,11 +27,14 @@
 import com.google.android.collect.Maps;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 
 /**
+ * Queue of pending sync operations. Not inherently thread safe, external
+ * callers are responsible for locking.
  *
  * @hide
  */
@@ -43,7 +46,7 @@ public class SyncQueue {
 
     // A Map of SyncOperations operationKey -> SyncOperation that is designed for
     // quick lookup of an enqueued SyncOperation.
-    public final HashMap<String, SyncOperation> mOperationsMap = Maps.newHashMap();
+    private final HashMap<String, SyncOperation> mOperationsMap = Maps.newHashMap();
 
     public SyncQueue(SyncStorageEngine syncStorageEngine, final SyncAdaptersCache syncAdapters) {
         mSyncStorageEngine = syncStorageEngine;
@@ -198,6 +201,10 @@ public void remove(Account account, int userId, String authority) {
         }
     }
 
+    public Collection<SyncOperation> getOperations() {
+        return mOperationsMap.values();
+    }
+
     public void dump(StringBuilder sb) {
         final long now = SystemClock.elapsedRealtime();
         sb.append("SyncQueue: ").append(mOperationsMap.size()).append(" operation(s)\n");

core/java/android/content/SyncStorageEngine.java

@@ -609,23 +609,25 @@ public void setBackoff(Account account, int userId, String providerName,
     public void clearAllBackoffs(SyncQueue syncQueue) {
         boolean changed = false;
         synchronized (mAuthorities) {
-            for (AccountInfo accountInfo : mAccounts.values()) {
-                for (AuthorityInfo authorityInfo : accountInfo.authorities.values()) {
-                    if (authorityInfo.backoffTime != NOT_IN_BACKOFF_MODE
-                            || authorityInfo.backoffDelay != NOT_IN_BACKOFF_MODE) {
-                        if (Log.isLoggable(TAG, Log.VERBOSE)) {
-                            Log.v(TAG, "clearAllBackoffs:"
-                                    + " authority:" + authorityInfo.authority
-                                    + " account:" + accountInfo.accountAndUser.account.name
-                                    + " user:" + accountInfo.accountAndUser.userId
-                                    + " backoffTime was: " + authorityInfo.backoffTime
-                                    + " backoffDelay was: " + authorityInfo.backoffDelay);
+            synchronized (syncQueue) {
+                for (AccountInfo accountInfo : mAccounts.values()) {
+                    for (AuthorityInfo authorityInfo : accountInfo.authorities.values()) {
+                        if (authorityInfo.backoffTime != NOT_IN_BACKOFF_MODE
+                                || authorityInfo.backoffDelay != NOT_IN_BACKOFF_MODE) {
+                            if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                                Log.v(TAG, "clearAllBackoffs:"
+                                        + " authority:" + authorityInfo.authority
+                                        + " account:" + accountInfo.accountAndUser.account.name
+                                        + " user:" + accountInfo.accountAndUser.userId
+                                        + " backoffTime was: " + authorityInfo.backoffTime
+                                        + " backoffDelay was: " + authorityInfo.backoffDelay);
+                            }
+                            authorityInfo.backoffTime = NOT_IN_BACKOFF_MODE;
+                            authorityInfo.backoffDelay = NOT_IN_BACKOFF_MODE;
+                            syncQueue.onBackoffChanged(accountInfo.accountAndUser.account,
+                                    accountInfo.accountAndUser.userId, authorityInfo.authority, 0);
+                            changed = true;
                         }
-                        authorityInfo.backoffTime = NOT_IN_BACKOFF_MODE;
-                        authorityInfo.backoffDelay = NOT_IN_BACKOFF_MODE;
-                        syncQueue.onBackoffChanged(accountInfo.accountAndUser.account,
-                                accountInfo.accountAndUser.userId, authorityInfo.authority, 0);
-                        changed = true;
                     }
                 }
             }

core/java/android/content/pm/RegisteredServicesCache.java

@@ -264,7 +264,8 @@ public Collection<ServiceInfo<V>> getAllServices(int userId) {
             if (user.services == null) {
                 generateServicesMap(userId);
             }
-            return Collections.unmodifiableCollection(user.services.values());
+            return Collections.unmodifiableCollection(
+                    new ArrayList<ServiceInfo<V>>(user.services.values()));
         }
     }