Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix extra data in cache

Browse Source 
Reverting the revert.
The original CL(commit a666d74d4b) had a 
bug in it. It was calling accountExistsCacheLocked(), while holding 
accounts.cacheLock. The function accountExistsCacheLocked, was in turn
calling into getUserAccounts, which acquires mUsers. And this causes
trouble. mUsers is a a lock on all accounts, and hence calling it after
holding accounts.cachelock is calling for trouble. Since the locks are 
acquired in other order, it causes a potential deadlock issue, which we 
discovered later on.

That bug was fixed by commit 0e592733ec.
We already have useraccount object and should reuse it.

This reverts commit 27d0e1fd66. This 
commit was unneeded as the bug had been fixed by that time.

Change-Id: I5328c31fd485fd2c1c652cd0e7c2c4bded38a5fd
This commit is contained in:
Simranjit Kohli
2016-03-10 18:36:11 +00:00
parent c1b8e88329
commit 858511cd2d
1 changed files with 53 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

89
services/core/java/com/android/server/accounts/AccountManagerService.java
View File

@@ -782,7 +782,12 @@ public class AccountManagerService
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
return readUserDataInternal(accounts, account, key);
synchronized (accounts.cacheLock) {
if (!accountExistsCacheLocked(accounts, account)) {
return null;
}
return readUserDataInternalLocked(accounts, account, key);
}
} finally {
restoreCallingIdentity(identityToken);
}
@@ -1869,44 +1874,57 @@ public class AccountManagerService
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
setUserdataInternal(accounts, account, key, value);
synchronized (accounts.cacheLock) {
if (!accountExistsCacheLocked(accounts, account)) {
return;
}
setUserdataInternalLocked(accounts, account, key, value);
}
} finally {
restoreCallingIdentity(identityToken);
}
}
private void setUserdataInternal(UserAccounts accounts, Account account, String key,
private boolean accountExistsCacheLocked(UserAccounts accounts, Account account) {
if (accounts.accountCache.containsKey(account.type)) {
for (Account acc : accounts.accountCache.get(account.type)) {
if (acc.name.equals(account.name)) {
return true;
}
}
}
return false;
}
private void setUserdataInternalLocked(UserAccounts accounts, Account account, String key,
String value) {
if (account == null || key == null) {
return;
}
synchronized (accounts.cacheLock) {
final SQLiteDatabase db = accounts.openHelper.getWritableDatabaseUserIsUnlocked();
db.beginTransaction();
try {
long accountId = getAccountIdLocked(db, account);
if (accountId < 0) {
final SQLiteDatabase db = accounts.openHelper.getWritableDatabase();
db.beginTransaction();
try {
long accountId = getAccountIdLocked(db, account);
if (accountId < 0) {
return;
}
long extrasId = getExtrasIdLocked(db, accountId, key);
if (extrasId < 0) {
extrasId = insertExtraLocked(db, accountId, key, value);
if (extrasId < 0) {
return;
}
long extrasId = getExtrasIdLocked(db, accountId, key);
if (extrasId < 0 ) {
extrasId = insertExtraLocked(db, accountId, key, value);
if (extrasId < 0) {
return;
}
} else {
ContentValues values = new ContentValues();
values.put(EXTRAS_VALUE, value);
if (1 != db.update(CE_TABLE_EXTRAS, values, EXTRAS_ID + "=" + extrasId, null)) {
return;
}
} else {
ContentValues values = new ContentValues();
values.put(EXTRAS_VALUE, value);
if (1 != db.update(TABLE_EXTRAS, values, EXTRAS_ID + "=" + extrasId, null)) {
return;
}
writeUserDataIntoCacheLocked(accounts, db, account, key, value);
db.setTransactionSuccessful();
} finally {
db.endTransaction();
}
writeUserDataIntoCacheLocked(accounts, db, account, key, value);
db.setTransactionSuccessful();
} finally {
db.endTransaction();
}
}
@@ -5262,17 +5280,16 @@ public class AccountManagerService
}
}
protected String readUserDataInternal(UserAccounts accounts, Account account, String key) {
synchronized (accounts.cacheLock) {
HashMap<String, String> userDataForAccount = accounts.userDataCache.get(account);
if (userDataForAccount == null) {
// need to populate the cache for this account
final SQLiteDatabase db = accounts.openHelper.getReadableDatabaseUserIsUnlocked();
userDataForAccount = readUserDataForAccountFromDatabaseLocked(db, account);
accounts.userDataCache.put(account, userDataForAccount);
}
return userDataForAccount.get(key);
protected String readUserDataInternalLocked(
UserAccounts accounts, Account account, String key) {
HashMap<String, String> userDataForAccount = accounts.userDataCache.get(account);
if (userDataForAccount == null) {
// need to populate the cache for this account
final SQLiteDatabase db = accounts.openHelper.getReadableDatabase();
userDataForAccount = readUserDataForAccountFromDatabaseLocked(db, account);
accounts.userDataCache.put(account, userDataForAccount);
}
return userDataForAccount.get(key);
}
protected HashMap<String, String> readUserDataForAccountFromDatabaseLocked(