From c2887cd81de8867092136e69a19c1e771a35a66a Mon Sep 17 00:00:00 2001 From: Andy Stadler Date: Thu, 21 May 2009 10:39:04 -0700 Subject: [PATCH] AI 149140: Automated g4 rollback of changelist 148333. *** Reason for rollback *** Rollback global lock because bug (now fixed) was not caused by threading/concurrency. *** Original change description *** Evidence from failures, and inspection of source, leads me to believe that SharedPreferences has some non-thread-safe paths. As a quick, brute-force workaround, I'm putting a global lock around our use of it. This is a bit inefficient, but cases of multiple threads writing to it should be very rare. Note, we don't have an explicit test for this (I will think about finding a way to write one), but the evidence of this failure is that after some amount of activity in the Email app, we see corrup ... description truncated by g4 rollback ... Automated import of CL 149140 --- src/com/android/email/Account.java | 294 +++++++++++++---------------- 1 file changed, 136 insertions(+), 158 deletions(-) diff --git a/src/com/android/email/Account.java b/src/com/android/email/Account.java index 243f4e141..cea3a25d6 100644 --- a/src/com/android/email/Account.java +++ b/src/com/android/email/Account.java @@ -111,60 +111,52 @@ public class Account implements Serializable { */ public void refresh(Preferences preferences) { mPreferences = preferences; + + mStoreUri = Utility.base64Decode(preferences.mSharedPreferences.getString(mUuid + + ".storeUri", null)); + mLocalStoreUri = preferences.mSharedPreferences.getString(mUuid + ".localStoreUri", null); - /** - * Note: Until we have resolved the potential for synchronization failures in - * SharedPreferences, we're going to do a global lock around the read and write - * functions. - */ - synchronized (Account.class) { - - mStoreUri = Utility.base64Decode(preferences.mSharedPreferences.getString(mUuid - + ".storeUri", null)); - mLocalStoreUri = preferences.mSharedPreferences.getString(mUuid + ".localStoreUri", null); - - String senderText = preferences.mSharedPreferences.getString(mUuid + ".senderUri", null); - if (senderText == null) { - // Preference ".senderUri" was called ".transportUri" in earlier versions, so we'll - // do a simple upgrade here when necessary. - senderText = preferences.mSharedPreferences.getString(mUuid + ".transportUri", null); - } - mSenderUri = Utility.base64Decode(senderText); - - mDescription = preferences.mSharedPreferences.getString(mUuid + ".description", null); - mName = preferences.mSharedPreferences.getString(mUuid + ".name", mName); - mEmail = preferences.mSharedPreferences.getString(mUuid + ".email", mEmail); - mAutomaticCheckIntervalMinutes = preferences.mSharedPreferences.getInt(mUuid - + ".automaticCheckIntervalMinutes", -1); - mLastAutomaticCheckTime = preferences.mSharedPreferences.getLong(mUuid - + ".lastAutomaticCheckTime", 0); - mNotifyNewMail = preferences.mSharedPreferences.getBoolean(mUuid + ".notifyNewMail", - false); - - // delete policy was incorrectly set on earlier versions, so we'll upgrade it here. - // rule: if IMAP account and policy = 0 ("never"), change policy to 2 ("on delete") - mDeletePolicy = preferences.mSharedPreferences.getInt(mUuid + ".deletePolicy", 0); - if (mDeletePolicy == DELETE_POLICY_NEVER && - mStoreUri != null && mStoreUri.toString().startsWith(Store.STORE_SCHEME_IMAP)) { - mDeletePolicy = DELETE_POLICY_ON_DELETE; - } - - mDraftsFolderName = preferences.mSharedPreferences.getString(mUuid + ".draftsFolderName", - "Drafts"); - mSentFolderName = preferences.mSharedPreferences.getString(mUuid + ".sentFolderName", - "Sent"); - mTrashFolderName = preferences.mSharedPreferences.getString(mUuid + ".trashFolderName", - "Trash"); - mOutboxFolderName = preferences.mSharedPreferences.getString(mUuid + ".outboxFolderName", - "Outbox"); - mAccountNumber = preferences.mSharedPreferences.getInt(mUuid + ".accountNumber", 0); - mVibrate = preferences.mSharedPreferences.getBoolean(mUuid + ".vibrate", false); - mRingtoneUri = preferences.mSharedPreferences.getString(mUuid + ".ringtone", - "content://settings/system/notification_sound"); - - mSyncWindow = preferences.mSharedPreferences.getInt(mUuid + KEY_SYNC_WINDOW, - SYNC_WINDOW_USER); + String senderText = preferences.mSharedPreferences.getString(mUuid + ".senderUri", null); + if (senderText == null) { + // Preference ".senderUri" was called ".transportUri" in earlier versions, so we'll + // do a simple upgrade here when necessary. + senderText = preferences.mSharedPreferences.getString(mUuid + ".transportUri", null); } + mSenderUri = Utility.base64Decode(senderText); + + mDescription = preferences.mSharedPreferences.getString(mUuid + ".description", null); + mName = preferences.mSharedPreferences.getString(mUuid + ".name", mName); + mEmail = preferences.mSharedPreferences.getString(mUuid + ".email", mEmail); + mAutomaticCheckIntervalMinutes = preferences.mSharedPreferences.getInt(mUuid + + ".automaticCheckIntervalMinutes", -1); + mLastAutomaticCheckTime = preferences.mSharedPreferences.getLong(mUuid + + ".lastAutomaticCheckTime", 0); + mNotifyNewMail = preferences.mSharedPreferences.getBoolean(mUuid + ".notifyNewMail", + false); + + // delete policy was incorrectly set on earlier versions, so we'll upgrade it here. + // rule: if IMAP account and policy = 0 ("never"), change policy to 2 ("on delete") + mDeletePolicy = preferences.mSharedPreferences.getInt(mUuid + ".deletePolicy", 0); + if (mDeletePolicy == DELETE_POLICY_NEVER && + mStoreUri != null && mStoreUri.toString().startsWith(Store.STORE_SCHEME_IMAP)) { + mDeletePolicy = DELETE_POLICY_ON_DELETE; + } + + mDraftsFolderName = preferences.mSharedPreferences.getString(mUuid + ".draftsFolderName", + "Drafts"); + mSentFolderName = preferences.mSharedPreferences.getString(mUuid + ".sentFolderName", + "Sent"); + mTrashFolderName = preferences.mSharedPreferences.getString(mUuid + ".trashFolderName", + "Trash"); + mOutboxFolderName = preferences.mSharedPreferences.getString(mUuid + ".outboxFolderName", + "Outbox"); + mAccountNumber = preferences.mSharedPreferences.getInt(mUuid + ".accountNumber", 0); + mVibrate = preferences.mSharedPreferences.getBoolean(mUuid + ".vibrate", false); + mRingtoneUri = preferences.mSharedPreferences.getString(mUuid + ".ringtone", + "content://settings/system/notification_sound"); + + mSyncWindow = preferences.mSharedPreferences.getInt(mUuid + KEY_SYNC_WINDOW, + SYNC_WINDOW_USER); } public String getUuid() { @@ -228,124 +220,110 @@ public class Account implements Serializable { } public void delete(Preferences preferences) { - /** - * Note: Until we have resolved the potential for synchronization failures in - * SharedPreferences, we're going to do a global lock around the read and write - * functions. - */ - synchronized (Account.class) { - String[] uuids = preferences.mSharedPreferences.getString("accountUuids", "").split(","); - StringBuffer sb = new StringBuffer(); - for (int i = 0, length = uuids.length; i < length; i++) { - if (!uuids[i].equals(mUuid)) { - if (sb.length() > 0) { - sb.append(','); - } - sb.append(uuids[i]); + String[] uuids = preferences.mSharedPreferences.getString("accountUuids", "").split(","); + StringBuffer sb = new StringBuffer(); + for (int i = 0, length = uuids.length; i < length; i++) { + if (!uuids[i].equals(mUuid)) { + if (sb.length() > 0) { + sb.append(','); } + sb.append(uuids[i]); } - String accountUuids = sb.toString(); - SharedPreferences.Editor editor = preferences.mSharedPreferences.edit(); - editor.putString("accountUuids", accountUuids); - - editor.remove(mUuid + ".storeUri"); - editor.remove(mUuid + ".localStoreUri"); - editor.remove(mUuid + ".senderUri"); - editor.remove(mUuid + ".description"); - editor.remove(mUuid + ".name"); - editor.remove(mUuid + ".email"); - editor.remove(mUuid + ".automaticCheckIntervalMinutes"); - editor.remove(mUuid + ".lastAutomaticCheckTime"); - editor.remove(mUuid + ".notifyNewMail"); - editor.remove(mUuid + ".deletePolicy"); - editor.remove(mUuid + ".draftsFolderName"); - editor.remove(mUuid + ".sentFolderName"); - editor.remove(mUuid + ".trashFolderName"); - editor.remove(mUuid + ".outboxFolderName"); - editor.remove(mUuid + ".accountNumber"); - editor.remove(mUuid + ".vibrate"); - editor.remove(mUuid + ".ringtone"); - editor.remove(mUuid + KEY_SYNC_WINDOW); - - // also delete any deprecated fields - editor.remove(mUuid + ".transportUri"); - - editor.commit(); } + String accountUuids = sb.toString(); + SharedPreferences.Editor editor = preferences.mSharedPreferences.edit(); + editor.putString("accountUuids", accountUuids); + + editor.remove(mUuid + ".storeUri"); + editor.remove(mUuid + ".localStoreUri"); + editor.remove(mUuid + ".senderUri"); + editor.remove(mUuid + ".description"); + editor.remove(mUuid + ".name"); + editor.remove(mUuid + ".email"); + editor.remove(mUuid + ".automaticCheckIntervalMinutes"); + editor.remove(mUuid + ".lastAutomaticCheckTime"); + editor.remove(mUuid + ".notifyNewMail"); + editor.remove(mUuid + ".deletePolicy"); + editor.remove(mUuid + ".draftsFolderName"); + editor.remove(mUuid + ".sentFolderName"); + editor.remove(mUuid + ".trashFolderName"); + editor.remove(mUuid + ".outboxFolderName"); + editor.remove(mUuid + ".accountNumber"); + editor.remove(mUuid + ".vibrate"); + editor.remove(mUuid + ".ringtone"); + editor.remove(mUuid + KEY_SYNC_WINDOW); + + // also delete any deprecated fields + editor.remove(mUuid + ".transportUri"); + + editor.commit(); } public void save(Preferences preferences) { mPreferences = preferences; - /** - * Note: Until we have resolved the potential for synchronization failures in - * SharedPreferences, we're going to do a global lock around the read and write - * functions. - */ - synchronized (Account.class) { - if (!preferences.mSharedPreferences.getString("accountUuids", "").contains(mUuid)) { - /* - * When the account is first created we assign it a unique account number. The - * account number will be unique to that account for the lifetime of the account. - * So, we get all the existing account numbers, sort them ascending, loop through - * the list and check if the number is greater than 1 + the previous number. If so - * we use the previous number + 1 as the account number. This refills gaps. - * mAccountNumber starts as -1 on a newly created account. It must be -1 for this - * algorithm to work. - * - * I bet there is a much smarter way to do this. Anyone like to suggest it? - */ - Account[] accounts = preferences.getAccounts(); - int[] accountNumbers = new int[accounts.length]; - for (int i = 0; i < accounts.length; i++) { - accountNumbers[i] = accounts[i].getAccountNumber(); - } - Arrays.sort(accountNumbers); - for (int accountNumber : accountNumbers) { - if (accountNumber > mAccountNumber + 1) { - break; - } - mAccountNumber = accountNumber; - } - mAccountNumber++; - - String accountUuids = preferences.mSharedPreferences.getString("accountUuids", ""); - accountUuids += (accountUuids.length() != 0 ? "," : "") + mUuid; - SharedPreferences.Editor editor = preferences.mSharedPreferences.edit(); - editor.putString("accountUuids", accountUuids); - editor.commit(); + if (!preferences.mSharedPreferences.getString("accountUuids", "").contains(mUuid)) { + /* + * When the account is first created we assign it a unique account number. The + * account number will be unique to that account for the lifetime of the account. + * So, we get all the existing account numbers, sort them ascending, loop through + * the list and check if the number is greater than 1 + the previous number. If so + * we use the previous number + 1 as the account number. This refills gaps. + * mAccountNumber starts as -1 on a newly created account. It must be -1 for this + * algorithm to work. + * + * I bet there is a much smarter way to do this. Anyone like to suggest it? + */ + Account[] accounts = preferences.getAccounts(); + int[] accountNumbers = new int[accounts.length]; + for (int i = 0; i < accounts.length; i++) { + accountNumbers[i] = accounts[i].getAccountNumber(); } - + Arrays.sort(accountNumbers); + for (int accountNumber : accountNumbers) { + if (accountNumber > mAccountNumber + 1) { + break; + } + mAccountNumber = accountNumber; + } + mAccountNumber++; + + String accountUuids = preferences.mSharedPreferences.getString("accountUuids", ""); + accountUuids += (accountUuids.length() != 0 ? "," : "") + mUuid; SharedPreferences.Editor editor = preferences.mSharedPreferences.edit(); - - editor.putString(mUuid + ".storeUri", Utility.base64Encode(mStoreUri)); - editor.putString(mUuid + ".localStoreUri", mLocalStoreUri); - editor.putString(mUuid + ".senderUri", Utility.base64Encode(mSenderUri)); - editor.putString(mUuid + ".description", mDescription); - editor.putString(mUuid + ".name", mName); - editor.putString(mUuid + ".email", mEmail); - editor.putInt(mUuid + ".automaticCheckIntervalMinutes", mAutomaticCheckIntervalMinutes); - editor.putLong(mUuid + ".lastAutomaticCheckTime", mLastAutomaticCheckTime); - editor.putBoolean(mUuid + ".notifyNewMail", mNotifyNewMail); - editor.putInt(mUuid + ".deletePolicy", mDeletePolicy); - editor.putString(mUuid + ".draftsFolderName", mDraftsFolderName); - editor.putString(mUuid + ".sentFolderName", mSentFolderName); - editor.putString(mUuid + ".trashFolderName", mTrashFolderName); - editor.putString(mUuid + ".outboxFolderName", mOutboxFolderName); - editor.putInt(mUuid + ".accountNumber", mAccountNumber); - editor.putBoolean(mUuid + ".vibrate", mVibrate); - editor.putString(mUuid + ".ringtone", mRingtoneUri); - editor.putInt(mUuid + KEY_SYNC_WINDOW, mSyncWindow); - - // The following fields are *not* written because they need to be more fine-grained - // and not risk rewriting with old data. - // editor.putString(mUuid + PREF_TAG_STORE_PERSISTENT, mStorePersistent); - - // also delete any deprecated fields - editor.remove(mUuid + ".transportUri"); - + editor.putString("accountUuids", accountUuids); editor.commit(); } + + SharedPreferences.Editor editor = preferences.mSharedPreferences.edit(); + + editor.putString(mUuid + ".storeUri", Utility.base64Encode(mStoreUri)); + editor.putString(mUuid + ".localStoreUri", mLocalStoreUri); + editor.putString(mUuid + ".senderUri", Utility.base64Encode(mSenderUri)); + editor.putString(mUuid + ".description", mDescription); + editor.putString(mUuid + ".name", mName); + editor.putString(mUuid + ".email", mEmail); + editor.putInt(mUuid + ".automaticCheckIntervalMinutes", mAutomaticCheckIntervalMinutes); + editor.putLong(mUuid + ".lastAutomaticCheckTime", mLastAutomaticCheckTime); + editor.putBoolean(mUuid + ".notifyNewMail", mNotifyNewMail); + editor.putInt(mUuid + ".deletePolicy", mDeletePolicy); + editor.putString(mUuid + ".draftsFolderName", mDraftsFolderName); + editor.putString(mUuid + ".sentFolderName", mSentFolderName); + editor.putString(mUuid + ".trashFolderName", mTrashFolderName); + editor.putString(mUuid + ".outboxFolderName", mOutboxFolderName); + editor.putInt(mUuid + ".accountNumber", mAccountNumber); + editor.putBoolean(mUuid + ".vibrate", mVibrate); + editor.putString(mUuid + ".ringtone", mRingtoneUri); + editor.putInt(mUuid + KEY_SYNC_WINDOW, mSyncWindow); + + // The following fields are *not* written because they need to be more fine-grained + // and not risk rewriting with old data. + // editor.putString(mUuid + PREF_TAG_STORE_PERSISTENT, mStorePersistent); + + // also delete any deprecated fields + editor.remove(mUuid + ".transportUri"); + + editor.commit(); } @Override