From 702d611662059810eca43613f200595d8b8f7b51 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Mon, 6 Jan 2020 10:59:29 +0800 Subject: [PATCH] Don't clean the sleep queue The code was there to prevent sleep timeouts from leaking to next tests and causing timeouts in enabling rollbacks. However, testEnableRollbackTimeoutFailsRollback_MultiPackage becomes flaky due to the clean-up: 1. Install#commit posts SessionCallback#onFinished to the handler thread of RollbackManagerServiceImpl. mSleepDuration.clear() is called there. 2. RollbackManager#blockRollbackManager posts mSleepDuration.addLast() to the handler thread. 3. Depending on the thread scheduling, mSleepDuration.clear() might happen after mSleepDuration.addLast() even though Install#commit happens before RollbackManager#blockRollbackManager. 4. This nullifies the effect of #blockRollbackManager and fails the test because enable-rollback-timeout didn't work as intended. 5. See b/145641719 comment#2 for detailed explanation. We remove the code to prevent the above scenario from happening at the risk of leaking sleep timeouts to next tests. However, this hypothetical leaking never happens. We should be fine removing the code. If the leak does happen, it could mean a bug in the install flow of the package manager. We need much deeper analysis before fixing the problem. Bug: 145641719 Test: atest RollbackTest Change-Id: I37dff9b66e69578749eaff332b84879c7956b911 --- .../android/server/rollback/RollbackManagerServiceImpl.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 1475ab9df37b7..ae3f368cad088 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -1139,9 +1139,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } else { removeRollbackForPackageSessionId(sessionId); } - - // Clear the queue so it will never be leaked to next tests. - mSleepDuration.clear(); } }