diff options
5 files changed, 50 insertions, 21 deletions
diff --git a/core/java/android/os/incremental/IncrementalFileStorages.java b/core/java/android/os/incremental/IncrementalFileStorages.java index 6e25968192ae..8ebc0814a3dc 100644 --- a/core/java/android/os/incremental/IncrementalFileStorages.java +++ b/core/java/android/os/incremental/IncrementalFileStorages.java @@ -160,7 +160,7 @@ public final class IncrementalFileStorages { /** * Starts or re-starts loading of data. */ - void startLoading( + public void startLoading( @NonNull DataLoaderParams dataLoaderParams, @Nullable IDataLoaderStatusListener statusListener, @Nullable StorageHealthCheckParams healthCheckParams, diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 8202587ecf53..4b0eb6546888 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -3760,11 +3760,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return true; } - // Retrying commit. - if (mIncrementalFileStorages != null) { - return false; - } - final List<InstallationFileParcel> addedFiles = new ArrayList<>(); final List<String> removedFiles = new ArrayList<>(); @@ -3925,18 +3920,24 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { (pkgInfo != null && pkgInfo.applicationInfo != null) ? new File( pkgInfo.applicationInfo.getCodePath()).getParentFile() : null; - mIncrementalFileStorages = IncrementalFileStorages.initialize(mContext, stageDir, - inheritedDir, params, statusListener, healthCheckParams, healthListener, - addedFiles, perUidReadTimeouts, - new IPackageLoadingProgressCallback.Stub() { - @Override - public void onPackageLoadingProgressChanged(float progress) { - synchronized (mProgressLock) { - mIncrementalProgress = progress; - computeProgressLocked(true); + if (mIncrementalFileStorages == null) { + mIncrementalFileStorages = IncrementalFileStorages.initialize(mContext, + stageDir, inheritedDir, params, statusListener, healthCheckParams, + healthListener, addedFiles, perUidReadTimeouts, + new IPackageLoadingProgressCallback.Stub() { + @Override + public void onPackageLoadingProgressChanged(float progress) { + synchronized (mProgressLock) { + mIncrementalProgress = progress; + computeProgressLocked(true); + } } - } - }); + }); + } else { + // Retrying commit. + mIncrementalFileStorages.startLoading(params, statusListener, healthCheckParams, + healthListener, perUidReadTimeouts); + } return false; } catch (IOException e) { throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(), 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; diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h index 5de7325b3d28..b81e1b1b071c 100644 --- a/services/incremental/IncrementalService.h +++ b/services/incremental/IncrementalService.h @@ -255,6 +255,7 @@ private: binder::Status onStatusChanged(MountId mount, int newStatus) final; void setCurrentStatus(int newStatus); + void compareAndSetCurrentStatus(int expectedStatus, int newStatus); sp<content::pm::IDataLoader> getDataLoader(); diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp index 094c5fb36eea..59d96d2393ef 100644 --- a/services/incremental/test/IncrementalServiceTest.cpp +++ b/services/incremental/test/IncrementalServiceTest.cpp @@ -808,7 +808,9 @@ public: METRICS_MILLIS_SINCE_OLDEST_PENDING_READ() .c_str()), &millisSinceOldestPendingRead)); - ASSERT_EQ(expectedMillisSinceOldestPendingRead, millisSinceOldestPendingRead); + // Allow 10ms. + ASSERT_LE(expectedMillisSinceOldestPendingRead, millisSinceOldestPendingRead); + ASSERT_GE(expectedMillisSinceOldestPendingRead + 10, millisSinceOldestPendingRead); int storageHealthStatusCode = -1; ASSERT_TRUE( result.getInt(String16(BnIncrementalService::METRICS_STORAGE_HEALTH_STATUS_CODE() |