From 24bb2dabd9dd7d8bd39fed53f312ae6034e373bb Mon Sep 17 00:00:00 2001 From: Martin Hibdon Date: Thu, 12 Sep 2013 16:04:37 -0700 Subject: [PATCH] Kill the process when an account is deleted. b/10653370 This prevents NPEs if a serviec happens to still be running when an account is deleted. This mirrors a similar pattern in the gmail app. Change-Id: I6fd8ae5ffe41580df0a321ec22535403e3f32eee --- .../emailcommon/provider/HostAuth.java | 5 ++++ .../emailcommon/service/ServiceProxy.java | 2 ++ .../email/provider/AccountReconciler.java | 30 +++++++++++++++++++ .../email/service/EmailServiceUtils.java | 18 +++++++++++ .../android/email/service/Pop3Service.java | 4 +++ 5 files changed, 59 insertions(+) diff --git a/emailcommon/src/com/android/emailcommon/provider/HostAuth.java b/emailcommon/src/com/android/emailcommon/provider/HostAuth.java index 72b6606e3..5e17a5106 100644 --- a/emailcommon/src/com/android/emailcommon/provider/HostAuth.java +++ b/emailcommon/src/com/android/emailcommon/provider/HostAuth.java @@ -427,4 +427,9 @@ public final class HostAuth extends EmailContent implements HostAuthColumns, Par setConnection(protocol, host, port, flags, clientCertAlias); } + @Override + public String toString() { + return "[protocol " + mProtocol + "]"; + } + } diff --git a/emailcommon/src/com/android/emailcommon/service/ServiceProxy.java b/emailcommon/src/com/android/emailcommon/service/ServiceProxy.java index 8e2e751fc..3a83f9c0b 100644 --- a/emailcommon/src/com/android/emailcommon/service/ServiceProxy.java +++ b/emailcommon/src/com/android/emailcommon/service/ServiceProxy.java @@ -42,6 +42,8 @@ import com.android.mail.utils.LogUtils; */ public abstract class ServiceProxy { + public static final String EXTRA_FORCE_SHUTDOWN = "ServiceProxy.FORCE_SHUTDOWN"; + private static final boolean DEBUG_PROXY = false; // DO NOT CHECK THIS IN SET TO TRUE private final String mTag; diff --git a/src/com/android/email/provider/AccountReconciler.java b/src/com/android/email/provider/AccountReconciler.java index e888b78cb..edb2861c7 100644 --- a/src/com/android/email/provider/AccountReconciler.java +++ b/src/com/android/email/provider/AccountReconciler.java @@ -22,12 +22,15 @@ import android.accounts.AuthenticatorException; import android.accounts.OperationCanceledException; import android.content.Context; import android.database.Cursor; +import android.text.TextUtils; import com.android.email.NotificationController; import com.android.email.R; +import com.android.email.service.EmailServiceUtils; import com.android.emailcommon.Logging; import com.android.emailcommon.provider.Account; import com.android.emailcommon.provider.EmailContent; +import com.android.emailcommon.provider.HostAuth; import com.android.emailcommon.provider.Mailbox; import com.android.mail.utils.LogUtils; import com.google.common.collect.ImmutableList; @@ -142,6 +145,10 @@ public class AccountReconciler { final List accountManagerAccounts, final boolean performReconciliation) { boolean needsReconciling = false; + boolean accountDeleted = false; + boolean exchangeAccountDeleted = false; + + LogUtils.d(Logging.LOG_TAG, "reconcileAccountsInternal"); // First, look through our EmailProvider accounts to make sure there's a corresponding // AccountManager account @@ -171,11 +178,20 @@ public class AccountReconciler { LogUtils.d(Logging.LOG_TAG, "Account deleted in AccountManager; deleting from provider: " + providerAccountName); + // See if this is an exchange account + final HostAuth auth = providerAccount.getOrCreateHostAuthRecv(context); + LogUtils.d(Logging.LOG_TAG, "deleted account with hostAuth " + auth); + if (auth != null && TextUtils.equals(auth.mProtocol, + context.getString(R.string.protocol_eas))) { + exchangeAccountDeleted = true; + } context.getContentResolver().delete( EmailProvider.uiUri("uiaccount", providerAccount.mId), null, null); // Cancel all notifications for this account NotificationController.cancelNotifications(context, providerAccount); + accountDeleted = true; + } } } @@ -209,6 +225,20 @@ public class AccountReconciler { } } + // If an account has been deleted, the simplest thing is just to kill our process. + // Otherwise we might have a service running trying to do something for the account + // which has been deleted, which can get NPEs. It's not as clean is it could be, but + // it still works pretty well because there is nowhere in the email app to delete the + // account. You have to go to Settings, so it's not user visible that the Email app + // has been killed. + if (accountDeleted) { + LogUtils.i(Logging.LOG_TAG, "Restarting because account deleted"); + if (exchangeAccountDeleted) { + EmailServiceUtils.killService(context, context.getString(R.string.protocol_eas)); + } + System.exit(-1); + } + return needsReconciling; } } diff --git a/src/com/android/email/service/EmailServiceUtils.java b/src/com/android/email/service/EmailServiceUtils.java index 74e0152a8..473afff3e 100644 --- a/src/com/android/email/service/EmailServiceUtils.java +++ b/src/com/android/email/service/EmailServiceUtils.java @@ -53,6 +53,7 @@ import com.android.emailcommon.service.EmailServiceProxy; import com.android.emailcommon.service.IEmailService; import com.android.emailcommon.service.IEmailServiceCallback; import com.android.emailcommon.service.SearchParams; +import com.android.emailcommon.service.ServiceProxy; import com.android.emailcommon.service.SyncWindow; import com.android.mail.utils.LogUtils; import com.google.common.collect.ImmutableMap; @@ -69,6 +70,23 @@ import java.util.Map; public class EmailServiceUtils { private static Map sServiceMap = null; + /** + * Ask a service to kill its process. This is used when an account is deleted so that + * no background thread that happens to be running will continue, possibly hitting an + * NPE or other error when trying to operate on an account that no longer exists. + * TODO: This is kind of a hack, it's only needed because we fail so badly if an account + * is deleted out from under us while a sync or other operation is in progress. It would + * be a lot cleaner if our background services could handle this without crashing. + */ + public static void killService(Context context, String protocol) { + EmailServiceInfo info = getServiceInfo(context, protocol); + if (info != null && info.intentAction != null) { + final Intent serviceIntent = getServiceIntent(info); + serviceIntent.putExtra(ServiceProxy.EXTRA_FORCE_SHUTDOWN, true); + context.startService(serviceIntent); + } + } + /** * Starts an EmailService by protocol */ diff --git a/src/com/android/email/service/Pop3Service.java b/src/com/android/email/service/Pop3Service.java index 3830bb29f..07f4a2459 100644 --- a/src/com/android/email/service/Pop3Service.java +++ b/src/com/android/email/service/Pop3Service.java @@ -439,6 +439,10 @@ public class Pop3Service extends Service { // Get rid of the temporary attachment resolver.delete(attUri, null, null); + } else { + // TODO: Should we mark this attachment as failed so we don't + // keep trying to download? + LogUtils.e(TAG, "Could not find message for attachment " + uid); } } }