Merge "Proper retrying DL installation sessions." into sc-dev

This commit is contained in:
TreeHugger Robot
2021-06-09 17:02:54 +00:00
committed by Android (Google) Code Review
5 changed files with 50 additions and 21 deletions

View File

@@ -160,7 +160,7 @@ public final class IncrementalFileStorages {
/** /**
* Starts or re-starts loading of data. * Starts or re-starts loading of data.
*/ */
void startLoading( public void startLoading(
@NonNull DataLoaderParams dataLoaderParams, @NonNull DataLoaderParams dataLoaderParams,
@Nullable IDataLoaderStatusListener statusListener, @Nullable IDataLoaderStatusListener statusListener,
@Nullable StorageHealthCheckParams healthCheckParams, @Nullable StorageHealthCheckParams healthCheckParams,

View File

@@ -3760,11 +3760,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
return true; return true;
} }
// Retrying commit.
if (mIncrementalFileStorages != null) {
return false;
}
final List<InstallationFileParcel> addedFiles = new ArrayList<>(); final List<InstallationFileParcel> addedFiles = new ArrayList<>();
final List<String> removedFiles = new ArrayList<>(); final List<String> removedFiles = new ArrayList<>();
@@ -3925,9 +3920,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
(pkgInfo != null && pkgInfo.applicationInfo != null) ? new File( (pkgInfo != null && pkgInfo.applicationInfo != null) ? new File(
pkgInfo.applicationInfo.getCodePath()).getParentFile() : null; pkgInfo.applicationInfo.getCodePath()).getParentFile() : null;
mIncrementalFileStorages = IncrementalFileStorages.initialize(mContext, stageDir, if (mIncrementalFileStorages == null) {
inheritedDir, params, statusListener, healthCheckParams, healthListener, mIncrementalFileStorages = IncrementalFileStorages.initialize(mContext,
addedFiles, perUidReadTimeouts, stageDir, inheritedDir, params, statusListener, healthCheckParams,
healthListener, addedFiles, perUidReadTimeouts,
new IPackageLoadingProgressCallback.Stub() { new IPackageLoadingProgressCallback.Stub() {
@Override @Override
public void onPackageLoadingProgressChanged(float progress) { public void onPackageLoadingProgressChanged(float progress) {
@@ -3937,6 +3933,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
} }
}); });
} else {
// Retrying commit.
mIncrementalFileStorages.startLoading(params, statusListener, healthCheckParams,
healthListener, perUidReadTimeouts);
}
return false; return false;
} catch (IOException e) { } catch (IOException e) {
throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(), throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(),

View File

@@ -89,6 +89,11 @@ struct Constants {
// Max interval after system invoked the DL when readlog collection can be enabled. // Max interval after system invoked the DL when readlog collection can be enabled.
static constexpr auto readLogsMaxInterval = 2h; 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() { static const Constants& constants() {
@@ -2554,7 +2559,7 @@ void IncrementalService::DataLoaderStub::cleanupResources() {
mControl = {}; mControl = {};
mHealthControl = {}; mHealthControl = {};
mHealthListener = {}; mHealthListener = {};
mStatusCondition.wait_until(lock, now + 60s, [this] { mStatusCondition.wait_until(lock, now + Constants::destroyTimeout, [this] {
return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED; return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
}); });
mStatusListener = {}; mStatusListener = {};
@@ -2754,8 +2759,16 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
switch (targetStatus) { switch (targetStatus) {
case IDataLoaderStatusListener::DATA_LOADER_DESTROYED: { case IDataLoaderStatusListener::DATA_LOADER_DESTROYED: {
switch (currentStatus) { 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: case IDataLoaderStatusListener::DATA_LOADER_BINDING:
setCurrentStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED); compareAndSetCurrentStatus(currentStatus,
IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
return true; return true;
default: default:
return destroy(); return destroy();
@@ -2776,7 +2789,11 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE: case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
// Before binding need to make sure we are unbound. // Before binding need to make sure we are unbound.
// Otherwise we'll get stuck binding. // 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_DESTROYED:
case IDataLoaderStatusListener::DATA_LOADER_BINDING: case IDataLoaderStatusListener::DATA_LOADER_BINDING:
return bind(); return bind();
@@ -2815,6 +2832,11 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
} }
void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) { void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
compareAndSetCurrentStatus(Constants::anyStatus, newStatus);
}
void IncrementalService::DataLoaderStub::compareAndSetCurrentStatus(int expectedStatus,
int newStatus) {
int oldStatus, oldTargetStatus, newTargetStatus; int oldStatus, oldTargetStatus, newTargetStatus;
DataLoaderStatusListener listener; DataLoaderStatusListener listener;
{ {
@@ -2822,6 +2844,9 @@ void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
if (mCurrentStatus == newStatus) { if (mCurrentStatus == newStatus) {
return; return;
} }
if (expectedStatus != Constants::anyStatus && expectedStatus != mCurrentStatus) {
return;
}
oldStatus = mCurrentStatus; oldStatus = mCurrentStatus;
oldTargetStatus = mTargetStatus; oldTargetStatus = mTargetStatus;

View File

@@ -255,6 +255,7 @@ private:
binder::Status onStatusChanged(MountId mount, int newStatus) final; binder::Status onStatusChanged(MountId mount, int newStatus) final;
void setCurrentStatus(int newStatus); void setCurrentStatus(int newStatus);
void compareAndSetCurrentStatus(int expectedStatus, int newStatus);
sp<content::pm::IDataLoader> getDataLoader(); sp<content::pm::IDataLoader> getDataLoader();

View File

@@ -808,7 +808,9 @@ public:
METRICS_MILLIS_SINCE_OLDEST_PENDING_READ() METRICS_MILLIS_SINCE_OLDEST_PENDING_READ()
.c_str()), .c_str()),
&millisSinceOldestPendingRead)); &millisSinceOldestPendingRead));
ASSERT_EQ(expectedMillisSinceOldestPendingRead, millisSinceOldestPendingRead); // Allow 10ms.
ASSERT_LE(expectedMillisSinceOldestPendingRead, millisSinceOldestPendingRead);
ASSERT_GE(expectedMillisSinceOldestPendingRead + 10, millisSinceOldestPendingRead);
int storageHealthStatusCode = -1; int storageHealthStatusCode = -1;
ASSERT_TRUE( ASSERT_TRUE(
result.getInt(String16(BnIncrementalService::METRICS_STORAGE_HEALTH_STATUS_CODE() result.getInt(String16(BnIncrementalService::METRICS_STORAGE_HEALTH_STATUS_CODE()