Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4bafd4dd authored by Alex Buynytskyy's avatar Alex Buynytskyy
Browse files

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
parent 0d0d1ae6

core/java/android/os/incremental/IncrementalFileStorages.java

+1 −1
Original line number Diff line number Diff line
@@ -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,

services/core/java/com/android/server/pm/PackageInstallerSession.java

+17 −16
Original line number Diff line number Diff line
@@ -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,9 +3920,10 @@ 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,

                if (mIncrementalFileStorages == null) {

                    mIncrementalFileStorages = IncrementalFileStorages.initialize(mContext,

                            stageDir, inheritedDir, params, statusListener, healthCheckParams,

                            healthListener, addedFiles, perUidReadTimeouts,

                            new IPackageLoadingProgressCallback.Stub() {

                                @Override

                                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;

            } catch (IOException e) {

                throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(),

services/incremental/IncrementalService.cpp

+28 −3
Original line number Diff line number Diff line
@@ -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;

services/incremental/IncrementalService.h

+1 −0
Original line number Diff line number Diff line
@@ -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();

services/incremental/test/IncrementalServiceTest.cpp

+3 −1
Original line number Diff line number Diff line
@@ -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()