Make sure that all access to service map is lock-protected

Bug: 7013731
Change-Id: I9da734d19597997b34d6079cf013064262a75ec4

1 files changed, 27 insertions, 40 deletions

diff --git a/emailsync/src/com/android/emailsync/SyncManager.java b/emailsync/src/com/android/emailsync/SyncManager.java
index b331501bb..785c825b5 100644
--- a/emailsync/src/com/android/emailsync/SyncManager.java
+++ b/emailsync/src/com/android/emailsync/SyncManager.java
@@ -283,6 +283,12 @@ public abstract class SyncManager extends Service implements Runnable {
      */
     public abstract Stub getCallbackProxy();
 
+    /**
+     * Called when a sync service has started (in case any action is needed). This method must
+     * not perform any long-lived actions (db access, network access, etc)
+     */
+    public abstract void onStartService(Mailbox mailbox);
+
     public class AccountList extends ArrayList<Account> {
         private static final long serialVersionUID = 1L;
 
@@ -1093,7 +1099,7 @@ public abstract class SyncManager extends Service implements Runnable {
         } else if (ssm == null) {
             context.startService(new Intent(context, SyncManager.class));
         } else {
-            final AbstractSyncService service = ssm.mServiceMap.get(id);
+            final AbstractSyncService service = ssm.getRunningService(id);
             if (service != null) {
                 // Handle alerts in a background thread, as we are typically called from a
                 // broadcast receiver, and are therefore running in the UI thread
@@ -1196,8 +1202,8 @@ public abstract class SyncManager extends Service implements Runnable {
      * @param m the Mailbox on which the service will operate
      */
     private void startServiceThread(AbstractSyncService service) {
+        final Mailbox mailbox = service.mMailbox;
         synchronized (sSyncLock) {
-            Mailbox mailbox = service.mMailbox;
             String mailboxName = mailbox.mDisplayName;
             String accountName = service.mAccount.mDisplayName;
             Thread thread = new Thread(service, mailboxName + "[" + accountName + "]");
@@ -1205,36 +1211,8 @@ public abstract class SyncManager extends Service implements Runnable {
             thread.start();
             mServiceMap.put(mailbox.mId, service);
             runAwake(mailbox.mId);
-            if (mailbox.mServerId != null && mailbox.mType != Mailbox.TYPE_EAS_ACCOUNT_MAILBOX) {
-                stopPing(mailbox.mAccountKey);
-            }
-        }
-    }
-
-    /**
-     * Stop any ping in progress for the given account
-     * @param accountId
-     */
-    private void stopPing(long accountId) {
-        // Go through our active mailboxes looking for the right one
-        synchronized (sSyncLock) {
-            AbstractSyncService serviceToReset = null;
-            for (long mailboxId: mServiceMap.keySet()) {
-                Mailbox m = Mailbox.restoreMailboxWithId(this, mailboxId);
-                if (m != null) {
-                    String serverId = m.mServerId;
-                    if (m.mAccountKey == accountId && serverId != null &&
-                          m.mType == Mailbox.TYPE_EAS_ACCOUNT_MAILBOX) {
-                        // Here's our account mailbox; reset him (stopping pings)
-                        serviceToReset = mServiceMap.get(mailboxId);
-                        break;
-                    }
-                }
-            }
-            if (serviceToReset != null) {
-                serviceToReset.reset();
-            }
         }
+        onStartService(mailbox);
     }
 
     private void requestSync(Mailbox m, int reason, Request req) {
@@ -1680,6 +1658,18 @@ public abstract class SyncManager extends Service implements Runnable {
     }
 
     /**
+     * Retrieve a running sync service for the passed-in mailbox id in a threadsafe manner
+     *
+     * @param mailboxId the id of the mailbox whose service is to be found
+     * @return the running service (a subclass of AbstractSyncService) or null if none
+     */
+    public AbstractSyncService getRunningService(long mailboxId) {
+        synchronized(sSyncLock) {
+            return mServiceMap.get(mailboxId);
+        }
+    }
+
+    /**
      * Check whether an Outbox (referenced by a Cursor) has any messages that can be sent
      * @param c the cursor to an Outbox
      * @return true if there is mail to be sent
@@ -1865,10 +1855,7 @@ public abstract class SyncManager extends Service implements Runnable {
         try {
             while (c.moveToNext()) {
                 long mailboxId = c.getLong(Mailbox.CONTENT_ID_COLUMN);
-                AbstractSyncService service = null;
-                synchronized (sSyncLock) {
-                    service = mServiceMap.get(mailboxId);
-                }
+                AbstractSyncService service = getRunningService(mailboxId);
                 if (service == null) {
                     // Get the cached account
                     Account account = getAccountById(c.getInt(Mailbox.CONTENT_ACCOUNT_KEY_COLUMN));
@@ -1990,7 +1977,7 @@ public abstract class SyncManager extends Service implements Runnable {
         Mailbox m = Mailbox.restoreMailboxWithId(ssm, mailboxId);
         if (m == null || !isSyncable(m)) return;
         try {
-            AbstractSyncService service = ssm.mServiceMap.get(mailboxId);
+            AbstractSyncService service = ssm.getRunningService(mailboxId);
             if (service != null) {
                 service.mRequestTime = System.currentTimeMillis() + ms;
                 kick("service request");
@@ -2005,7 +1992,7 @@ public abstract class SyncManager extends Service implements Runnable {
     static public void serviceRequestImmediate(long mailboxId) {
         SyncManager ssm = INSTANCE;
         if (ssm == null) return;
-        AbstractSyncService service = ssm.mServiceMap.get(mailboxId);
+        AbstractSyncService service = ssm.getRunningService(mailboxId);
         if (service != null) {
             service.mRequestTime = System.currentTimeMillis();
             Mailbox m = Mailbox.restoreMailboxWithId(ssm, mailboxId);
@@ -2047,7 +2034,7 @@ public abstract class SyncManager extends Service implements Runnable {
     static public void sendRequest(long mailboxId, Request req) {
         SyncManager ssm = INSTANCE;
         if (ssm == null) return;
-        AbstractSyncService service = ssm.mServiceMap.get(mailboxId);
+        AbstractSyncService service = ssm.getRunningService(mailboxId);
         if (service == null) {
             startManualSync(mailboxId, SYNC_SERVICE_PART_REQUEST, req);
             kick("part request");
@@ -2067,7 +2054,7 @@ public abstract class SyncManager extends Service implements Runnable {
         SyncManager ssm = INSTANCE;
         if (ssm == null) return PING_STATUS_OK;
         // Already syncing...
-        if (ssm.mServiceMap.get(mailboxId) != null) {
+        if (ssm.getRunningService(mailboxId) != null) {
             return PING_STATUS_RUNNING;
         }
         // No errors or a transient error, don't ping...
@@ -2149,7 +2136,7 @@ public abstract class SyncManager extends Service implements Runnable {
     }
 
     private boolean isRunningInServiceThread(long mailboxId) {
-        AbstractSyncService syncService = mServiceMap.get(mailboxId);
+        AbstractSyncService syncService = getRunningService(mailboxId);
         Thread thisThread = Thread.currentThread();
         return syncService != null && syncService.mThread != null &&
             thisThread == syncService.mThread;