From a706e2fd0059b1bb86c487722dbc9fc0fda9c980 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 16 Oct 2012 12:02:42 -0700 Subject: [PATCH] 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 --- core/java/android/content/SyncManager.java | 17 ++++++---- core/java/android/content/SyncQueue.java | 9 ++++- .../android/content/SyncStorageEngine.java | 34 ++++++++++--------- .../content/pm/RegisteredServicesCache.java | 3 +- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/core/java/android/content/SyncManager.java b/core/java/android/content/SyncManager.java index 1e4ad76f0b37e..58df16773f7e4 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 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 operationIterator = - mSyncQueue.mOperationsMap.values().iterator(); + final Iterator 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 395658c196d47..14bfc5b52271c 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 mOperationsMap = Maps.newHashMap(); + private final HashMap mOperationsMap = Maps.newHashMap(); public SyncQueue(SyncStorageEngine syncStorageEngine, final SyncAdaptersCache syncAdapters) { mSyncStorageEngine = syncStorageEngine; @@ -198,6 +201,10 @@ public class SyncQueue { } } + public Collection 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 de97481775dd6..10e7bff785d4d 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 0b917860c0011..6def4a1697030 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 { if (user.services == null) { generateServicesMap(userId); } - return Collections.unmodifiableCollection(user.services.values()); + return Collections.unmodifiableCollection( + new ArrayList>(user.services.values())); } }