AI 148333: 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 corruption in the string mSenderUri. BUG=1822859 Automated import of CL 148333
This commit is contained in:
Andy Stadler
The Android Open Source Project
parent
843125b98a
commit
5293030ba0
@ -111,52 +111,60 @@ public class Account implements Serializable {
|
|||||||
*/
|
*/
|
||||||
public void refresh(Preferences preferences) {
|
public void refresh(Preferences preferences) {
|
||||||
mPreferences = 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) {
|
||||||
|
|
||||||
mStoreUri = Utility.base64Decode(preferences.mSharedPreferences.getString(mUuid
|
mStoreUri = Utility.base64Decode(preferences.mSharedPreferences.getString(mUuid
|
||||||
+ ".storeUri", null));
|
+ ".storeUri", null));
|
||||||
mLocalStoreUri = preferences.mSharedPreferences.getString(mUuid + ".localStoreUri", null);
|
mLocalStoreUri = preferences.mSharedPreferences.getString(mUuid + ".localStoreUri", null);
|
||||||
|
|
||||||
String senderText = preferences.mSharedPreferences.getString(mUuid + ".senderUri", null);
|
String senderText = preferences.mSharedPreferences.getString(mUuid + ".senderUri", null);
|
||||||
if (senderText == null) {
|
if (senderText == null) {
|
||||||
// Preference ".senderUri" was called ".transportUri" in earlier versions, so we'll
|
// Preference ".senderUri" was called ".transportUri" in earlier versions, so we'll
|
||||||
// do a simple upgrade here when necessary.
|
// do a simple upgrade here when necessary.
|
||||||
senderText = preferences.mSharedPreferences.getString(mUuid + ".transportUri", null);
|
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);
|
||||||
}
|
}
|
||||||
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() {
|
public String getUuid() {
|
||||||
@ -220,110 +228,124 @@ public class Account implements Serializable {
|
|||||||
}
|
}
|
||||||
|
|
||||||
public void delete(Preferences preferences) {
|
public void delete(Preferences preferences) {
|
||||||
String[] uuids = preferences.mSharedPreferences.getString("accountUuids", "").split(",");
|
/**
|
||||||
StringBuffer sb = new StringBuffer();
|
* Note: Until we have resolved the potential for synchronization failures in
|
||||||
for (int i = 0, length = uuids.length; i < length; i++) {
|
* SharedPreferences, we're going to do a global lock around the read and write
|
||||||
if (!uuids[i].equals(mUuid)) {
|
* functions.
|
||||||
if (sb.length() > 0) {
|
*/
|
||||||
sb.append(',');
|
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]);
|
||||||
}
|
}
|
||||||
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) {
|
public void save(Preferences preferences) {
|
||||||
mPreferences = preferences;
|
mPreferences = preferences;
|
||||||
|
|
||||||
if (!preferences.mSharedPreferences.getString("accountUuids", "").contains(mUuid)) {
|
/**
|
||||||
/*
|
* Note: Until we have resolved the potential for synchronization failures in
|
||||||
* When the account is first created we assign it a unique account number. The
|
* SharedPreferences, we're going to do a global lock around the read and write
|
||||||
* account number will be unique to that account for the lifetime of the account.
|
* functions.
|
||||||
* 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
|
synchronized (Account.class) {
|
||||||
* we use the previous number + 1 as the account number. This refills gaps.
|
if (!preferences.mSharedPreferences.getString("accountUuids", "").contains(mUuid)) {
|
||||||
* mAccountNumber starts as -1 on a newly created account. It must be -1 for this
|
/*
|
||||||
* algorithm to work.
|
* 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.
|
||||||
* I bet there is a much smarter way to do this. Anyone like to suggest it?
|
* 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
|
||||||
Account[] accounts = preferences.getAccounts();
|
* we use the previous number + 1 as the account number. This refills gaps.
|
||||||
int[] accountNumbers = new int[accounts.length];
|
* mAccountNumber starts as -1 on a newly created account. It must be -1 for this
|
||||||
for (int i = 0; i < accounts.length; i++) {
|
* algorithm to work.
|
||||||
accountNumbers[i] = accounts[i].getAccountNumber();
|
*
|
||||||
}
|
* I bet there is a much smarter way to do this. Anyone like to suggest it?
|
||||||
Arrays.sort(accountNumbers);
|
*/
|
||||||
for (int accountNumber : accountNumbers) {
|
Account[] accounts = preferences.getAccounts();
|
||||||
if (accountNumber > mAccountNumber + 1) {
|
int[] accountNumbers = new int[accounts.length];
|
||||||
break;
|
for (int i = 0; i < accounts.length; i++) {
|
||||||
|
accountNumbers[i] = accounts[i].getAccountNumber();
|
||||||
}
|
}
|
||||||
mAccountNumber = accountNumber;
|
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();
|
||||||
}
|
}
|
||||||
mAccountNumber++;
|
|
||||||
|
|
||||||
String accountUuids = preferences.mSharedPreferences.getString("accountUuids", "");
|
|
||||||
accountUuids += (accountUuids.length() != 0 ? "," : "") + mUuid;
|
|
||||||
SharedPreferences.Editor editor = preferences.mSharedPreferences.edit();
|
SharedPreferences.Editor editor = preferences.mSharedPreferences.edit();
|
||||||
editor.putString("accountUuids", accountUuids);
|
|
||||||
|
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();
|
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
|
@Override
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user