From 0741b41d949f5f91e6ed34e15c67327ae21bbd76 Mon Sep 17 00:00:00 2001 From: Richard Uhler Date: Wed, 27 Mar 2019 13:27:55 +0000 Subject: [PATCH] Block getAvailableRollbacks on the handler thread. To reduce flakiness in the CTS test, which checks for an available rollback immediately after an app update is committed. Remove sleeps from the RollbackTest that should no longer be needed. Test: atest RollbackTest CtsRollbackManagerTestCases Bug: 127933960 Change-Id: I60adb0f58eaba02d08ecd44b44866a8505c58238 --- .../rollback/RollbackManagerServiceImpl.java | 16 +++++++ .../android/tests/rollback/RollbackTest.java | 44 ------------------- 2 files changed, 16 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index d6327494a24dc..791c8fc996e43 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -225,6 +225,22 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { public ParceledListSlice getAvailableRollbacks() { enforceManageRollbacks("getAvailableRollbacks"); + // Wait for the handler thread to get the list of available rollbacks + // to get the most up-to-date results. This is intended to reduce test + // flakiness when checking available rollbacks immediately after + // installing a package with rollback enabled. + final LinkedBlockingQueue result = new LinkedBlockingQueue<>(); + getHandler().post(() -> result.offer(true)); + + try { + result.take(); + } catch (InterruptedException ie) { + // We may not get the most up-to-date information, but whatever we + // can get now is better than nothing, so log but otherwise ignore + // the exception. + Log.w(TAG, "Interrupted while waiting for handler thread in getAvailableRollbacks"); + } + synchronized (mLock) { ensureRollbackDataLoadedLocked(); List rollbacks = new ArrayList<>(); diff --git a/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java b/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java index 834743d7ffed2..d41a5d68912e4 100644 --- a/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java +++ b/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java @@ -108,10 +108,6 @@ public class RollbackTest { } // The app should not be available for rollback. - // TODO: See if there is a way to remove this race condition - // between when the app is uninstalled and when the previously - // available rollback, if any, is removed. - Thread.sleep(1000); assertNull(getUniqueRollbackInfoForPackage(rm.getAvailableRollbacks(), TEST_APP_A)); // There should be no recently committed rollbacks for this package. @@ -127,10 +123,6 @@ public class RollbackTest { assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); // The app should now be available for rollback. - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // is made available. - Thread.sleep(1000); RollbackInfo rollback = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TEST_APP_A); assertRollbackInfoEquals(TEST_APP_A, 2, 1, rollback); @@ -187,11 +179,6 @@ public class RollbackTest { assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_B)); // Both test apps should now be available for rollback. - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // is made available. - Thread.sleep(1000); - RollbackInfo rollbackA = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TEST_APP_A); assertRollbackInfoEquals(TEST_APP_A, 2, 1, rollbackA); @@ -246,11 +233,6 @@ public class RollbackTest { assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_B)); // The app should now be available for rollback. - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // is made available. - Thread.sleep(1000); - RollbackInfo rollbackA = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TEST_APP_A); assertRollbackInfoForAandB(rollbackA); @@ -297,10 +279,6 @@ public class RollbackTest { assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); // The app should now be available for rollback. - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // is made available. - Thread.sleep(1000); RollbackInfo rollback = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TEST_APP_A); @@ -481,10 +459,6 @@ public class RollbackTest { assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); // The app should now be available for rollback. - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // is made available. - Thread.sleep(1000); RollbackInfo rollback = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TEST_APP_A); assertRollbackInfoEquals(TEST_APP_A, 2, 1, rollback); @@ -610,10 +584,6 @@ public class RollbackTest { // Both test apps should now be available for rollback, and the // RollbackInfo returned for the rollbacks should be correct. - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // is made available. - Thread.sleep(1000); RollbackInfo rollbackA = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TEST_APP_A); assertRollbackInfoEquals(TEST_APP_A, 2, 1, rollbackA); @@ -709,11 +679,6 @@ public class RollbackTest { // been enabled. assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // would be made available. - Thread.sleep(1000); - RollbackTestUtils.adoptShellPermissionIdentity( Manifest.permission.TEST_MANAGE_ROLLBACKS); RollbackManager rm = RollbackTestUtils.getRollbackManager(); @@ -745,11 +710,6 @@ public class RollbackTest { // been enabled because the test app is not a module. assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // would be made available. - Thread.sleep(1000); - RollbackManager rm = RollbackTestUtils.getRollbackManager(); assertNull(getUniqueRollbackInfoForPackage(rm.getAvailableRollbacks(), TEST_APP_A)); } finally { @@ -844,10 +804,6 @@ public class RollbackTest { // Both test apps should now be available for rollback, and the // targetPackage returned for rollback should be correct. - // TODO: See if there is a way to remove this race condition - // between when the app is installed and when the rollback - // is made available. - Thread.sleep(1000); RollbackInfo rollbackA = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TEST_APP_A); assertRollbackInfoEquals(TEST_APP_A, 2, 1, rollbackA);