From dd5e5e35b2e02f5a1a962d9a1de8507dede269c1 Mon Sep 17 00:00:00 2001 From: Ruslan Tkhakokhov Date: Tue, 16 Apr 2019 11:51:17 +0100 Subject: BroadcastReceiver in UserBackupManagerService can crash system_server Make sure mBroadcastReceiver is initialized after its dependencies, i.e. mTransportManager. Bug: 130408863 Test: 1) atest RunBackupFrameworksServicesRoboTests 2) atest CtsBackupTestCases 3) atest CtsBackupHostTestCases 4) atest GtsBackupTestCases 5) atest GtsBackupHostTestCases Manual test: 1) Before fix: Add Thread.sleep() before mTransportManager is initialized in constructor and tirgger PACKAGE_CHANGED event. Verify broadcast receiver callback is triggered and systen_process crashes. 2) After fix: Add Thread.sleep() before mTransportManager is initialized in constructor and tirgger PACKAGE_CHANGED event. Verify broadcast receiver callback is not triggered. Change-Id: If1628628176a08a2d33d020ce270de92b606d6df --- .../server/backup/UserBackupManagerService.java | 13 +++++++--- .../backup/UserBackupManagerServiceTest.java | 29 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java index 447bd8c237dd..03f21498766d 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java @@ -566,10 +566,6 @@ public class UserBackupManagerService { // require frequent starting and stopping. mConstants.start(); - // Set up the various sorts of package tracking we do - mFullBackupScheduleFile = new File(mBaseStateDir, "fb-schedule"); - initPackageTracking(); - // Build our mapping of uid to backup client services. This implicitly // schedules a backup pass on the Package Manager metadata the first // time anything needs to be backed up. @@ -589,6 +585,10 @@ public class UserBackupManagerService { // Power management mWakelock = mPowerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "*backup*-" + userId); + + // Set up the various sorts of package tracking we do + mFullBackupScheduleFile = new File(mBaseStateDir, "fb-schedule"); + initPackageTracking(); } void initializeBackupEnableState() { @@ -744,6 +744,11 @@ public class UserBackupManagerService { return mDataDir; } + @VisibleForTesting + BroadcastReceiver getPackageTrackingReceiver() { + return mBroadcastReceiver; + } + @Nullable public DataChangedJournal getJournal() { return mJournal; diff --git a/services/robotests/backup/src/com/android/server/backup/UserBackupManagerServiceTest.java b/services/robotests/backup/src/com/android/server/backup/UserBackupManagerServiceTest.java index c8d1eb413ecf..74fe81c6f68e 100644 --- a/services/robotests/backup/src/com/android/server/backup/UserBackupManagerServiceTest.java +++ b/services/robotests/backup/src/com/android/server/backup/UserBackupManagerServiceTest.java @@ -30,6 +30,7 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.robolectric.Shadows.shadowOf; @@ -38,6 +39,7 @@ import static org.testng.Assert.expectThrows; import android.app.backup.BackupManager; import android.app.backup.IBackupObserver; import android.app.backup.ISelectBackupTransportCallback; +import android.content.BroadcastReceiver; import android.content.ComponentName; import android.content.Context; import android.content.ContextWrapper; @@ -48,6 +50,7 @@ import android.os.Binder; import android.os.HandlerThread; import android.os.PowerManager; import android.os.PowerSaveState; +import android.os.UserHandle; import android.platform.test.annotations.Presubmit; import android.provider.Settings; @@ -66,6 +69,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; @@ -1130,6 +1134,31 @@ public class UserBackupManagerServiceTest { /* transportManager */ null)); } + /** + * Test verifying that creating a new instance registers the broadcast receiver for package + * tracking + */ + @Test + public void testCreateAndInitializeService_registersPackageTrackingReceiver() throws Exception { + Context contextSpy = Mockito.spy(mContext); + + UserBackupManagerService service = UserBackupManagerService.createAndInitializeService( + USER_ID, + contextSpy, + new Trampoline(mContext), + mBackupThread, + mBaseStateDir, + mDataDir, + mTransportManager); + + BroadcastReceiver packageTrackingReceiver = service.getPackageTrackingReceiver(); + assertThat(packageTrackingReceiver).isNotNull(); + + // One call for package changes and one call for sd card events. + verify(contextSpy, times(2)).registerReceiverAsUser( + eq(packageTrackingReceiver), eq(UserHandle.of(USER_ID)), any(), any(), any()); + } + private UserBackupManagerService createUserBackupManagerServiceAndRunTasks() { return BackupManagerServiceTestUtils.createUserBackupManagerServiceAndRunTasks( USER_ID, mContext, mBackupThread, mBaseStateDir, mDataDir, mTransportManager); -- cgit v1.2.3-59-g8ed1b