Proper retrying DL installation sessions.

Plus more robust handling of broken DLs.

Bug: 190012477
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest com.google.android.packageinstallerv2proxy.host.gts.IncrementalInstallerHostTest
Change-Id: I5cb037d49cd2b140bed1045c99f072112495acfc

1 files changed, 28 insertions, 3 deletions

diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 47d59b250e29..757c9dec06ed 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -89,6 +89,11 @@ struct Constants {
 
     // Max interval after system invoked the DL when readlog collection can be enabled.
     static constexpr auto readLogsMaxInterval = 2h;
+
+    // How long should we wait till dataLoader reports destroyed.
+    static constexpr auto destroyTimeout = 60s;
+
+    static constexpr auto anyStatus = INT_MIN;
 };
 
 static const Constants& constants() {
@@ -2554,7 +2559,7 @@ void IncrementalService::DataLoaderStub::cleanupResources() {
         mControl = {};
         mHealthControl = {};
         mHealthListener = {};
-        mStatusCondition.wait_until(lock, now + 60s, [this] {
+        mStatusCondition.wait_until(lock, now + Constants::destroyTimeout, [this] {
             return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
         });
         mStatusListener = {};
@@ -2754,8 +2759,16 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
     switch (targetStatus) {
         case IDataLoaderStatusListener::DATA_LOADER_DESTROYED: {
             switch (currentStatus) {
+                case IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE:
+                case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
+                    destroy();
+                    // DataLoader is broken, just assume it's destroyed.
+                    compareAndSetCurrentStatus(currentStatus,
+                                               IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
+                    return true;
                 case IDataLoaderStatusListener::DATA_LOADER_BINDING:
-                    setCurrentStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
+                    compareAndSetCurrentStatus(currentStatus,
+                                               IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
                     return true;
                 default:
                     return destroy();
@@ -2776,7 +2789,11 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
                 case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
                     // Before binding need to make sure we are unbound.
                     // Otherwise we'll get stuck binding.
-                    return destroy();
+                    destroy();
+                    // DataLoader is broken, just assume it's destroyed.
+                    compareAndSetCurrentStatus(currentStatus,
+                                               IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
+                    return true;
                 case IDataLoaderStatusListener::DATA_LOADER_DESTROYED:
                 case IDataLoaderStatusListener::DATA_LOADER_BINDING:
                     return bind();
@@ -2815,6 +2832,11 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
 }
 
 void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
+    compareAndSetCurrentStatus(Constants::anyStatus, newStatus);
+}
+
+void IncrementalService::DataLoaderStub::compareAndSetCurrentStatus(int expectedStatus,
+                                                                    int newStatus) {
     int oldStatus, oldTargetStatus, newTargetStatus;
     DataLoaderStatusListener listener;
     {
@@ -2822,6 +2844,9 @@ void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
         if (mCurrentStatus == newStatus) {
             return;
         }
+        if (expectedStatus != Constants::anyStatus && expectedStatus != mCurrentStatus) {
+            return;
+        }
 
         oldStatus = mCurrentStatus;
         oldTargetStatus = mTargetStatus;