commit de4b8230abe1745c41381792b0c8dded03f638f0
author Alex Buynytskyy <alexbuy@google.com> Sun Apr 25 12:43:26 2021 -0700
committer Alex Buynytskyy <alexbuy@google.com> Mon Apr 26 11:21:01 2021 -0700
tree 344429a230dd18755cb2fe52d36bd9881a58f95e
parent 265ee869175661379b0defcd580ecc7750f2b4e4

Retry on unavailable.

Bug: 182214420
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest PackageManagerServiceTest ChecksumsTest
Change-Id: Iaf61b6825ced45ffdc7e9c87dfea830e50633476

services/incremental/IncrementalService.cpp

diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 4ce336d..9b42799 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -2660,9 +2660,6 @@
     }
 
     switch (targetStatus) {
-        case IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE:
-            // Do nothing, this is a reset state.
-            break;
         case IDataLoaderStatusListener::DATA_LOADER_DESTROYED: {
             switch (currentStatus) {
                 case IDataLoaderStatusListener::DATA_LOADER_BINDING:
@@ -2683,8 +2680,12 @@
         }
         case IDataLoaderStatusListener::DATA_LOADER_CREATED:
             switch (currentStatus) {
-                case IDataLoaderStatusListener::DATA_LOADER_DESTROYED:
                 case IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE:
+                case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
+                    // Before binding need to make sure we are unbound.
+                    // Otherwise we'll get stuck binding.
+                    return destroy();
+                case IDataLoaderStatusListener::DATA_LOADER_DESTROYED:
                 case IDataLoaderStatusListener::DATA_LOADER_BINDING:
                     return bind();
                 case IDataLoaderStatusListener::DATA_LOADER_BOUND:
@@ -2709,7 +2710,8 @@
                    << ", but got: " << mountId;
         return binder::Status::fromServiceSpecificError(-EPERM, "Mount ID mismatch.");
     }
-    if (newStatus == IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE) {
+    if (newStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE ||
+        newStatus == IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE) {
         // User-provided status, let's postpone the handling to avoid possible deadlocks.
         mService.addTimedJob(*mService.mTimedQueue, id(), Constants::userStatusDelay,
                              [this, newStatus]() { setCurrentStatus(newStatus); });
@@ -2721,7 +2723,7 @@
 }
 
 void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
-    int targetStatus, oldStatus;
+    int oldStatus, oldTargetStatus, newTargetStatus;
     DataLoaderStatusListener listener;
     {
         std::unique_lock lock(mMutex);
@@ -2730,22 +2732,31 @@
         }
 
         oldStatus = mCurrentStatus;
-        targetStatus = mTargetStatus;
+        oldTargetStatus = mTargetStatus;
         listener = mStatusListener;
 
         // Change the status.
         mCurrentStatus = newStatus;
         mCurrentStatusTs = mService.mClock->now();
 
-        if (mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE ||
-            mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE) {
-            // For unavailable, unbind from DataLoader to ensure proper re-commit.
-            setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
+        switch (mCurrentStatus) {
+            case IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE:
+                // Unavailable, retry.
+                setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_STARTED);
+                break;
+            case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
+                // Unrecoverable, just unbind.
+                setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
+                break;
+            default:
+                break;
         }
+
+        newTargetStatus = mTargetStatus;
     }
 
     LOG(DEBUG) << "Current status update for DataLoader " << id() << ": " << oldStatus << " -> "
-               << newStatus << " (target " << targetStatus << ")";
+               << newStatus << " (target " << oldTargetStatus << " -> " << newTargetStatus << ")";
 
     if (listener) {
         listener->onStatusChanged(id(), newStatus);