Don't try to recreate IncrementalFileStorages on re-commit.
Bug: 156287164
Fixes 156287164
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Change-Id: I75e392a1fa84df8b6ac0b855233f9a2662998e96
diff --git a/core/java/android/os/incremental/IncrementalFileStorages.java b/core/java/android/os/incremental/IncrementalFileStorages.java
index 321dc9e..958c7fb 100644
--- a/core/java/android/os/incremental/IncrementalFileStorages.java
+++ b/core/java/android/os/incremental/IncrementalFileStorages.java
@@ -92,10 +92,7 @@
}
}
- if (!result.mDefaultStorage.startLoading()) {
- // TODO(b/146080380): add incremental-specific error code
- throw new IOException("Failed to start loading data for Incremental installation.");
- }
+ result.startLoading();
return result;
}
@@ -144,6 +141,15 @@
}
/**
+ * Starts or re-starts loading of data.
+ */
+ public void startLoading() throws IOException {
+ if (!mDefaultStorage.startLoading()) {
+ throw new IOException("Failed to start loading data for Incremental installation.");
+ }
+ }
+
+ /**
* Resets the states and unbinds storage instances for an installation session.
* TODO(b/136132412): make sure unnecessary binds are removed but useful storages are kept
*/
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index e0d057a..448ce44 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -395,7 +395,6 @@
private boolean mDataLoaderFinished = false;
- // TODO(b/146080380): merge file list with Callback installation.
private IncrementalFileStorages mIncrementalFileStorages;
private static final FileFilter sAddedApkFilter = new FileFilter() {
@@ -2682,6 +2681,7 @@
/**
* Makes sure files are present in staging location.
+ * @return if the image is ready for installation
*/
@GuardedBy("mLock")
private boolean prepareDataLoaderLocked()
@@ -2693,6 +2693,17 @@
return true;
}
+ // Retrying commit.
+ if (mIncrementalFileStorages != null) {
+ try {
+ mIncrementalFileStorages.startLoading();
+ } catch (IOException e) {
+ throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(),
+ e.getCause());
+ }
+ return false;
+ }
+
final List<InstallationFileParcel> addedFiles = new ArrayList<>();
final List<String> removedFiles = new ArrayList<>();
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index a9dc92f..78439db 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -748,7 +748,7 @@
return -EINVAL;
}
- LOG(INFO) << "Removing bind point " << target;
+ LOG(INFO) << "Removing bind point " << target << " for storage " << storage;
// Here we should only look up by the exact target, not by a subdirectory of any existing mount,
// otherwise there's a chance to unmount something completely unrelated
@@ -1807,6 +1807,8 @@
targetStatus = mTargetStatus;
}
+ LOG(DEBUG) << "fsmStep: " << mId << ": " << currentStatus << " -> " << targetStatus;
+
if (currentStatus == targetStatus) {
return true;
}
@@ -1868,8 +1870,8 @@
listener = mListener;
if (mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE) {
- // For unavailable, reset target status.
- setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE);
+ // For unavailable, unbind from DataLoader to ensure proper re-commit.
+ setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
}
}
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 325218d..2e4625c 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -677,10 +677,10 @@
mDataLoaderManager->bindToDataLoaderSuccess();
mDataLoaderManager->getDataLoaderSuccess();
EXPECT_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _)).Times(2);
- EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(1);
+ EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(2);
EXPECT_CALL(*mDataLoader, create(_, _, _, _)).Times(2);
EXPECT_CALL(*mDataLoader, start(_)).Times(0);
- EXPECT_CALL(*mDataLoader, destroy(_)).Times(1);
+ EXPECT_CALL(*mDataLoader, destroy(_)).Times(2);
EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
EXPECT_CALL(*mLooper, addFd(MockIncFs::kPendingReadsFd, _, _, _, _)).Times(1);
EXPECT_CALL(*mLooper, removeFd(MockIncFs::kPendingReadsFd)).Times(1);