diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java index edcd4b7ba99b0..0dd518141cf63 100644 --- a/services/backup/java/com/android/server/backup/BackupManagerService.java +++ b/services/backup/java/com/android/server/backup/BackupManagerService.java @@ -1983,7 +1983,7 @@ public class BackupManagerService implements BackupManagerServiceInterface { if (uri == null) { return; } - String pkgName = uri.getSchemeSpecificPart(); + final String pkgName = uri.getSchemeSpecificPart(); if (pkgName != null) { pkgList = new String[] { pkgName }; } @@ -1991,7 +1991,7 @@ public class BackupManagerService implements BackupManagerServiceInterface { // At package-changed we only care about looking at new transport states if (changed) { - String[] components = + final String[] components = intent.getStringArrayExtra(Intent.EXTRA_CHANGED_COMPONENT_NAME_LIST); if (MORE_DEBUG) { @@ -2001,7 +2001,8 @@ public class BackupManagerService implements BackupManagerServiceInterface { } } - mTransportManager.onPackageChanged(pkgName, components); + mBackupHandler.post( + () -> mTransportManager.onPackageChanged(pkgName, components)); return; // nothing more to do in the PACKAGE_CHANGED case } @@ -2033,7 +2034,7 @@ public class BackupManagerService implements BackupManagerServiceInterface { } // If they're full-backup candidates, add them there instead final long now = System.currentTimeMillis(); - for (String packageName : pkgList) { + for (final String packageName : pkgList) { try { PackageInfo app = mPackageManager.getPackageInfo(packageName, 0); if (appGetsFullBackup(app) @@ -2050,7 +2051,8 @@ public class BackupManagerService implements BackupManagerServiceInterface { writeFullBackupScheduleAsync(); } - mTransportManager.onPackageAdded(packageName); + mBackupHandler.post( + () -> mTransportManager.onPackageAdded(packageName)); } catch (NameNotFoundException e) { // doesn't really exist; ignore it @@ -2074,8 +2076,9 @@ public class BackupManagerService implements BackupManagerServiceInterface { removePackageParticipantsLocked(pkgList, uid); } } - for (String pkgName : pkgList) { - mTransportManager.onPackageRemoved(pkgName); + for (final String pkgName : pkgList) { + mBackupHandler.post( + () -> mTransportManager.onPackageRemoved(pkgName)); } } } diff --git a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java index 74859777ae1e6..23abd934334bb 100644 --- a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java @@ -1197,7 +1197,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter if (uri == null) { return; } - String pkgName = uri.getSchemeSpecificPart(); + final String pkgName = uri.getSchemeSpecificPart(); if (pkgName != null) { pkgList = new String[]{pkgName}; } @@ -1205,7 +1205,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter // At package-changed we only care about looking at new transport states if (changed) { - String[] components = + final String[] components = intent.getStringArrayExtra(Intent.EXTRA_CHANGED_COMPONENT_NAME_LIST); if (MORE_DEBUG) { @@ -1215,7 +1215,8 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter } } - mTransportManager.onPackageChanged(pkgName, components); + mBackupHandler.post( + () -> mTransportManager.onPackageChanged(pkgName, components)); return; // nothing more to do in the PACKAGE_CHANGED case } @@ -1247,7 +1248,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter } // If they're full-backup candidates, add them there instead final long now = System.currentTimeMillis(); - for (String packageName : pkgList) { + for (final String packageName : pkgList) { try { PackageInfo app = mPackageManager.getPackageInfo(packageName, 0); if (AppBackupUtils.appGetsFullBackup(app) @@ -1265,7 +1266,8 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter writeFullBackupScheduleAsync(); } - mTransportManager.onPackageAdded(packageName); + mBackupHandler.post( + () -> mTransportManager.onPackageAdded(packageName)); } catch (NameNotFoundException e) { // doesn't really exist; ignore it @@ -1289,8 +1291,9 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter removePackageParticipantsLocked(pkgList, uid); } } - for (String pkgName : pkgList) { - mTransportManager.onPackageRemoved(pkgName); + for (final String pkgName : pkgList) { + mBackupHandler.post( + () -> mTransportManager.onPackageRemoved(pkgName)); } } } diff --git a/services/backup/java/com/android/server/backup/TransportManager.java b/services/backup/java/com/android/server/backup/TransportManager.java index fb2982eb0baaa..dab218db51b2c 100644 --- a/services/backup/java/com/android/server/backup/TransportManager.java +++ b/services/backup/java/com/android/server/backup/TransportManager.java @@ -316,9 +316,9 @@ public class TransportManager { private class TransportConnection implements ServiceConnection { // Hold mTransportsLock to access these fields so as to provide a consistent view of them. - private IBackupTransport mBinder; + private volatile IBackupTransport mBinder; private final List mListeners = new ArrayList<>(); - private String mTransportName; + private volatile String mTransportName; private final ComponentName mTransportComponent; @@ -401,25 +401,24 @@ public class TransportManager { + rebindTimeout + "ms"); } + // Intentionally not synchronized -- the variable is volatile and changes to its value + // are inside synchronized blocks, providing a memory sync barrier; and this method + // does not touch any other state protected by that lock. private IBackupTransport getBinder() { - synchronized (mTransportLock) { - return mBinder; - } + return mBinder; } + // Intentionally not synchronized; same as getBinder() private String getName() { - synchronized (mTransportLock) { - return mTransportName; - } + return mTransportName; } + // Intentionally not synchronized; same as getBinder() private void bindIfUnbound() { - synchronized (mTransportLock) { - if (mBinder == null) { - Slog.d(TAG, - "Rebinding to transport " + mTransportComponent.flattenToShortString()); - bindToTransport(mTransportComponent, this); - } + if (mBinder == null) { + Slog.d(TAG, + "Rebinding to transport " + mTransportComponent.flattenToShortString()); + bindToTransport(mTransportComponent, this); } }