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

Commit de4b8230 authored by Alex Buynytskyy's avatar Alex Buynytskyy
Browse files

Retry on unavailable.

Bug: 182214420
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest PackageManagerServiceTest ChecksumsTest
Change-Id: Iaf61b6825ced45ffdc7e9c87dfea830e50633476
parent 265ee869
Loading
Loading
Loading
Loading
+23 −12
Original line number Diff line number Diff line
@@ -2660,9 +2660,6 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
    }

    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 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
        }
        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 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
                   << ", 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 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
}

void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
    int targetStatus, oldStatus;
    int oldStatus, oldTargetStatus, newTargetStatus;
    DataLoaderStatusListener listener;
    {
        std::unique_lock lock(mMutex);
@@ -2730,22 +2732,31 @@ void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
        }

        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.
        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);
+85 −5
Original line number Diff line number Diff line
@@ -126,7 +126,7 @@ private:
class MockDataLoader : public IDataLoader {
public:
    MockDataLoader() {
        ON_CALL(*this, create(_, _, _, _)).WillByDefault(Invoke(this, &MockDataLoader::createOk));
        initializeCreateOk();
        ON_CALL(*this, start(_)).WillByDefault(Invoke(this, &MockDataLoader::startOk));
        ON_CALL(*this, stop(_)).WillByDefault(Invoke(this, &MockDataLoader::stopOk));
        ON_CALL(*this, destroy(_)).WillByDefault(Invoke(this, &MockDataLoader::destroyOk));
@@ -145,6 +145,10 @@ public:
                 binder::Status(int32_t id, const std::vector<InstallationFileParcel>& addedFiles,
                                const std::vector<std::string>& removedFiles));

    void initializeCreateOk() {
        ON_CALL(*this, create(_, _, _, _)).WillByDefault(Invoke(this, &MockDataLoader::createOk));
    }

    void initializeCreateOkNoStatus() {
        ON_CALL(*this, create(_, _, _, _))
                .WillByDefault(Invoke(this, &MockDataLoader::createOkNoStatus));
@@ -275,6 +279,14 @@ public:
        }
        return binder::Status::ok();
    }
    binder::Status bindToDataLoaderOkWithNoDelay(int32_t mountId,
                                                 const DataLoaderParamsParcel& params,
                                                 int bindDelayMs,
                                                 const sp<IDataLoaderStatusListener>& listener,
                                                 bool* _aidl_return) {
        CHECK(bindDelayMs == 0) << bindDelayMs;
        return bindToDataLoaderOk(mountId, params, bindDelayMs, listener, _aidl_return);
    }
    binder::Status bindToDataLoaderOkWith1sDelay(int32_t mountId,
                                                 const DataLoaderParamsParcel& params,
                                                 int bindDelayMs,
@@ -338,14 +350,13 @@ public:
        mListener->onStatusChanged(mId, IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE);
    }
    binder::Status unbindFromDataLoaderOk(int32_t id) {
        mBindDelayMs = -1;
        if (mDataLoader) {
            if (auto status = mDataLoader->destroy(id); !status.isOk()) {
                return status;
            }
            mDataLoader = nullptr;
        }
        mBindDelayMs = -1;
        if (mListener) {
        } else if (mListener) {
            mListener->onStatusChanged(id, IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
        }
        return binder::Status::ok();
@@ -1156,12 +1167,81 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderRecreateOnPendingReads) {
    ASSERT_GE(storageId, 0);
    ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
                                                  {}, {}));
    mDataLoaderManager->setDataLoaderStatusUnavailable();
    mDataLoaderManager->setDataLoaderStatusUnrecoverable();

    // Timed callback present.
    ASSERT_EQ(storageId, mTimedQueue->mId);
    ASSERT_GE(mTimedQueue->mAfter, 10ms);
    auto timedCallback = mTimedQueue->mWhat;
    mTimedQueue->clearJob(storageId);

    // First callback call to propagate unrecoverable.
    timedCallback();

    // And second call to trigger recreation.
    ASSERT_NE(nullptr, mLooper->mCallback);
    ASSERT_NE(nullptr, mLooper->mCallbackData);
    mLooper->mCallback(-1, -1, mLooper->mCallbackData);
}

TEST_F(IncrementalServiceTest, testStartDataLoaderUnavailable) {
    mIncFs->openMountSuccess();
    mDataLoader->initializeCreateOkNoStatus();

    EXPECT_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _)).Times(3);
    EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(3);
    EXPECT_CALL(*mDataLoader, create(_, _, _, _)).Times(3);
    EXPECT_CALL(*mDataLoader, start(_)).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);
    TemporaryDir tempDir;
    int storageId =
            mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
                                               IncrementalService::CreateOptions::CreateNew);
    ASSERT_GE(storageId, 0);

    ON_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _))
            .WillByDefault(Invoke(mDataLoaderManager,
                                  &MockDataLoaderManager::bindToDataLoaderOkWithNoDelay));

    ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
                                                  {}, {}));

    // Unavailable.
    mDataLoaderManager->setDataLoaderStatusUnavailable();

    // Timed callback present.
    ASSERT_EQ(storageId, mTimedQueue->mId);
    ASSERT_GE(mTimedQueue->mAfter, 10ms);
    auto timedCallback = mTimedQueue->mWhat;
    mTimedQueue->clearJob(storageId);

    // Propagating unavailable and expecting it to trigger rebind with 1s retry delay.
    ON_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _))
            .WillByDefault(Invoke(mDataLoaderManager,
                                  &MockDataLoaderManager::bindToDataLoaderOkWith1sDelay));
    timedCallback();

    // Unavailable #2.
    mDataLoaderManager->setDataLoaderStatusUnavailable();

    // Timed callback present.
    ASSERT_EQ(storageId, mTimedQueue->mId);
    ASSERT_GE(mTimedQueue->mAfter, 10ms);
    timedCallback = mTimedQueue->mWhat;
    mTimedQueue->clearJob(storageId);

    // Propagating unavailable and expecting it to trigger rebind with 10s retry delay.
    // This time succeed.
    mDataLoader->initializeCreateOk();
    ON_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _))
            .WillByDefault(Invoke(mDataLoaderManager,
                                  &MockDataLoaderManager::bindToDataLoaderOkWith10sDelay));
    timedCallback();
}

TEST_F(IncrementalServiceTest, testStartDataLoaderUnhealthyStorage) {
    mIncFs->openMountSuccess();