Fix deadlock occurring on account add in SyncManager.
deadlock caused by autosync from changing sync settings.
Some minor refactoring of the function that caused the deadlock
Bug: 10666901
Change-Id: I7cf901b1954e59dbb0bc71a5de23117353b460b1
diff --git a/services/java/com/android/server/content/ContentService.java b/services/java/com/android/server/content/ContentService.java
index ab20461..e37505f 100644
--- a/services/java/com/android/server/content/ContentService.java
+++ b/services/java/com/android/server/content/ContentService.java
@@ -630,16 +630,16 @@
public void setServiceActive(ComponentName cname, boolean active) {
mContext.enforceCallingOrSelfPermission(Manifest.permission.WRITE_SYNC_SETTINGS,
"no permission to write the sync settings");
- verifySignatureForPackage(Binder.getCallingUid(), cname.getPackageName(), "setIsEnabled");
+ verifySignatureForPackage(Binder.getCallingUid(), cname.getPackageName(),
+ "setServiceActive");
int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
SyncManager syncManager = getSyncManager();
if (syncManager != null) {
- int syncable = active ? 1 : 0;
- syncManager.getSyncStorageEngine().setIsEnabled(
- cname, userId, syncable);
+ syncManager.getSyncStorageEngine().setIsTargetServiceActive(
+ cname, userId, active);
}
} finally {
restoreCallingIdentity(identityToken);
@@ -656,7 +656,8 @@
try {
SyncManager syncManager = getSyncManager();
if (syncManager != null) {
- return (syncManager.getIsTargetServiceActive(cname, userId) == 1);
+ return syncManager.getSyncStorageEngine()
+ .getIsTargetServiceActive(cname, userId);
}
} finally {
restoreCallingIdentity(identityToken);
diff --git a/services/java/com/android/server/content/SyncManager.java b/services/java/com/android/server/content/SyncManager.java
index 3802415..5cc6f65f 100644
--- a/services/java/com/android/server/content/SyncManager.java
+++ b/services/java/com/android/server/content/SyncManager.java
@@ -523,14 +523,6 @@
}
}
- /**
- * TODO: Decide if restricted users have different sync options for the sync service (as is
- * the case with sync adapters).
- */
- public int getIsTargetServiceActive(ComponentName cname, int userId) {
- return mSyncStorageEngine.getIsTargetServiceActive(cname, userId);
- }
-
private void ensureAlarmService() {
if (mAlarmService == null) {
mAlarmService = (AlarmManager) mContext.getSystemService(Context.ALARM_SERVICE);
@@ -564,7 +556,7 @@
final boolean ignoreSettings =
extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, false);
int source = SyncStorageEngine.SOURCE_SERVICE;
- int isEnabled = getIsTargetServiceActive(cname, userId);
+ boolean isEnabled = mSyncStorageEngine.getIsTargetServiceActive(cname, userId);
// Only schedule this sync if
// - we've explicitly been told to ignore settings.
// - global sync is enabled for this user.
@@ -577,7 +569,7 @@
}
return;
}
- if (isEnabled == 0) {
+ if (isEnabled) {
if (isLoggable) {
Log.d(TAG, "scheduleSync: " + cname + " is not enabled, dropping request");
}
@@ -587,7 +579,7 @@
Pair<Long, Long> backoff = mSyncStorageEngine.getBackoff(info);
long delayUntil = mSyncStorageEngine.getDelayUntilTime(info);
final long backoffTime = backoff != null ? backoff.first : 0;
- if (isEnabled < 0) {
+ if (isEnabled) {
// Initialisation sync.
Bundle newExtras = new Bundle();
newExtras.putBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, true);
@@ -2144,7 +2136,7 @@
return false;
}
} else if (target.target_service) {
- if (getIsTargetServiceActive(target.service, target.userId) == 0) {
+ if (mSyncStorageEngine.getIsTargetServiceActive(target.service, target.userId)) {
if (isLoggable) {
Log.v(TAG, " Not scheduling periodic operation: isEnabled == 0.");
}
@@ -2548,7 +2540,8 @@
return false;
}
} else if (target.target_service) {
- state = getIsTargetServiceActive(target.service, target.userId);
+ state = mSyncStorageEngine.getIsTargetServiceActive(target.service, target.userId)
+ ? 1 : 0;
if (state == 0) {
// TODO: Change this to not drop disabled syncs - keep them in the pending queue.
if (isLoggable) {
diff --git a/services/java/com/android/server/content/SyncStorageEngine.java b/services/java/com/android/server/content/SyncStorageEngine.java
index 6cc3d8a..061c042 100644
--- a/services/java/com/android/server/content/SyncStorageEngine.java
+++ b/services/java/com/android/server/content/SyncStorageEngine.java
@@ -696,65 +696,55 @@
}
public void setIsSyncable(Account account, int userId, String providerName, int syncable) {
- synchronized (mAuthorities) {
- AuthorityInfo authority =
- getOrCreateAuthorityLocked(
- new EndPoint(account, providerName, userId),
- -1 /* ident */,
- false);
- setSyncableLocked(authority, syncable);
- }
+ setSyncableStateForTarget(new EndPoint(account, providerName, userId), syncable);
}
- public int getIsTargetServiceActive(ComponentName cname, int userId) {
+ public boolean getIsTargetServiceActive(ComponentName cname, int userId) {
synchronized (mAuthorities) {
if (cname != null) {
AuthorityInfo authority = getAuthorityLocked(
new EndPoint(cname, userId),
"get service enabled");
if (authority == null) {
- return -1;
+ return false;
}
- return authority.syncable;
+ return (authority.syncable == 1);
}
- return -1;
+ return false;
}
}
- public void setIsEnabled(ComponentName cname, int userId, int syncable) {
- synchronized (mAuthorities) {
- AuthorityInfo authority =
- getOrCreateAuthorityLocked(
- new EndPoint(cname, userId), -1, false);
- setSyncableLocked(authority, syncable);
- }
+ public void setIsTargetServiceActive(ComponentName cname, int userId, boolean active) {
+ setSyncableStateForTarget(new EndPoint(cname, userId), active ? 1 : 0);
}
/**
* An enabled sync service and a syncable provider's adapter both get resolved to the same
* persisted variable - namely the "syncable" attribute for an AuthorityInfo in accounts.xml.
- * @param aInfo
- * @param syncable
+ * @param target target to set value for.
+ * @param syncable 0 indicates unsyncable, <0 unknown, >0 is active/syncable.
*/
- private void setSyncableLocked(AuthorityInfo aInfo, int syncable) {
- if (syncable > 1) {
- syncable = 1;
- } else if (syncable < -1) {
- syncable = -1;
- }
- if (Log.isLoggable(TAG, Log.VERBOSE)) {
- Log.d(TAG, "setIsSyncable: " + aInfo.toString() + " -> " + syncable);
- }
-
- if (aInfo.syncable == syncable) {
- if (Log.isLoggable(TAG, Log.VERBOSE)) {
- Log.d(TAG, "setIsSyncable: already set to " + syncable + ", doing nothing");
+ private void setSyncableStateForTarget(EndPoint target, int syncable) {
+ AuthorityInfo aInfo;
+ synchronized (mAuthorities) {
+ aInfo = getOrCreateAuthorityLocked(target, -1, false);
+ if (syncable > 1) {
+ syncable = 1;
+ } else if (syncable < -1) {
+ syncable = -1;
}
- return;
+ if (Log.isLoggable(TAG, Log.VERBOSE)) {
+ Log.d(TAG, "setIsSyncable: " + aInfo.toString() + " -> " + syncable);
+ }
+ if (aInfo.syncable == syncable) {
+ if (Log.isLoggable(TAG, Log.VERBOSE)) {
+ Log.d(TAG, "setIsSyncable: already set to " + syncable + ", doing nothing");
+ }
+ return;
+ }
+ aInfo.syncable = syncable;
+ writeAccountInfoLocked();
}
- aInfo.syncable = syncable;
- writeAccountInfoLocked();
-
if (syncable > 0) {
requestSync(aInfo, SyncOperation.REASON_IS_SYNCABLE, new Bundle());
}
@@ -816,6 +806,7 @@
* @param account account for which to set backoff. Null to specify all accounts.
* @param userId id of the user making this request.
* @param providerName provider for which to set backoff. Null to specify all providers.
+ * @return true if a change occured.
*/
private boolean setBackoffLocked(Account account, int userId, String providerName,
long nextSyncTime, long nextDelay) {