Bugfix 9373708

JBMR2 runtime restart (system process crash in the sync manager) during setup

The fix is to ensure that all access to SyncStatusInfo and related objects happens
while holding the mAuthority lock or is on a per-thread copy of the objects

Also, includes an unrelated fix for a bug I just noticed in the way
dumpSyncState() prints the periodic sync info

Change-Id: Id9e4dff41029412e133bdabc843d555434d9a12f
(cherry picked from commit 182ff3acbad9850b40d37ad1c23106be6eda8476)

2 files changed, 98 insertions, 63 deletions

diff --git a/services/java/com/android/server/content/SyncManager.java b/services/java/com/android/server/content/SyncManager.java
index 80c7d88..a233d2c 100644
--- a/services/java/com/android/server/content/SyncManager.java
+++ b/services/java/com/android/server/content/SyncManager.java
@@ -75,6 +75,7 @@ import com.android.internal.annotations.GuardedBy;
 import com.android.internal.os.BackgroundThread;
 import com.android.internal.util.IndentingPrintWriter;
 import com.android.server.accounts.AccountManagerService;
+import com.android.server.content.SyncStorageEngine.AuthorityInfo;
 import com.android.server.content.SyncStorageEngine.OnSyncRequestListener;
 import com.google.android.collect.Lists;
 import com.google.android.collect.Maps;
@@ -1248,10 +1249,11 @@ public class SyncManager {
                     continue;
                 }
                 int row = table.getNumRows();
-                SyncStorageEngine.AuthorityInfo settings =
-                        mSyncStorageEngine.getOrCreateAuthority(
+                Pair<AuthorityInfo, SyncStatusInfo> syncAuthoritySyncStatus = 
+                        mSyncStorageEngine.getCopyOfAuthorityWithSyncStatus(
                                 account.account, account.userId, syncAdapterType.type.authority);
-                SyncStatusInfo status = mSyncStorageEngine.getOrCreateSyncStatus(settings);
+                SyncStorageEngine.AuthorityInfo settings = syncAuthoritySyncStatus.first;
+                SyncStatusInfo status = syncAuthoritySyncStatus.second;
 
                 String authority = settings.authority;
                 if (authority.length() > 50) {
@@ -1269,10 +1271,10 @@ public class SyncManager {
 
 
                 for (int i = 0; i < settings.periodicSyncs.size(); i++) {
-                    final Pair<Bundle, Long> pair = settings.periodicSyncs.get(0);
+                    final Pair<Bundle, Long> pair = settings.periodicSyncs.get(i);
                     final String period = String.valueOf(pair.second);
                     final String extras = pair.first.size() > 0 ? pair.first.toString() : "";
-                    final String next = formatTime(status.getPeriodicSyncTime(0)
+                    final String next = formatTime(status.getPeriodicSyncTime(i)
                             + pair.second * 1000);
                     table.set(row + i * 2, 12, period + extras);
                     table.set(row + i * 2 + 1, 12, next);
@@ -1942,30 +1944,36 @@ public class SyncManager {
 
             final long nowAbsolute = System.currentTimeMillis();
             final long shiftedNowAbsolute = (0 < nowAbsolute - mSyncRandomOffsetMillis)
-                                               ? (nowAbsolute  - mSyncRandomOffsetMillis) : 0;
-
-            ArrayList<SyncStorageEngine.AuthorityInfo> infos = mSyncStorageEngine.getAuthorities();
-            for (SyncStorageEngine.AuthorityInfo info : infos) {
-                // skip the sync if the account of this operation no longer exists
-                if (!containsAccountAndUser(accounts, info.account, info.userId)) {
+                    ? (nowAbsolute - mSyncRandomOffsetMillis) : 0;
+
+            ArrayList<Pair<AuthorityInfo, SyncStatusInfo>> infos = mSyncStorageEngine
+                    .getCopyOfAllAuthoritiesWithSyncStatus();
+            for (Pair<AuthorityInfo, SyncStatusInfo> info : infos) {
+                final AuthorityInfo authorityInfo = info.first;
+                final SyncStatusInfo status = info.second;
+                // skip the sync if the account of this operation no longer
+                // exists
+                if (!containsAccountAndUser(
+                        accounts, authorityInfo.account, authorityInfo.userId)) {
                     continue;
                 }
 
-                if (!mSyncStorageEngine.getMasterSyncAutomatically(info.userId)
-                        || !mSyncStorageEngine.getSyncAutomatically(info.account, info.userId,
-                                info.authority)) {
+                if (!mSyncStorageEngine.getMasterSyncAutomatically(authorityInfo.userId)
+                        || !mSyncStorageEngine.getSyncAutomatically(
+                                authorityInfo.account, authorityInfo.userId,
+                                authorityInfo.authority)) {
                     continue;
                 }
 
-                if (getIsSyncable(info.account, info.userId, info.authority)
+                if (getIsSyncable(
+                        authorityInfo.account, authorityInfo.userId, authorityInfo.authority)
                         == 0) {
                     continue;
                 }
 
-                SyncStatusInfo status = mSyncStorageEngine.getOrCreateSyncStatus(info);
-                for (int i = 0, N = info.periodicSyncs.size(); i < N; i++) {
-                    final Bundle extras = info.periodicSyncs.get(i).first;
-                    final Long periodInMillis = info.periodicSyncs.get(i).second * 1000;
+                for (int i = 0, N = authorityInfo.periodicSyncs.size(); i < N; i++) {
+                    final Bundle extras = authorityInfo.periodicSyncs.get(i).first;
+                    final Long periodInMillis = authorityInfo.periodicSyncs.get(i).second * 1000;
                     // Skip if the period is invalid
                     if (periodInMillis <= 0) {
                         continue;
@@ -1974,51 +1982,56 @@ public class SyncManager {
                     final long lastPollTimeAbsolute = status.getPeriodicSyncTime(i);
 
                     long remainingMillis
-                            = periodInMillis - (shiftedNowAbsolute % periodInMillis);
+                    = periodInMillis - (shiftedNowAbsolute % periodInMillis);
 
                     /*
-                     * Sync scheduling strategy:
-                     *    Set the next periodic sync based on a random offset (in seconds).
-                     *
-                     *    Also sync right now if any of the following cases hold
-                     *    and mark it as having been scheduled
-                     *
-                     * Case 1:  This sync is ready to run now.
-                     * Case 2:  If the lastPollTimeAbsolute is in the future,
-                     *          sync now and reinitialize. This can happen for
-                     *          example if the user changed the time, synced and
-                     *          changed back.
-                     * Case 3:  If we failed to sync at the last scheduled time
+                     * Sync scheduling strategy: Set the next periodic sync
+                     * based on a random offset (in seconds). Also sync right
+                     * now if any of the following cases hold and mark it as
+                     * having been scheduled
+                     * Case 1: This sync is ready to run
+                     * now.
+                     * Case 2: If the lastPollTimeAbsolute is in the
+                     * future, sync now and reinitialize. This can happen for
+                     * example if the user changed the time, synced and changed
+                     * back.
+                     * Case 3: If we failed to sync at the last scheduled
+                     * time
                      */
-                    if (remainingMillis == periodInMillis  // Case 1
+                    if (remainingMillis == periodInMillis // Case 1
                             || lastPollTimeAbsolute > nowAbsolute // Case 2
                             || (nowAbsolute - lastPollTimeAbsolute
                                     >= periodInMillis)) { // Case 3
                         // Sync now
                         final Pair<Long, Long> backoff = mSyncStorageEngine.getBackoff(
-                                info.account, info.userId, info.authority);
+                                authorityInfo.account, authorityInfo.userId,
+                                authorityInfo.authority);
                         final RegisteredServicesCache.ServiceInfo<SyncAdapterType> syncAdapterInfo;
                         syncAdapterInfo = mSyncAdapters.getServiceInfo(
-                                SyncAdapterType.newKey(info.authority, info.account.type),
-                                info.userId);
+                                SyncAdapterType.newKey(
+                                        authorityInfo.authority, authorityInfo.account.type),
+                                authorityInfo.userId);
                         if (syncAdapterInfo == null) {
                             continue;
                         }
                         scheduleSyncOperation(
-                                new SyncOperation(info.account, info.userId,
+                                new SyncOperation(authorityInfo.account, authorityInfo.userId,
                                         SyncOperation.REASON_PERIODIC,
                                         SyncStorageEngine.SOURCE_PERIODIC,
-                                        info.authority, extras, 0 /* delay */,
-                                        backoff != null ? backoff.first : 0,
+                                        authorityInfo.authority, extras, 0 /* delay */,
+                                                backoff != null ? backoff.first : 0,
                                         mSyncStorageEngine.getDelayUntilTime(
-                                                info.account, info.userId, info.authority),
+                                                authorityInfo.account, authorityInfo.userId,
+                                                authorityInfo.authority),
                                         syncAdapterInfo.type.allowParallelSyncs()));
-                        status.setPeriodicSyncTime(i, nowAbsolute);
+                        mSyncStorageEngine.setPeriodicSyncTime(authorityInfo.ident,
+                                authorityInfo.periodicSyncs.get(i), nowAbsolute);
                     }
                     // Compute when this periodic sync should next run
                     final long nextPollTimeAbsolute = nowAbsolute + remainingMillis;
 
-                    // remember this time if it is earlier than earliestFuturePollTime
+                    // remember this time if it is earlier than
+                    // earliestFuturePollTime
                     if (nextPollTimeAbsolute < earliestFuturePollTime) {
                         earliestFuturePollTime = nextPollTimeAbsolute;
                     }
@@ -2032,8 +2045,8 @@ public class SyncManager {
             // convert absolute time to elapsed time
             return SystemClock.elapsedRealtime()
                     + ((earliestFuturePollTime < nowAbsolute)
-                      ? 0
-                      : (earliestFuturePollTime - nowAbsolute));
+                            ? 0
+                            : (earliestFuturePollTime - nowAbsolute));
         }
 
         private long maybeStartNextSyncLocked() {
diff --git a/services/java/com/android/server/content/SyncStorageEngine.java b/services/java/com/android/server/content/SyncStorageEngine.java
index 5b8d26f..863def3 100644
--- a/services/java/com/android/server/content/SyncStorageEngine.java
+++ b/services/java/com/android/server/content/SyncStorageEngine.java
@@ -44,6 +44,7 @@ import android.util.Xml;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.ArrayUtils;
 import com.android.internal.util.FastXmlSerializer;
+import com.android.server.content.SyncStorageEngine.AuthorityInfo;
 
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
@@ -813,14 +814,6 @@ public class SyncStorageEngine extends Handler {
         }
     }
 
-    public AuthorityInfo getOrCreateAuthority(Account account, int userId, String authority) {
-        synchronized (mAuthorities) {
-            return getOrCreateAuthorityLocked(account, userId, authority,
-                    -1 /* assign a new identifier if creating a new authority */,
-                    true /* write to storage if this results in a change */);
-        }
-    }
-
     public void removeAuthority(Account account, int userId, String authority) {
         synchronized (mAuthorities) {
             removeAuthorityLocked(account, userId, authority, true /* doWrite */);
@@ -1238,17 +1231,27 @@ public class SyncStorageEngine extends Handler {
     }
 
     /**
-     * Return an array of the current authorities. Note
-     * that the objects inside the array are the real, live objects,
-     * so be careful what you do with them.
+     * Return a copy of the specified authority with the corresponding sync status
      */
-    public ArrayList<AuthorityInfo> getAuthorities() {
+    public Pair<AuthorityInfo, SyncStatusInfo> getCopyOfAuthorityWithSyncStatus(
+            Account account, int userId, String authority) {
         synchronized (mAuthorities) {
-            final int N = mAuthorities.size();
-            ArrayList<AuthorityInfo> infos = new ArrayList<AuthorityInfo>(N);
-            for (int i=0; i<N; i++) {
-                // Make deep copy because AuthorityInfo syncs are liable to change.
-                infos.add(new AuthorityInfo(mAuthorities.valueAt(i)));
+            AuthorityInfo authorityInfo = getOrCreateAuthorityLocked(account, userId, authority,
+                    -1 /* assign a new identifier if creating a new authority */,
+                    true /* write to storage if this results in a change */);
+            return createCopyPairOfAuthorityWithSyncStatusLocked(authorityInfo);
+        }
+    }
+
+    /**
+     * Return a copy of all authorities with their corresponding sync status
+     */
+    public ArrayList<Pair<AuthorityInfo, SyncStatusInfo>> getCopyOfAllAuthoritiesWithSyncStatus() {
+        synchronized (mAuthorities) {
+            ArrayList<Pair<AuthorityInfo, SyncStatusInfo>> infos =
+                    new ArrayList<Pair<AuthorityInfo, SyncStatusInfo>>(mAuthorities.size());
+            for (int i = 0; i < mAuthorities.size(); i++) {
+                infos.add(createCopyPairOfAuthorityWithSyncStatusLocked(mAuthorities.valueAt(i)));
             }
             return infos;
         }
@@ -1337,6 +1340,12 @@ public class SyncStorageEngine extends Handler {
         }
     }
 
+    private Pair<AuthorityInfo, SyncStatusInfo> createCopyPairOfAuthorityWithSyncStatusLocked(
+            AuthorityInfo authorityInfo) {
+        SyncStatusInfo syncStatusInfo = getOrCreateSyncStatusLocked(authorityInfo.ident);
+        return Pair.create(new AuthorityInfo(authorityInfo), new SyncStatusInfo(syncStatusInfo));
+    }
+
     private int getCurrentDayLocked() {
         mCal.setTimeInMillis(System.currentTimeMillis());
         final int dayOfYear = mCal.get(Calendar.DAY_OF_YEAR);
@@ -1427,9 +1436,22 @@ public class SyncStorageEngine extends Handler {
         }
     }
 
-    public SyncStatusInfo getOrCreateSyncStatus(AuthorityInfo authority) {
+    /**
+     * Updates (in a synchronized way) the periodic sync time of the specified
+     * authority id and target periodic sync
+     */
+    public void setPeriodicSyncTime(
+            int authorityId, Pair<Bundle, Long> targetPeriodicSync, long when) {
         synchronized (mAuthorities) {
-            return getOrCreateSyncStatusLocked(authority.ident);
+            final AuthorityInfo authority = mAuthorities.get(authorityId);
+            for (int i = 0; i < authority.periodicSyncs.size(); i++) {
+                Pair<Bundle, Long> periodicSync = authority.periodicSyncs.get(i);
+                if (periodicSync.first == targetPeriodicSync.first
+                        && periodicSync.second == targetPeriodicSync.second) {
+                    mSyncStatus.get(authorityId).setPeriodicSyncTime(i, when);
+                    break;
+                }
+            }
         }
     }