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

4 files changed, 38 insertions, 25 deletions

diff --git a/core/java/android/content/SyncManager.java b/core/java/android/content/SyncManager.java
index 1e4ad76f0b37..58df16773f7e 100644
--- a/core/java/android/content/SyncManager.java
+++ b/core/java/android/content/SyncManager.java
@@ -21,7 +21,6 @@ import android.accounts.AccountAndUser;
 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 @@ public class SyncManager {
 
         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 @@ public class SyncManager {
             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 @@ public class SyncManager {
                     runSyncFinishedOrCanceledLocked(null, toReschedule);
                     scheduleSyncOperation(toReschedule.mSyncOperation);
                 }
-                synchronized (mSyncQueue){
+                synchronized (mSyncQueue) {
                     mSyncQueue.remove(candidate);
                 }
                 dispatchSyncOperation(candidate);
diff --git a/core/java/android/content/SyncQueue.java b/core/java/android/content/SyncQueue.java
index 395658c196d4..14bfc5b52271 100644
--- a/core/java/android/content/SyncQueue.java
+++ b/core/java/android/content/SyncQueue.java
@@ -27,11 +27,14 @@ import android.util.Pair;
 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 class SyncQueue {
         }
     }
 
+    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");
diff --git a/core/java/android/content/SyncStorageEngine.java b/core/java/android/content/SyncStorageEngine.java
index de97481775dd..10e7bff785d4 100644
--- a/core/java/android/content/SyncStorageEngine.java
+++ b/core/java/android/content/SyncStorageEngine.java
@@ -609,23 +609,25 @@ public class SyncStorageEngine extends Handler {
     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;
                     }
                 }
             }
diff --git a/core/java/android/content/pm/RegisteredServicesCache.java b/core/java/android/content/pm/RegisteredServicesCache.java
index 0b917860c001..6def4a169703 100644
--- a/core/java/android/content/pm/RegisteredServicesCache.java
+++ b/core/java/android/content/pm/RegisteredServicesCache.java
@@ -264,7 +264,8 @@ public abstract class RegisteredServicesCache<V> {
             if (user.services == null) {
                 generateServicesMap(userId);
             }
-            return Collections.unmodifiableCollection(user.services.values());
+            return Collections.unmodifiableCollection(
+                    new ArrayList<ServiceInfo<V>>(user.services.values()));
         }
     }