Prevent MailService from spamming AccountManager during reconcile

* When any Account is modified, MailService gets a content notification and
  runs reconciliation in an AsyncTask.  Reconciliation ends up calling the
  AccountManager, which also runs asynchronously.  The net effect is that,
  especially during unit tests, where we create/destroy accounts rapidly,
  these calls can "back up", ending in a situation in which the worker pool
  for AsyncTask is filled, with a resulting RejectedExecutionException
* We fix this by preventing more than one request for reconciliation to
  be queued at a time
* Added a unit test that thrashes the notification handler

Bug: 2937628
Change-Id: Iaf25806efb46831f31704604360df091752d9525
This commit is contained in:
Marc Blank 2010-10-17 12:54:09 -07:00
parent 07a91c1655
commit eb49659423
2 changed files with 54 additions and 5 deletions

View File

@ -47,6 +47,7 @@ import android.content.SyncStatusObserver;
import android.database.Cursor;
import android.net.ConnectivityManager;
import android.net.Uri;
import android.os.AsyncTask;
import android.os.Bundle;
import android.os.Handler;
import android.os.IBinder;
@ -58,6 +59,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
/**
* Background service for refreshing non-push email accounts.
@ -93,11 +95,17 @@ public class MailService extends Service {
private static final String[] NEW_MESSAGE_COUNT_PROJECTION =
new String[] {AccountColumns.NEW_MESSAGE_COUNT};
// Keep track of the number of times we're calling the reconciler
// If the count is > 1, don't try to run another one (it means that one is running and one is
// already queued).
private static AtomicInteger sReconcilerCount = new AtomicInteger(0);
private static MailService sMailService;
/*package*/ Controller mController;
private final Controller.Result mControllerCallback = new ControllerResults();
private ContentResolver mContentResolver;
private Context mContext;
private AccountsUpdatedListener mAccountsUpdatedListener;
/*package*/ AccountsUpdatedListener mAccountsUpdatedListener;
private Handler mHandler = new Handler();
private int mStartId;
@ -182,10 +190,17 @@ public class MailService extends Service {
context.startService(i);
}
/*package*/ static MailService getMailServiceForTest() {
return sMailService;
}
@Override
public int onStartCommand(Intent intent, int flags, int startId) {
super.onStartCommand(intent, flags, startId);
// Save the service away (for unit tests)
sMailService = this;
// Restore accounts, if it has not happened already
AccountBackupRestore.restoreAccountsIfNeeded(this);
@ -787,12 +802,28 @@ public class MailService extends Service {
* required argument is ignored (we request those of our specific type within the method)
*/
public class AccountsUpdatedListener implements OnAccountsUpdateListener {
public void onAccountsUpdated(android.accounts.Account[] accounts) {
Utility.runAsync(new Runnable() {
@Override
public void run() {
// Only allow one to be queued at a time (and one running)
if (sReconcilerCount.getAndIncrement() > 1) {
sReconcilerCount.decrementAndGet();
return;
}
new AccountReconcilerTask().execute();
}
private class AccountReconcilerTask extends AsyncTask<Void, Void, Void> {
@Override
protected Void doInBackground(Void... params) {
try {
reconcilePopImapAccounts(MailService.this);
}});
} finally {
// Belt & suspenders; most likely, any RuntimeException in this code would
// kill the Email process
sReconcilerCount.decrementAndGet();
}
return null;
}
}
}

View File

@ -19,6 +19,7 @@ package com.android.email.service;
import com.android.email.AccountTestCase;
import com.android.email.Controller;
import com.android.email.Email;
import com.android.email.Utility;
import com.android.email.provider.EmailContent;
import com.android.email.provider.EmailProvider;
import com.android.email.provider.ProviderTestUtils;
@ -99,6 +100,23 @@ public class MailServiceTests extends AccountTestCase {
}
}
/**
* Call onAccountsUpdated as fast as we can from a single thread (simulating them all coming
* from the UI thread); it shouldn't crash
*/
public void testThrashOnAccountsUpdated() {
final MailService mailService = MailService.getMailServiceForTest();
assertNotNull(mailService);
final android.accounts.Account[] accounts = getExchangeAccounts();
Utility.runAsync(new Runnable() {
@Override
public void run() {
for (int i = 0; i < 1000; i++) {
mailService.mAccountsUpdatedListener.onAccountsUpdated(accounts);
}
}});
}
/**
* Note, there is some inherent risk in this test, as it creates *real* accounts in the
* system (it cannot use the mock context with the Account Manager).